Patchwork [STABLE] hg: use "os.path.join()" to join path components which may be empty (issue4203)

login
register
mail settings
Submitter Katsunori FUJIWARA
Date March 25, 2014, 10:36 a.m.
Message ID <2d982084f093a9d2bc0f.1395743783@juju>
Download mbox | patch
Permalink /patch/4060/
State Accepted
Commit dda11e79952959a87298e5e094bf087c1837a0a3
Headers show

Comments

Katsunori FUJIWARA - March 25, 2014, 10:36 a.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1395743657 -32400
#      Tue Mar 25 19:34:17 2014 +0900
# Branch stable
# Node ID 2d982084f093a9d2bc0f9927ffc28a85884b1563
# Parent  e5641536e4d509b2dc5fab783344f86ea61b17c2
hg: use "os.path.join()" to join path components which may be empty (issue4203)

Changset 2d0ab571b822 rewriting "hg.copystore()" with vfs uses
'dstbase + "/lock"' instead of "os.path.join()", because target files
given from "store.copyfiles()" already uses "/" as path separator

But in the repository using revlog format 0, "dstbase" becomes empty
("data" directory is located under ".hg" directly), and 'dstbase +
"/lock"' is treated as "/lock": in almost all cases, write access to
"/lock" causes "permission denied".

This patch uses "os.path.join()" to join path components which may be
empty in "hg.copystore()".
David Soria Parra - March 25, 2014, 4:54 p.m.
FUJIWARA Katsunori <foozy@lares.dti.ne.jp> writes:

> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1395743657 -32400
> #      Tue Mar 25 19:34:17 2014 +0900
> # Branch stable
> # Node ID 2d982084f093a9d2bc0f9927ffc28a85884b1563
> # Parent  e5641536e4d509b2dc5fab783344f86ea61b17c2
> hg: use "os.path.join()" to join path components which may be empty (issue4203)
>
> Changset 2d0ab571b822 rewriting "hg.copystore()" with vfs uses
> 'dstbase + "/lock"' instead of "os.path.join()", because target files
> given from "store.copyfiles()" already uses "/" as path separator
>
> But in the repository using revlog format 0, "dstbase" becomes empty
> ("data" directory is located under ".hg" directly), and 'dstbase +
> "/lock"' is treated as "/lock": in almost all cases, write access to
> "/lock" causes "permission denied".
>
> This patch uses "os.path.join()" to join path components which may be
> empty in "hg.copystore()".

Looks good to me. Thank you.
Queued to http://hg.netv6.net/hg-fb-reviewers.

Patch

diff --git a/mercurial/hg.py b/mercurial/hg.py
--- a/mercurial/hg.py
+++ b/mercurial/hg.py
@@ -213,8 +213,10 @@ 
                 dstvfs.mkdir(dstbase)
             if srcvfs.exists(f):
                 if f.endswith('data'):
+                    # 'dstbase' may be empty (e.g. revlog format 0)
+                    lockfile = os.path.join(dstbase, "lock")
                     # lock to avoid premature writing to the target
-                    destlock = lock.lock(dstvfs, dstbase + "/lock")
+                    destlock = lock.lock(dstvfs, lockfile)
                 hardlink, n = util.copyfiles(srcvfs.join(f), dstvfs.join(f),
                                              hardlink)
                 num += n
diff --git a/tests/test-clone.t b/tests/test-clone.t
--- a/tests/test-clone.t
+++ b/tests/test-clone.t
@@ -621,3 +621,17 @@ 
 #endif
 
   $ cd ..
+
+Test clone from the repository in (emulated) revlog format 0 (issue4203):
+
+  $ mkdir issue4203
+  $ mkdir -p src/.hg
+  $ echo foo > src/foo
+  $ hg -R src add src/foo
+  $ hg -R src commit -m '#0'
+  $ hg -R src log -q
+  0:e1bab28bca43
+  $ hg clone -U -q src dst
+  $ hg -R dst log -q
+  0:e1bab28bca43
+  $ cd ..