[PATCH stable] patch: allow empty patches on existing files (issue2135)
Mads Kiilerich
mads at kiilerich.com
Sun Oct 28 11:49:38 CDT 2012
Durham Goode wrote, On 10/26/2012 04:42 AM:
> # HG changeset patch
> # User Durham Goode <durham at fb.com>
> # Date 1351103179 25200
> # Node ID e010bb5b0ee2f9e9241a3dddb7faf0fe2f22a71f
> # Parent 917a37b76845811aaea1dc9b1ad0a8110e8b7439
> patch: allow empty patches on existing files (issue2135)
>
> When applying a patch that added an empty file,
> the patch would fail if the file already existed at the destination.
Patch is patch and is usually very rigid, always creating rejects. A
patch that already has been applied (or where the identical change
already has been made) can not be applied correctly again. In best case
it will fail, in worst case it will apply but be wrong.
I don't think it is a good idea to make patch smart in this single case
where it easily and quite safely can be made "smart" and ignore an order
to create an empty file because a file by that name already exists. It
will make patch more complex and less reliable and less consistent and
will probably not be a real help in any real use cases.
Actually, it seems like it used to be that way, but that was a bug that
was fixed by ee574cfd0c32.
The old/existing behaviour in your test case is:
cannot create b3: destination already exists
abort: fix up the merge and run hg transplant --continue
[255]
As far as I can see it is more correct than the new.
The patch behaviour shown in
http://bz.selenic.com/show_bug.cgi?id=2135#c6 also seems correct.
Anyway:
> This changes patch.py to succeed in that scenario, and emits a
> warning message notifying the user of this case. Example:
>
> file foo.py not changed: file already exists and patch file is empty
It would be more correct to say:
skipped creating empty foo.py: file already exists
... but that also makes it more obvious that it shouldn't skip making a
new empty file just because "another" file with the same name exists.
> Adds a test for this in the transplant test suite, since that's
> where the issue was originally encountered.
The code path can also be reached from 'hg import'. I think it would be
relevant (and faster) to test it in test-import-git.t.
It might still be relevant to test it in context of transplant - I don't
know.
> diff -r 917a37b76845 -r e010bb5b0ee2 mercurial/patch.py
> --- a/mercurial/patch.py Tue Oct 23 09:28:42 2012 +0200
> +++ b/mercurial/patch.py Wed Oct 24 11:26:19 2012 -0700
> @@ -1328,10 +1328,14 @@
> # must be created
> data = ''
> if data or mode:
> - if (gp.op in ('ADD', 'RENAME', 'COPY')
> + if (gp.op in ('RENAME', 'COPY')
> and backend.exists(gp.path)):
> raise PatchError(_("cannot create %s: destination "
> "already exists") % gp.path)
> + if (gp.op == 'ADD' and backend.exists(gp.path)):
> + ui.warn(_('file %s not changed: file already exists '
> + 'and file patch is empty\n') % gp.path)
> + continue
> backend.setfile(gp.path, data, mode, gp.oldpath)
> continue
> try:
> diff -r 917a37b76845 -r e010bb5b0ee2 tests/test-transplant.t
> --- a/tests/test-transplant.t Tue Oct 23 09:28:42 2012 +0200
> +++ b/tests/test-transplant.t Wed Oct 24 11:26:19 2012 -0700
> @@ -632,6 +632,22 @@
> skipping emptied changeset 7a7d57e15850
> $ cd ..
>
> +Issue2135: test transplanting an added empty file onto an existing file
> +
> + $ hg init emptyfilerepo
> + $ cd emptyfilerepo
> + $ touch b3
> + $ hg ci -Am addb3
> + adding b3
> + $ cd ../t
> + $ hg transplant -s ../emptyfilerepo 0
> + searching for changes
> + warning: repository is unrelated
> + applying 74c381100194
> + file b3 not changed: file already exists and file patch is empty
> + skipping emptied changeset 74c381100194
> + $ cd ..
> +
> Explicitly kill daemons to let the test exit on Windows
>
> $ "$TESTDIR/killdaemons.py" $DAEMON_PIDS
/Mads
More information about the Mercurial-devel
mailing list