Patchwork [1,of,2] largefiles: use the core file copy logic to validate the destination path

login
register
mail settings
Submitter Matt Harbison
Date Feb. 3, 2015, 3:41 a.m.
Message ID <e91865b59a4ef9eaf56c.1422934862@Envy>
Download mbox | patch
Permalink /patch/7623/
State Accepted
Headers show

Comments

Matt Harbison - Feb. 3, 2015, 3:41 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1422681209 18000
#      Sat Jan 31 00:13:29 2015 -0500
# Node ID e91865b59a4ef9eaf56cc20bc61c2665a08402db
# Parent  8b88870cbd1eeefaee0af053ae36728f8c0a1847
largefiles: use the core file copy logic to validate the destination path

The destination is validated by pathutil.canonpath() for illegal components, and
that it is in the repository.  The logic for creating the standin directory tree
was calling this before cmdutil.copy(), but without the destination file name
component.  The cmdutil.copy() logic also calls pathutil.canonpath(), but with
the file name component.  By always calling the core logic first, the error
message is always consistent.  Specifically, the old behavior for these tests
was to say '.hg' contains an illegal component, and '..' is not under root.

A user wasn't likely to notice the discrepancy, but this eliminates a needless
difference when running the test suite with --config extensions.largefiles=.

Patch

diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -559,16 +559,6 @@ 
         # this isn't legal, let the original function deal with it
         return orig(ui, repo, pats, opts, rename)
 
-    def makestandin(relpath):
-        path = pathutil.canonpath(repo.root, repo.getcwd(), relpath)
-        return os.path.join(repo.wjoin(lfutil.standin(path)))
-
-    fullpats = scmutil.expandpats(pats)
-    dest = fullpats[-1]
-
-    if os.path.isdir(dest):
-        if not os.path.isdir(makestandin(dest)):
-            os.makedirs(makestandin(dest))
     # This could copy both lfiles and normal files in one command,
     # but we don't want to do that. First replace their matcher to
     # only match normal files and run it, then replace it to just
@@ -595,6 +585,17 @@ 
     except OSError:
         return result
 
+    def makestandin(relpath):
+        path = pathutil.canonpath(repo.root, repo.getcwd(), relpath)
+        return os.path.join(repo.wjoin(lfutil.standin(path)))
+
+    fullpats = scmutil.expandpats(pats)
+    dest = fullpats[-1]
+
+    if os.path.isdir(dest):
+        if not os.path.isdir(makestandin(dest)):
+            os.makedirs(makestandin(dest))
+
     try:
         try:
             # When we call orig below it creates the standins but we don't add
diff --git a/tests/test-rename.t b/tests/test-rename.t
--- a/tests/test-rename.t
+++ b/tests/test-rename.t
@@ -620,10 +620,16 @@ 
   $ hg rename d1/d11/a1 .hg
   abort: path contains illegal component: .hg/a1 (glob)
   [255]
+  $ hg --config extensions.largefiles= rename d1/d11/a1 .hg
+  abort: path contains illegal component: .hg/a1 (glob)
+  [255]
   $ hg status -C
   $ hg rename d1/d11/a1 ..
   abort: ../a1 not under root '$TESTTMP' (glob)
   [255]
+  $ hg --config extensions.largefiles= rename d1/d11/a1 ..
+  abort: ../a1 not under root '$TESTTMP' (glob)
+  [255]
   $ hg status -C
 
   $ mv d1/d11/a1 .hg