[PATCH] posix: give the cached symlink a real target

Jun Wu quark at fb.com
Wed Nov 30 11:58:28 EST 2016


Looks good to me. Thanks for the fix!
A small nit inline - probably could be fixed on flight.

Excerpts from Martijn Pieters's message of 2016-11-30 16:51:41 +0000:
> # HG changeset patch
> # User Martijn Pieters <mjpieters at fb.com>
> # Date 1480523976 0
> #      Wed Nov 30 16:39:36 2016 +0000
> # Node ID 4053f60f77657f8f54afc72d00bf629b75d0b4b9
> # Parent  9e29d4e4e08b5996adda49cdd0b497d89e2b16ee
> posix: give the cached symlink a real target
> 
> The NamedTemporaryFile file is cleared up so checklink ends up as a dangling
> symlink, causing cp -r in tests to complain on both Solaris and OS X. Use
> a permanent file instead when there is a .hg/cache directory.
> 
> diff --git a/mercurial/posix.py b/mercurial/posix.py
> --- a/mercurial/posix.py
> +++ b/mercurial/posix.py
> @@ -231,10 +231,18 @@
>              cachedir = None
>          name = tempfile.mktemp(dir=checkdir, prefix='checklink-')
>          try:
> -            fd = tempfile.NamedTemporaryFile(dir=checkdir,
> -                                             prefix='hg-checklink-')
> +            fd = None
> +            if cachedir is None:
> +                fd = tempfile.NamedTemporaryFile(dir=checkdir,
> +                                                 prefix='hg-checklink-')
> +                target = os.path.basename(fd.name)
> +            else:
> +                # create a fixed file to link to; doesn't matter if it
> +                # already exists.
> +                target = 'checklink-target'
> +                open(os.path.join(cachedir, target), 'w')

Should we append ".close()" here to just make things explicit?

>              try:
> -                os.symlink(os.path.basename(fd.name), name)
> +                os.symlink(target, name)
>                  if cachedir is None:
>                      os.unlink(name)
>                  else:
> @@ -249,7 +257,8 @@
>                      continue
>                  raise
>              finally:
> -                fd.close()
> +                if fd is not None:
> +                    fd.close()
>          except AttributeError:
>              return False
>          except OSError as inst:
> diff --git a/tests/test-clone.t b/tests/test-clone.t
> --- a/tests/test-clone.t
> +++ b/tests/test-clone.t
> @@ -33,6 +33,7 @@
>    branch2-served
>    checkisexec
>    checklink
> +  checklink-target
>    checknoexec
>    rbc-names-v1
>    rbc-revs-v1
> @@ -50,6 +51,7 @@
>    branch2-served
>    checkisexec
>    checklink
> +  checklink-target
>  
>    $ cat a
>    a
> diff --git a/tests/test-hardlinks.t b/tests/test-hardlinks.t
> --- a/tests/test-hardlinks.t
> +++ b/tests/test-hardlinks.t
> @@ -212,6 +212,8 @@
>    2 r4/.hg/branch
>    2 r4/.hg/cache/branch2-served
>    2 r4/.hg/cache/checkisexec
> +  3 r4/.hg/cache/checklink (?)
> +  ? r4/.hg/cache/checklink-target (glob)
>    2 r4/.hg/cache/checknoexec
>    2 r4/.hg/cache/rbc-names-v1
>    2 r4/.hg/cache/rbc-revs-v1
> @@ -250,6 +252,7 @@
>    1 r4/.hg/branch
>    2 r4/.hg/cache/branch2-served
>    2 r4/.hg/cache/checkisexec
> +  2 r4/.hg/cache/checklink-target
>    2 r4/.hg/cache/checknoexec
>    2 r4/.hg/cache/rbc-names-v1
>    2 r4/.hg/cache/rbc-revs-v1
> diff --git a/tests/test-tags.t b/tests/test-tags.t
> --- a/tests/test-tags.t
> +++ b/tests/test-tags.t
> @@ -674,6 +674,7 @@
>    branch2-served
>    checkisexec
>    checklink
> +  checklink-target
>    hgtagsfnodes1
>    rbc-names-v1
>    rbc-revs-v1
> @@ -700,6 +701,7 @@
>    branch2-served
>    checkisexec
>    checklink
> +  checklink-target
>    hgtagsfnodes1
>    rbc-names-v1
>    rbc-revs-v1


More information about the Mercurial-devel mailing list