Patchwork manifestmerge: local unknown, remote created: don't traverse symlinks

login
register
mail settings
Submitter Siddharth Agarwal
Date May 8, 2013, 9:40 p.m.
Message ID <0fc5b11beaa38d4ffde0.1368049210@dev1091.prn1.facebook.com>
Download mbox | patch
Permalink /patch/1591/
State Superseded
Commit 9bfa86746c9c1f6ab51deb8f174ffc482417d09f
Headers show

Comments

Siddharth Agarwal - May 8, 2013, 9:40 p.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1368047461 25200
#      Wed May 08 14:11:01 2013 -0700
# Node ID 0fc5b11beaa38d4ffde0bc61ee5f798b392cdd2e
# Parent  c4f58fe8798f11a2b851412b1b8f3d0e23c7ed29
manifestmerge: local unknown, remote created: don't traverse symlinks

To figure out what to do with locally unknown files, Mercurial attempts to read
them if they exist. When an attempt is made to read a file that exists but
traverses a symlink, Mercurial aborts.

With this patch, we first ensure that the file doesn't traverse a symlink
before opening it. This is fine because a file being "remote created" means the
symlink doesn't exist remotely, which means it will be deleted in the apply
phase.
Augie Fackler - May 9, 2013, 1:45 p.m.
On Wed, May 08, 2013 at 02:40:10PM -0700, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1368047461 25200
> #      Wed May 08 14:11:01 2013 -0700
> # Node ID 0fc5b11beaa38d4ffde0bc61ee5f798b392cdd2e
> # Parent  c4f58fe8798f11a2b851412b1b8f3d0e23c7ed29
> manifestmerge: local unknown, remote created: don't traverse symlinks

Queued, thanks.

>
> To figure out what to do with locally unknown files, Mercurial attempts to read
> them if they exist. When an attempt is made to read a file that exists but
> traverses a symlink, Mercurial aborts.
>
> With this patch, we first ensure that the file doesn't traverse a symlink
> before opening it. This is fine because a file being "remote created" means the
> symlink doesn't exist remotely, which means it will be deleted in the apply
> phase.
>
> diff --git a/mercurial/merge.py b/mercurial/merge.py
> --- a/mercurial/merge.py
> +++ b/mercurial/merge.py
> @@ -95,6 +95,7 @@
>  def _checkunknownfile(repo, wctx, mctx, f):
>      return (not repo.dirstate._ignore(f)
>          and os.path.isfile(repo.wjoin(f))
> +        and repo.wopener.audit.check(f)
>          and repo.dirstate.normalize(f) not in repo.dirstate
>          and mctx[f].cmp(wctx[f]))
>
> diff --git a/tests/test-symlinks.t b/tests/test-symlinks.t
> --- a/tests/test-symlinks.t
> +++ b/tests/test-symlinks.t
> @@ -160,6 +160,15 @@
>    adding bar/a
>    adding foo
>    removing foo/a
> +
> +commit and update back
> +
> +  $ hg ci -mb
> +  $ hg up '.^'
> +  1 files updated, 0 files merged, 2 files removed, 0 files unresolved
> +  $ hg up tip
> +  2 files updated, 0 files merged, 1 files removed, 0 files unresolved
> +
>    $ cd ..
>
>  == root of repository is symlinked ==
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Bryan O'Sullivan - May 9, 2013, 7:17 p.m.
On Thu, May 9, 2013 at 6:45 AM, Augie Fackler <raf@durin42.com> wrote:

> Queued, thanks.
>

Did you put this on the stable branch, perchance? Seems like that's where
it belongs.
Augie Fackler - May 9, 2013, 8:13 p.m.
On Thu, May 9, 2013 at 3:17 PM, Bryan O'Sullivan <bos@serpentine.com> wrote:
>
> On Thu, May 9, 2013 at 6:45 AM, Augie Fackler <raf@durin42.com> wrote:
>>
>> Queued, thanks.
>
>
> Did you put this on the stable branch, perchance? Seems like that's where it
> belongs.

I didn't, but if you feel it does there's always graft.
Katsunori FUJIWARA - May 10, 2013, 2:22 p.m.
At Thu, 9 May 2013 16:13:58 -0400,
Augie Fackler wrote:
> 
> On Thu, May 9, 2013 at 3:17 PM, Bryan O'Sullivan <bos@serpentine.com> wrote:
> >
> > On Thu, May 9, 2013 at 6:45 AM, Augie Fackler <raf@durin42.com> wrote:
> >>
> >> Queued, thanks.
> >
> >
> > Did you put this on the stable branch, perchance? Seems like that's where it
> > belongs.
> 
> I didn't, but if you feel it does there's always graft.

I'm planning to re-post refined patches around
"merge._checkunknownfile()" below:

  (1) check collision between file(remote) and directory(local unknown)

    http://www.selenic.com/pipermail/mercurial-devel/2013-May/051084.html

  (2) check collision between file(remote) and symlink(local unknown)
      or vice versa

    http://www.selenic.com/pipermail/mercurial-devel/2013-May/051083.html

as a part of series, which also contains below, to fix collision
problems at (linear/branch) merging:

  (3) issue29: check collision between file and directory in merged
      manifest

    http://www.selenic.com/pipermail/mercurial-devel/2013-May/051082.html

  (4) check collision between normal and large files in merged
      manifest on icasefs

    http://www.selenic.com/pipermail/mercurial-devel/2013-May/051087.html

IMHO, (1) and (2) should be applied on Siddharth's work.

Should I post (1)/(2) for default and (3)/(4) for stable separately ?

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -95,6 +95,7 @@ 
 def _checkunknownfile(repo, wctx, mctx, f):
     return (not repo.dirstate._ignore(f)
         and os.path.isfile(repo.wjoin(f))
+        and repo.wopener.audit.check(f)
         and repo.dirstate.normalize(f) not in repo.dirstate
         and mctx[f].cmp(wctx[f]))
 
diff --git a/tests/test-symlinks.t b/tests/test-symlinks.t
--- a/tests/test-symlinks.t
+++ b/tests/test-symlinks.t
@@ -160,6 +160,15 @@ 
   adding bar/a
   adding foo
   removing foo/a
+
+commit and update back
+
+  $ hg ci -mb
+  $ hg up '.^'
+  1 files updated, 0 files merged, 2 files removed, 0 files unresolved
+  $ hg up tip
+  2 files updated, 0 files merged, 1 files removed, 0 files unresolved
+
   $ cd ..
 
 == root of repository is symlinked ==