Patchwork D1258: tersestatus: make sure we use os.path.* to split paths (issue5715)

login
register
mail settings
Submitter phabricator
Date Oct. 27, 2017, 8:40 p.m.
Message ID <differential-rev-PHID-DREV-cthezy64mwzrld7urnc7-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/25294/
State Superseded
Headers show

Comments

phabricator - Oct. 27, 2017, 8:40 p.m.
pulkit created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Not using os.path.* functions can cause problem on windows which led to failure
  of test-status-terse.t on windows. This patch make sure we use them.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1258

AFFECTED FILES
  mercurial/cmdutil.py

CHANGE DETAILS




To: pulkit, #hg-reviewers
Cc: mercurial-devel
phabricator - Oct. 27, 2017, 8:42 p.m.
pulkit added a subscriber: mharbison72.
pulkit added a comment.


  @mharbison72 I didn't has access to windows machine but I think this should solve the issue, apart from test failure related to different separator which we can glob out. Please test it and let me know whether it works or not.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1258

To: pulkit, #hg-reviewers
Cc: mharbison72, mercurial-devel
phabricator - Oct. 28, 2017, 3:34 a.m.
mharbison72 added a comment.


  No joy.  But it looks like you can use '/' unconditionally, because scmutil.status paths are in repo normalized format.  I'll send a patch.
  
  I think there's something else goofy here:
  
  $ ../hg st -c --terse c ../mercurial/help/
  C ..\mercurial\
  
  I'd expect the output to be:
  
  $ ../hg st -c --terse c ../mercurial/help/
  C ..\mercurial\help
  
  I can write up another bug if you don't have any ideas off the top of your head.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1258

To: pulkit, #hg-reviewers
Cc: mharbison72, mercurial-devel
phabricator - Oct. 28, 2017, 7:59 a.m.
yuja requested changes to this revision.
yuja added a comment.
This revision now requires changes to proceed.


  New patch landed.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1258

To: pulkit, #hg-reviewers, yuja
Cc: yuja, mharbison72, mercurial-devel
phabricator - Oct. 30, 2017, 9:01 p.m.
durin42 commandeered this revision.
durin42 added a reviewer: pulkit.
durin42 added a comment.


  Obsoleted by 362096 as far as I can tell.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1258

To: durin42, #hg-reviewers, yuja, pulkit
Cc: durin42, yuja, mharbison72, mercurial-devel

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -404,11 +404,6 @@ 
 
     return commit(ui, repo, recordinwlock, pats, opts)
 
-
-# extracted at module level as it's required each time a file will be added
-# to dirnode class object below
-pathsep = pycompat.ossep
-
 class dirnode(object):
     """
     Represent a directory in user working copy with information required for
@@ -435,6 +430,19 @@ 
         """Add a file in this directory as a direct child."""
         self.files.append((filename, status))
 
+    def _splitbyroot(self, path):
+        """
+        splits the path from the first occurence of os.sep and returns a tuple
+        (rootdirectory, restofthepath)
+        """
+        #XXX: must be replaced with pathlib once we drop Py2.7
+        parentdir = os.path.dirname(path)
+        temp = path
+        while parentdir:
+            temp = parentdir
+            parentdir = os.path.dirname(parentdir)
+        return temp, path[len(temp) + 1:]
+
     def addfile(self, filename, status):
         """
         Add a file to this directory or to its direct parent directory.
@@ -446,8 +454,8 @@ 
 
         # the filename contains a path separator, it means it's not the direct
         # child of this directory
-        if pathsep in filename:
-            subdir, filep = filename.split(pathsep, 1)
+        if pycompat.ossep in filename:
+            subdir, filep = self._splitbyroot(filename)
 
             # does the dirnode object for subdir exists
             if subdir not in self.subdirs: