Patchwork [2,of,2] ignore: resolve ignore files relative to config file (issue4473) (BC)

login
register
mail settings
Submitter Siddharth Agarwal
Date Dec. 18, 2014, 4:52 a.m.
Message ID <f9fd51dd63455c6b555b.1418878365@devbig136.prn2.facebook.com>
Download mbox | patch
Permalink /patch/7163/
State Superseded
Headers show

Comments

Siddharth Agarwal - Dec. 18, 2014, 4:52 a.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1418769293 28800
#      Tue Dec 16 14:34:53 2014 -0800
# Node ID f9fd51dd63455c6b555bc4efeac661a9ccef0421
# Parent  a0103901de548b143d3764b27783b9c0acaba185
ignore: resolve ignore files relative to config file (issue4473) (BC)

Previously these would be considered to be relative to the current working
directory. That behavior is both undocumented and doesn't really make sense.
There are two reasonable options for how to resolve relative paths:
- relative to the repo root
- relative to the config file

Resolving these files relative to the repo root matches existing behavior with
hooks. An earlier discussion about this is available at
http://mercurial.markmail.org/thread/tvu7yhzsiywgkjzl.

Thanks to Isaac Jurado <diptongo@gmail.com> for the initial patchset that
spurred the discussion.
Siddharth Agarwal - Dec. 18, 2014, 4:53 a.m.
On 12/17/2014 08:52 PM, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1418769293 28800
> #      Tue Dec 16 14:34:53 2014 -0800
> # Node ID f9fd51dd63455c6b555bc4efeac661a9ccef0421
> # Parent  a0103901de548b143d3764b27783b9c0acaba185
> ignore: resolve ignore files relative to config file (issue4473) (BC)

Gah, I meant relative to repo root :/

>
> Previously these would be considered to be relative to the current working
> directory. That behavior is both undocumented and doesn't really make sense.
> There are two reasonable options for how to resolve relative paths:
> - relative to the repo root
> - relative to the config file
>
> Resolving these files relative to the repo root matches existing behavior with
> hooks. An earlier discussion about this is available at
> http://mercurial.markmail.org/thread/tvu7yhzsiywgkjzl.
>
> Thanks to Isaac Jurado <diptongo@gmail.com> for the initial patchset that
> spurred the discussion.
>
> diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
> --- a/mercurial/dirstate.py
> +++ b/mercurial/dirstate.py
> @@ -130,7 +130,9 @@
>          files = [self._join('.hgignore')]
>          for name, path in self._ui.configitems("ui"):
>              if name == 'ignore' or name.startswith('ignore.'):
> -                files.append(util.expandpath(path))
> +                # we need to use os.path.join here rather than self._join
> +                # because path is arbitrary and user-specified
> +                files.append(os.path.join(self._rootdir, util.expandpath(path)))
>          return ignore.ignore(self._root, files, self._ui.warn)
>  
>      @propertycache
> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -1338,11 +1338,11 @@
>  
>  ``ignore``
>      A file to read per-user ignore patterns from. This file should be
> -    in the same format as a repository-wide .hgignore file. This
> -    option supports hook syntax, so if you want to specify multiple
> -    ignore files, you can do so by setting something like
> -    ``ignore.other = ~/.hgignore2``. For details of the ignore file
> -    format, see the ``hgignore(5)`` man page.
> +    in the same format as a repository-wide .hgignore file. Filenames
> +    are relative to the repository root. This option supports hook syntax,
> +    so if you want to specify multiple ignore files, you can do so by
> +    setting something like ``ignore.other = ~/.hgignore2``. For details
> +    of the ignore file format, see the ``hgignore(5)`` man page.
>  
>  ``interactive``
>      Allow to prompt the user. True or False. Default is True.
> diff --git a/tests/test-hgignore.t b/tests/test-hgignore.t
> --- a/tests/test-hgignore.t
> +++ b/tests/test-hgignore.t
> @@ -80,13 +80,23 @@
>  
>  empty out testhgignore
>    $ echo > .hg/testhgignore
> -  $ echo "glob:*.o" > .hgignore
> +
> +Test relative ignore path (issue4473):
> +
> +  $ cat >> $HGRCPATH << EOF
> +  > [ui]
> +  > ignore.relative = .hg/testhgignorerel
> +  > EOF
> +  $ echo "glob:*.o" > .hg/testhgignorerel
> +  $ cd dir
>    $ hg status
>    A dir/b.o
>    ? .hgignore
>    ? a.c
>    ? syntax
>  
> +  $ cd ..
> +  $ echo > .hg/testhgignorerel
>    $ echo "syntax: glob" > .hgignore
>    $ echo "re:.*\.o" >> .hgignore
>    $ hg status
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -130,7 +130,9 @@ 
         files = [self._join('.hgignore')]
         for name, path in self._ui.configitems("ui"):
             if name == 'ignore' or name.startswith('ignore.'):
-                files.append(util.expandpath(path))
+                # we need to use os.path.join here rather than self._join
+                # because path is arbitrary and user-specified
+                files.append(os.path.join(self._rootdir, util.expandpath(path)))
         return ignore.ignore(self._root, files, self._ui.warn)
 
     @propertycache
diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -1338,11 +1338,11 @@ 
 
 ``ignore``
     A file to read per-user ignore patterns from. This file should be
-    in the same format as a repository-wide .hgignore file. This
-    option supports hook syntax, so if you want to specify multiple
-    ignore files, you can do so by setting something like
-    ``ignore.other = ~/.hgignore2``. For details of the ignore file
-    format, see the ``hgignore(5)`` man page.
+    in the same format as a repository-wide .hgignore file. Filenames
+    are relative to the repository root. This option supports hook syntax,
+    so if you want to specify multiple ignore files, you can do so by
+    setting something like ``ignore.other = ~/.hgignore2``. For details
+    of the ignore file format, see the ``hgignore(5)`` man page.
 
 ``interactive``
     Allow to prompt the user. True or False. Default is True.
diff --git a/tests/test-hgignore.t b/tests/test-hgignore.t
--- a/tests/test-hgignore.t
+++ b/tests/test-hgignore.t
@@ -80,13 +80,23 @@ 
 
 empty out testhgignore
   $ echo > .hg/testhgignore
-  $ echo "glob:*.o" > .hgignore
+
+Test relative ignore path (issue4473):
+
+  $ cat >> $HGRCPATH << EOF
+  > [ui]
+  > ignore.relative = .hg/testhgignorerel
+  > EOF
+  $ echo "glob:*.o" > .hg/testhgignorerel
+  $ cd dir
   $ hg status
   A dir/b.o
   ? .hgignore
   ? a.c
   ? syntax
 
+  $ cd ..
+  $ echo > .hg/testhgignorerel
   $ echo "syntax: glob" > .hgignore
   $ echo "re:.*\.o" >> .hgignore
   $ hg status