[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