Patchwork [STABLE] checkexec: create destination directory if necessary

login
register
mail settings
Submitter Boris Feld
Date Nov. 15, 2018, 10:48 a.m.
Message ID <99a25b3b768a6434c09f.1542278931@localhost.localdomain>
Download mbox | patch
Permalink /patch/36601/
State Superseded
Headers show

Comments

Boris Feld - Nov. 15, 2018, 10:48 a.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1542247763 -3600
#      Thu Nov 15 03:09:23 2018 +0100
# Branch stable
# Node ID 99a25b3b768a6434c09f66f6980ac76de0245366
# Parent  e5f54c4ec075f28770ae16619a839e6431df0935
# EXP-Topic wcache
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 99a25b3b768a
checkexec: create destination directory if necessary

Since 460733327640, a "share" use the cache of the source repository. A side
effect is that no `.hg/cache` directory exists in the "share" anymore. As a
result, the checkexec logic can't use it to create its temporary file and have
to use the working copy for that.

This is suboptimal, it pollutes the working copy and prevents them to keep the
file around in cache. We do not want to use the cache directory for the share
target, it might be on a different file system.

So instead, we (try to) create the directory if it is missing. This is a
simple change that fixes the current behavior regression on stable.

On default, we should probably ensure the proper directories are created when
initializing the repository. We should also introduce a 'wcache' directory to
hold cache file related to the working copy. This would clarify the cache
situation regarding shares.

The tests catch a couple of other affected cases.
Yuya Nishihara - Nov. 17, 2018, 4:14 a.m.
On Thu, 15 Nov 2018 11:48:51 +0100, Boris Feld wrote:
> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1542247763 -3600
> #      Thu Nov 15 03:09:23 2018 +0100
> # Branch stable
> # Node ID 99a25b3b768a6434c09f66f6980ac76de0245366
> # Parent  e5f54c4ec075f28770ae16619a839e6431df0935
> # EXP-Topic wcache
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 99a25b3b768a
> checkexec: create destination directory if necessary

> --- a/mercurial/posix.py
> +++ b/mercurial/posix.py
> @@ -181,7 +181,15 @@ def checkexec(path):
>  
>      try:
>          EXECFLAGS = stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH
> -        cachedir = os.path.join(path, '.hg', 'cache')
> +        basedir = os.path.join(path, '.hg')
> +        cachedir = os.path.join(basedir, 'cache')
> +        if not os.path.exists(cachedir):
> +            try:
> +                os.mkdir(cachedir)

This handles the case where '.hg' directory doesn't exist. Can you add inline
comment so we won't change it to os.makedirs() by mistake?

> +                copymode(basedir, cachedir)

It disagree with the inline comment. The permission of the cache directory
has to be inherited from the store, not from the .hg directory.

> --- a/tests/test-inherit-mode.t
> +++ b/tests/test-inherit-mode.t
> @@ -67,8 +67,11 @@ new directories are setgid
>    $ "$PYTHON" ../printmodes.py .
>    00700 ./.hg/
>    00600 ./.hg/00changelog.i
> -  00770 ./.hg/cache/
> +  00700 ./.hg/cache/

Patch

diff --git a/mercurial/posix.py b/mercurial/posix.py
--- a/mercurial/posix.py
+++ b/mercurial/posix.py
@@ -181,7 +181,15 @@  def checkexec(path):
 
     try:
         EXECFLAGS = stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH
-        cachedir = os.path.join(path, '.hg', 'cache')
+        basedir = os.path.join(path, '.hg')
+        cachedir = os.path.join(basedir, 'cache')
+        if not os.path.exists(cachedir):
+            try:
+                os.mkdir(cachedir)
+                copymode(basedir, cachedir)
+            except (IOError, OSError):
+                # we other fallback logic triggers
+                pass
         if os.path.isdir(cachedir):
             checkisexec = os.path.join(cachedir, 'checkisexec')
             checknoexec = os.path.join(cachedir, 'checknoexec')
diff --git a/tests/test-fncache.t b/tests/test-fncache.t
--- a/tests/test-fncache.t
+++ b/tests/test-fncache.t
@@ -88,6 +88,9 @@  Non store repo:
   .hg/00manifest.i
   .hg/cache
   .hg/cache/branch2-served
+  .hg/cache/checkisexec
+  .hg/cache/checklink
+  .hg/cache/checklink-target
   .hg/cache/manifestfulltextcache (reporevlogstore !)
   .hg/cache/rbc-names-v1
   .hg/cache/rbc-revs-v1
@@ -122,6 +125,9 @@  Non fncache repo:
   .hg/00changelog.i
   .hg/cache
   .hg/cache/branch2-served
+  .hg/cache/checkisexec
+  .hg/cache/checklink
+  .hg/cache/checklink-target
   .hg/cache/manifestfulltextcache (reporevlogstore !)
   .hg/cache/rbc-names-v1
   .hg/cache/rbc-revs-v1
diff --git a/tests/test-inherit-mode.t b/tests/test-inherit-mode.t
--- a/tests/test-inherit-mode.t
+++ b/tests/test-inherit-mode.t
@@ -67,8 +67,11 @@  new directories are setgid
   $ "$PYTHON" ../printmodes.py .
   00700 ./.hg/
   00600 ./.hg/00changelog.i
-  00770 ./.hg/cache/
+  00700 ./.hg/cache/
   00660 ./.hg/cache/branch2-served
+  00711 ./.hg/cache/checkisexec
+  00777 ./.hg/cache/checklink
+  00600 ./.hg/cache/checklink-target
   00660 ./.hg/cache/manifestfulltextcache (reporevlogstore !)
   00660 ./.hg/cache/rbc-names-v1
   00660 ./.hg/cache/rbc-revs-v1
diff --git a/tests/test-share.t b/tests/test-share.t
--- a/tests/test-share.t
+++ b/tests/test-share.t
@@ -22,16 +22,21 @@  share shouldn't have a store dir
   $ test -d .hg/store
   [1]
 
-share shouldn't have a cache dir, original repo should
+share shouldn't have a full cache dir, original repo should
 
   $ hg branches
   default                        0:d3873e73d99e
   $ hg tags
   tip                                0:d3873e73d99e
-  $ test -d .hg/cache
-  [1]
+  $ ls -1 .hg/cache
+  checkisexec
+  checklink
+  checklink-target
   $ ls -1 ../repo1/.hg/cache
   branch2-served
+  checkisexec
+  checklink
+  checklink-target
   manifestfulltextcache (reporevlogstore !)
   rbc-names-v1
   rbc-revs-v1