Patchwork [4,of,7] match: don't print explicitly listed files with wrong case (BC)

login
register
mail settings
Submitter via Mercurial-devel
Date May 22, 2017, 6:16 a.m.
Message ID <fcf2e822e20b4c25e452.1495433764@martinvonz.svl.corp.google.com>
Download mbox | patch
Permalink /patch/20815/
State Accepted
Headers show

Comments

via Mercurial-devel - May 22, 2017, 6:16 a.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@google.com>
# Date 1495148746 25200
#      Thu May 18 16:05:46 2017 -0700
# Node ID fcf2e822e20b4c25e4525057a0fdf8221e48b0ec
# Parent  6bf950ac0b443b409e744bb0cff6b340197dbdd2
match: don't print explicitly listed files with wrong case (BC)

On case-insensitive file systems, if file A exists and you try to
remove it (or add, etc.) by specifying a different case, you will see
something like this:

  $ hg rm a
  removing file A

I honestly found this surprising because it seems to me like it was
explicitly listed by the user. Still, there is a comment in the code
describing it, so it is very clearly intentional. The code was added
in baa11dde8c0e (match: add a subclass for dirstate normalizing of the
matched patterns, 2015-04-12).

I'm going to do a lot of refactoring to matchers and the feature
mentioned above is going to get in my way. I'm therefore removing it
for the time being and we can hopefully add it back when I'm done.
Yuya Nishihara - May 22, 2017, 2:38 p.m.
On Sun, 21 May 2017 23:16:04 -0700, Martin von Zweigbergk via Mercurial-devel wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1495148746 25200
> #      Thu May 18 16:05:46 2017 -0700
> # Node ID fcf2e822e20b4c25e4525057a0fdf8221e48b0ec
> # Parent  6bf950ac0b443b409e744bb0cff6b340197dbdd2
> match: don't print explicitly listed files with wrong case (BC)
> 
> On case-insensitive file systems, if file A exists and you try to
> remove it (or add, etc.) by specifying a different case, you will see
> something like this:
> 
>   $ hg rm a
>   removing file A
> 
> I honestly found this surprising because it seems to me like it was
> explicitly listed by the user. Still, there is a comment in the code
> describing it, so it is very clearly intentional. The code was added
> in baa11dde8c0e (match: add a subclass for dirstate normalizing of the
> matched patterns, 2015-04-12).
> 
> I'm going to do a lot of refactoring to matchers and the feature
> mentioned above is going to get in my way. I'm therefore removing it
> for the time being and we can hopefully add it back when I'm done.
> 
> diff --git a/mercurial/match.py b/mercurial/match.py
> --- a/mercurial/match.py
> +++ b/mercurial/match.py
> @@ -450,19 +450,11 @@
>          init(root, cwd, patterns, include, exclude, default, auditor=auditor,
>               ctx=ctx, listsubrepos=listsubrepos, badfn=badfn)
>  
> -        # m.exact(file) must be based off of the actual user input, otherwise
> -        # inexact case matches are treated as exact, and not noted without -v.
> -        if self._files:
> -            roots, dirs = _rootsanddirs(self._kp)
> -            self._fileset = set(roots)
> -            self._fileset.update(dirs)

Dropping inexact message seems fine. Matt, please let us know if you remember
a pitfall other than the status message. icasefs seems really picky.
Matt Harbison - May 23, 2017, 4:01 a.m.
On Mon, 22 May 2017 02:16:04 -0400, Martin von Zweigbergk
<martinvonz@google.com> wrote:

> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1495148746 25200
> #      Thu May 18 16:05:46 2017 -0700
> # Node ID fcf2e822e20b4c25e4525057a0fdf8221e48b0ec
> # Parent  6bf950ac0b443b409e744bb0cff6b340197dbdd2
> match: don't print explicitly listed files with wrong case (BC)
>
> On case-insensitive file systems, if file A exists and you try to
> remove it (or add, etc.) by specifying a different case, you will see
> something like this:
>
>   $ hg rm a
>   removing file A
>
> I honestly found this surprising because it seems to me like it was
> explicitly listed by the user. Still, there is a comment in the code
> describing it, so it is very clearly intentional. The code was added
> in baa11dde8c0e (match: add a subclass for dirstate normalizing of the
> matched patterns, 2015-04-12).

That behavior predates this commit.  I added this commit, probably to be
consistent with the existing behavior that I observed with forget a few
weeks before:

https://www.mercurial-scm.org/repo/hg/rev/c780a63f61ca

My assumed reason for this output was simply that if you don't type the
filename exactly (e.g., like implying a file by naming a directory), it
prints out the actual name.  I've never played with a repo that has
Foo.txt and foo.txt, and then brought it over to icasefs to give it a
spin, so I'm not sure how important this was.

I don't have a problem with dropping it for now.  It might be nice to get
back when the cleanup is done, if possible.

