Patchwork [1,of,3,v4,sparse-ext] sparse: properly error out when absolute paths are used

login
register
mail settings
Submitter Kostia Balytskyi
Date Aug. 4, 2017, 12:45 p.m.
Message ID <d24d6711aa2a5397d52f.1501850757@devvm1416.lla2.facebook.com>
Download mbox | patch
Permalink /patch/22664/
State Accepted
Headers show

Comments

Kostia Balytskyi - Aug. 4, 2017, 12:45 p.m.
# HG changeset patch
# User Kostia Balytskyi <ikostia@fb.com>
# Date 1501711521 25200
#      Wed Aug 02 15:05:21 2017 -0700
# Node ID d24d6711aa2a5397d52f46ccffa64c2a638c088b
# Parent  609606d217659e0a6c1cf6f907b6512be5340e57
sparse: properly error out when absolute paths are used

Current logic is misleading (it says it drops only absolute paths, but
it actually drops all of them), not cross-platform (does not support Windows)
and IMO just wrong (as it should just error out if absolute paths are given).

This commit fixes it.

Patch

diff --git a/mercurial/sparse.py b/mercurial/sparse.py
--- a/mercurial/sparse.py
+++ b/mercurial/sparse.py
@@ -636,10 +636,10 @@  def updateconfig(repo, pats, opts, inclu
             newexclude = set(oldexclude)
             newprofiles = set(oldprofiles)
 
-        if any(pat.startswith('/') for pat in pats):
-            repo.ui.warn(_('warning: paths cannot start with /, ignoring: %s\n')
-                         % ([pat for pat in pats if pat.startswith('/')]))
-        elif include:
+        if any(os.path.isabs(pat) for pat in pats):
+            raise error.Abort(_('paths cannot be absolute'))
+
+        if include:
             newinclude.update(pats)
         elif exclude:
             newexclude.update(pats)
diff --git a/tests/test-sparse.t b/tests/test-sparse.t
--- a/tests/test-sparse.t
+++ b/tests/test-sparse.t
@@ -29,20 +29,22 @@  Absolute paths outside the repo should j
 
 #if no-windows
   $ hg debugsparse --include /foo/bar
-  warning: paths cannot start with /, ignoring: ['/foo/bar']
+  abort: paths cannot be absolute
+  [255]
   $ hg debugsparse --include '$TESTTMP/myrepo/hide'
 
   $ hg debugsparse --include '/root'
-  warning: paths cannot start with /, ignoring: ['/root']
+  abort: paths cannot be absolute
+  [255]
 #else
 TODO: See if this can be made to fail the same way as on Unix
   $ hg debugsparse --include /c/foo/bar
-  abort: c:/foo/bar not under root '$TESTTMP/myrepo' (glob)
+  abort: paths cannot be absolute
   [255]
   $ hg debugsparse --include '$TESTTMP/myrepo/hide'
 
   $ hg debugsparse --include '/c/root'
-  abort: c:/root not under root '$TESTTMP/myrepo' (glob)
+  abort: paths cannot be absolute
   [255]
 #endif