Patchwork dirstate: use repository root as base for relative paths read from ui.ignore

login
register
mail settings
Submitter Isaac Jurado
Date April 18, 2013, 7:56 p.m.
Message ID <380de75c43f4a186a1b8.1366315008@findus>
Download mbox | patch
Permalink /patch/1436/
State Accepted, archived
Headers show

Comments

Isaac Jurado - April 18, 2013, 7:56 p.m.
# HG changeset patch
# User Isaac Jurado <diptongo@gmail.com>
# Date 1366314160 -7200
#      Thu Apr 18 21:42:40 2013 +0200
# Node ID 380de75c43f4a186a1b8255393cf52c3af43820a
# Parent  7d31f2e42a8afb54c8fae87e8e3e29a63578aea4
dirstate: use repository root as base for relative paths read from ui.ignore

Convert relative paths from the ui.ignore* configurations to absolute by
prepending the path to the repository root.  This breaks the old, and probably
unused, behaviour where relative paths were interpreted from the working
directory.

Absolute paths are kept untouched.
Matt Mackall - April 18, 2013, 10:47 p.m.
On Thu, 2013-04-18 at 21:56 +0200, diptongo@gmail.com wrote:
> # HG changeset patch
> # User Isaac Jurado <diptongo@gmail.com>
> # Date 1366314160 -7200
> #      Thu Apr 18 21:42:40 2013 +0200
> # Node ID 380de75c43f4a186a1b8255393cf52c3af43820a
> # Parent  7d31f2e42a8afb54c8fae87e8e3e29a63578aea4
> dirstate: use repository root as base for relative paths read from ui.ignore
> 
> Convert relative paths from the ui.ignore* configurations to absolute by
> prepending the path to the repository root.  This breaks the old, and probably
> unused, behaviour where relative paths were interpreted from the working
> directory.
> 
> Absolute paths are kept untouched.

Seems safe enough. Queued for default with a (BC) marker.
Matt Mackall - April 18, 2013, 11:16 p.m.
On Thu, 2013-04-18 at 17:47 -0500, Matt Mackall wrote:
> On Thu, 2013-04-18 at 21:56 +0200, diptongo@gmail.com wrote:
> > # HG changeset patch
> > # User Isaac Jurado <diptongo@gmail.com>
> > # Date 1366314160 -7200
> > #      Thu Apr 18 21:42:40 2013 +0200
> > # Node ID 380de75c43f4a186a1b8255393cf52c3af43820a
> > # Parent  7d31f2e42a8afb54c8fae87e8e3e29a63578aea4
> > dirstate: use repository root as base for relative paths read from ui.ignore
> > 
> > Convert relative paths from the ui.ignore* configurations to absolute by
> > prepending the path to the repository root.  This breaks the old, and probably
> > unused, behaviour where relative paths were interpreted from the working
> > directory.
> > 
> > Absolute paths are kept untouched.
> 
> Seems safe enough. Queued for default with a (BC) marker.

Or maybe not. Something just destroyed my rsync-to-box-in-cloud-and-test
script, and this is the most likely culprit.
Isaac Jurado - April 19, 2013, 8:53 a.m.
On Fri, Apr 19, 2013 at 1:16 AM, Matt Mackall <mpm@selenic.com> wrote:
> On Thu, 2013-04-18 at 17:47 -0500, Matt Mackall wrote:
>> On Thu, 2013-04-18 at 21:56 +0200, diptongo@gmail.com wrote:
>> > # HG changeset patch
>> > # User Isaac Jurado <diptongo@gmail.com>
>> > # Date 1366314160 -7200
>> > #      Thu Apr 18 21:42:40 2013 +0200
>> > # Node ID 380de75c43f4a186a1b8255393cf52c3af43820a
>> > # Parent  7d31f2e42a8afb54c8fae87e8e3e29a63578aea4
>> > dirstate: use repository root as base for relative paths read from ui.ignore
>> >
>> > Convert relative paths from the ui.ignore* configurations to absolute by
>> > prepending the path to the repository root.  This breaks the old, and probably
>> > unused, behaviour where relative paths were interpreted from the working
>> > directory.
>> >
>> > Absolute paths are kept untouched.
>>
>> Seems safe enough. Queued for default with a (BC) marker.
>
> Or maybe not. Something just destroyed my
> rsync-to-box-in-cloud-and-test script, and this is the most likely
> culprit.

Ok, I'll resend on May.  Meanwhile, it could be useful to have more
details abut such failure, if possible.

Cheers.
Mads Kiilerich - April 19, 2013, 4:43 p.m.
On 04/18/2013 09:56 PM, diptongo@gmail.com wrote:
> # HG changeset patch
> # User Isaac Jurado <diptongo@gmail.com>
> # Date 1366314160 -7200
> #      Thu Apr 18 21:42:40 2013 +0200
> # Node ID 380de75c43f4a186a1b8255393cf52c3af43820a
> # Parent  7d31f2e42a8afb54c8fae87e8e3e29a63578aea4
> dirstate: use repository root as base for relative paths read from ui.ignore
>
> Convert relative paths from the ui.ignore* configurations to absolute by
> prepending the path to the repository root.  This breaks the old, and probably
> unused, behaviour where relative paths were interpreted from the working
> directory.

I agree that the current behaviour is useless, but think that would you 
describe would be surprising. I would expect a relative filename in a 
config file to be relative to the file it is specified in (or cwd if 
specified on the command line).

See also http://markmail.org/message/fcgvz5zij62triwk for a similar 
discussion and some unanswered questions you might want to pick up.

/Mads
Matt Mackall - April 21, 2013, 10:32 p.m.
On Fri, 2013-04-19 at 10:53 +0200, Isaac Jurado wrote:
> On Fri, Apr 19, 2013 at 1:16 AM, Matt Mackall <mpm@selenic.com> wrote:
> > On Thu, 2013-04-18 at 17:47 -0500, Matt Mackall wrote:
> >> On Thu, 2013-04-18 at 21:56 +0200, diptongo@gmail.com wrote:
> >> > # HG changeset patch
> >> > # User Isaac Jurado <diptongo@gmail.com>
> >> > # Date 1366314160 -7200
> >> > #      Thu Apr 18 21:42:40 2013 +0200
> >> > # Node ID 380de75c43f4a186a1b8255393cf52c3af43820a
> >> > # Parent  7d31f2e42a8afb54c8fae87e8e3e29a63578aea4
> >> > dirstate: use repository root as base for relative paths read from ui.ignore
> >> >
> >> > Convert relative paths from the ui.ignore* configurations to absolute by
> >> > prepending the path to the repository root.  This breaks the old, and probably
> >> > unused, behaviour where relative paths were interpreted from the working
> >> > directory.
> >> >
> >> > Absolute paths are kept untouched.
> >>
> >> Seems safe enough. Queued for default with a (BC) marker.
> >
> > Or maybe not. Something just destroyed my
> > rsync-to-box-in-cloud-and-test script, and this is the most likely
> > culprit.
> 
> Ok, I'll resend on May.  Meanwhile, it could be useful to have more
> details abut such failure, if possible.

Turns out this was probably the fault of the recent color extension bug
and nothing wrong with your patch.

Patch

diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -103,7 +103,11 @@ 
         files = [self._join('.hgignore')]
         for name, path in self._ui.configitems("ui"):
             if name == 'ignore' or name.startswith('ignore.'):
-                files.append(util.expandpath(path))
+                p = util.expandpath(path)
+                if os.path.isabs(p):
+                    files.append(p)
+                else:
+                    files.append(self._join(p))
         return ignore.ignore(self._root, files, self._ui.warn)
 
     @propertycache