As far as pitfalls with icasefs, I went back to all the commits in the
obsolete line, and it looks like baa11dde8c0e talks about the ones that I
knew about at the time.  I'm not aware that many (any?) were subsequently
fixed.  The subrepo thing is still an issue.
Matt Harbison - May 23, 2017, 4:01 a.m.
On Mon, 22 May 2017 02:16:04 -0400, Martin von Zweigbergk
<martinvonz@google.com> wrote:

> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1495148746 25200
> #      Thu May 18 16:05:46 2017 -0700
> # Node ID fcf2e822e20b4c25e4525057a0fdf8221e48b0ec
> # Parent  6bf950ac0b443b409e744bb0cff6b340197dbdd2
> match: don't print explicitly listed files with wrong case (BC)
>
> On case-insensitive file systems, if file A exists and you try to
> remove it (or add, etc.) by specifying a different case, you will see
> something like this:
>
>   $ hg rm a
>   removing file A
>
> I honestly found this surprising because it seems to me like it was
> explicitly listed by the user. Still, there is a comment in the code
> describing it, so it is very clearly intentional. The code was added
> in baa11dde8c0e (match: add a subclass for dirstate normalizing of the
> matched patterns, 2015-04-12).

That behavior predates this commit.  I added this commit, probably to be
consistent with the existing behavior that I observed with forget a few
weeks before:

https://www.mercurial-scm.org/repo/hg/rev/c780a63f61ca

My assumed reason for this output was simply that if you don't type the
filename exactly (e.g., like implying a file by naming a directory), it
prints out the actual name.  I've never played with a repo that has
Foo.txt and foo.txt, and then brought it over to icasefs to give it a
spin, so I'm not sure how important this was.

I don't have a problem with dropping it for now.  It might be nice to get
back when the cleanup is done, if possible.

As far as pitfalls with icasefs, I went back to all the commits in the
obsolete line, and it looks like baa11dde8c0e talks about the ones that I
knew about at the time.  I'm not aware that many (any?) were subsequently
fixed.  The subrepo thing is still an issue.

Patch

diff --git a/mercurial/match.py b/mercurial/match.py
--- a/mercurial/match.py
+++ b/mercurial/match.py
@@ -450,19 +450,11 @@ 
         init(root, cwd, patterns, include, exclude, default, auditor=auditor,
              ctx=ctx, listsubrepos=listsubrepos, badfn=badfn)
 
-        # m.exact(file) must be based off of the actual user input, otherwise
-        # inexact case matches are treated as exact, and not noted without -v.
-        if self._files:
-            roots, dirs = _rootsanddirs(self._kp)
-            self._fileset = set(roots)
-            self._fileset.update(dirs)
-
     def _normalize(self, patterns, default, root, cwd, auditor, warn):
-        self._kp = super(icasefsmatcher, self)._normalize(patterns, default,
-                                                          root, cwd, auditor,
-                                                          warn)
+        kp = super(icasefsmatcher, self)._normalize(patterns, default, root,
+                                                    cwd, auditor, warn)
         kindpats = []
-        for kind, pats, source in self._kp:
+        for kind, pats, source in kp:
             if kind not in ('re', 'relre'):  # regex can't be normalized
                 p = pats
                 pats = self._dsnormalize(pats)
diff --git a/tests/test-add.t b/tests/test-add.t
--- a/tests/test-add.t
+++ b/tests/test-add.t
@@ -196,7 +196,6 @@ 
   adding CapsDir1/CapsDir/SubDir/Def.txt (glob)
 
   $ hg forget capsdir1/capsdir/abc.txt
-  removing CapsDir1/CapsDir/AbC.txt (glob)
 
   $ hg forget capsdir1/capsdir
   removing CapsDir1/CapsDir/SubDir/Def.txt (glob)
@@ -232,7 +231,6 @@ 
   +def
 
   $ hg mv CapsDir1/CapsDir/abc.txt CapsDir1/CapsDir/ABC.txt
-  moving CapsDir1/CapsDir/AbC.txt to CapsDir1/CapsDir/ABC.txt (glob)
   $ hg ci -m "case changing rename" CapsDir1/CapsDir/AbC.txt CapsDir1/CapsDir/ABC.txt
 
   $ hg status -A capsdir1/capsdir
diff --git a/tests/test-casefolding.t b/tests/test-casefolding.t
--- a/tests/test-casefolding.t
+++ b/tests/test-casefolding.t
@@ -9,7 +9,6 @@ 
   $ cd repo1
   $ echo a > a
   $ hg add A
-  adding a
   $ hg st
   A a
   $ hg ci -m adda
@@ -71,14 +70,12 @@ 
   A D/c
   $ hg ci -m addc D/c
   $ hg mv d/b d/e
-  moving D/b to D/e (glob)
   $ hg st
   A D/e
   R D/b
   $ hg revert -aq
   $ rm d/e
   $ hg mv d/b D/B
-  moving D/b to D/B (glob)
   $ hg st
   A D/B
   R D/b