[PATCH 2 of 2] subrepo: warn when adding already tracked files in gitsubrepo
Mathias De Maré
mathias.demare at gmail.com
Wed Mar 4 00:14:04 CST 2015
On Wed, Mar 4, 2015 at 3:34 AM, Matt Harbison <mharbison72 at gmail.com> wrote:
> On Tue, 03 Mar 2015 12:38:56 -0500, Mathias De Maré <
> mathias.demare at gmail.com> wrote:
>
> On Tue, Mar 3, 2015 at 5:26 PM, Augie Fackler <raf at durin42.com> wrote:
>>
>> On Mon, Mar 02, 2015 at 10:43:41PM -0500, Matt Harbison wrote:
>>> > # HG changeset patch
>>> > # User Matt Harbison <matt_harbison at yahoo.com>
>>> > # Date 1425097842 18000
>>> > # Fri Feb 27 23:30:42 2015 -0500
>>> > # Node ID 24c36aaa69efe05e36a94191d2ca71a96b8c299e
>>> > # Parent 72f885c8b7aff110a147cc7ff12f5951bf5aaf03
>>> > subrepo: warn when adding already tracked files in gitsubrepo
>>>
>>> queued these, thanks
>>>
>>> >
>>> > This follows normal Mercurial rules, and the message is lifted from
>>> > workingctx.add(). The file is printed with abs() to be consistent with
>>> how it
>>> > is printed in workingctx, even though that is inconsistent with how
>>> added files
>>> > are printed in verbose mode. Further, the 'already tracked'
>>> notifications come
>>> > after all of the files that are added are printed, like in Mercurial.
>>> >
>>> > As a side effect, we now have the reject list to return to the caller,
>>> so that
>>> > 'hg add' exits with the proper code. It looks like an abort occurs if
>>> git fails
>>> > to add the file. Prior to touching 'snake.python' in the test, this
>>> was
>>> the
>>> > result of attempting to add the file after a 'git rm':
>>> >
>>> > fatal: pathspec 'snake.python' did not match any files
>>> > abort: git add error 128 in s (in subrepo s)
>>> >
>>> > I'm not sure what happens when git is a deep subrepo, but the 'in s'
>>> and
>>> > 'in subrepo s' from @annotatesubrepoerror are redundant here. Maybe we
>>> should
>>> > stat the files before invoking git to catch this case and print out the
>>> prettier
>>> > hg message? The other thing missing from workingctx.add() is the call
>>> to
>>> > scmutil.checkportable(), but that would need to borrow the parent's ui
>>> object.
>>> >
>>> > diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
>>> > --- a/mercurial/subrepo.py
>>> > +++ b/mercurial/subrepo.py
>>> > @@ -1530,10 +1530,17 @@
>>> > (modified, added, removed,
>>> > deleted, unknown, ignored, clean) = self.status(None)
>>> >
>>> > + tracked = set()
>>> > + # dirstates 'amn' warn, 'r' is added again
>>> > + for l in (modified, added, deleted, clean):
>>> > + tracked.update(l)
>>>
>>> Note that this will not work for clean files right now. That's not
>> really a
>> major issue, I think.
>> Additionally, to get this to work for clean files, 'self.status(None)'
>> will
>> have to be changed to 'self.status(None, clean=True)'.
>>
>
> Good catch, I missed that. I'll follow up with this change, so the caller
> is correct.
>
> Unfortunately that won't change anything, because it looks like deleted,
> ignored and clean are never populated. It also looks like whether or not
> ignored, clean and unknown are fetched is not controlled by the
> corresponding parameter. I just submitted a patch for unknown.
>
> Is this something you can follow up on when you get a chance? You will
> need clean if you want to implement file removal (so you can warn about
> dirty files).
>
It's on my TODO list, I plan to follow up on this (at least for clean).
Thanks for the reminder :-)
Greetings,
Mathias
>
> --Matt
>
>
> Other than that, this patch looks good. Nice additional improvement to
>> add!
>>
>> Greetings,
>> Mathias
>>
>> > +
>>> > # Unknown files not of interest will be rejected by the
>>> matcher
>>> > files = unknown
>>> > files.extend(match.files())
>>> >
>>> > + rejected = []
>>> > +
>>> > files = [f for f in sorted(set(files)) if match(f)]
>>> > for f in files:
>>> > exact = match.exact(f)
>>> > @@ -1542,9 +1549,18 @@
>>> > command.append("-f") #should be added, even if ignored
>>> > if ui.verbose or not exact:
>>> > ui.status(_('adding %s\n') % match.rel(f))
>>> > +
>>> > + if f in tracked: # hg prints 'adding' even if already
>>> tracked
>>> > + if exact:
>>> > + rejected.append(f)
>>> > + continue
>>> > if not opts.get('dry_run'):
>>> > self._gitcommand(command + [f])
>>> > - return []
>>> > +
>>> > + for f in rejected:
>>> > + ui.warn(_("%s already tracked!\n") % match.abs(f))
>>> > +
>>> > + return rejected
>>> >
>>> > @annotatesubrepoerror
>>> > def remove(self):
>>> > diff --git a/tests/test-subrepo-git.t b/tests/test-subrepo-git.t
>>> > --- a/tests/test-subrepo-git.t
>>> > +++ b/tests/test-subrepo-git.t
>>> > @@ -974,7 +974,26 @@
>>> > ? s/cpp.cpp
>>> > ? s/foobar.orig
>>> >
>>> > -currently no error given when adding an already tracked file
>>> > +error given when adding an already tracked file
>>> > $ hg add s/.gitignore
>>> > + s/.gitignore already tracked!
>>> > + [1]
>>> > +
>>> > +removed files can be re-added
>>> > + $ hg ci --subrepos -m 'snake'
>>> > + committing subrepository s
>>> > + $ cd s
>>> > + $ git rm snake.python
>>> > + rm 'snake.python'
>>> > + $ touch snake.python
>>> > + $ cd ..
>>> > + $ hg add s/snake.python
>>> > + $ hg status -S
>>> > + M s/snake.python
>>> > + ? .hgignore
>>> > + ? s/barfoo
>>> > + ? s/c.c
>>> > + ? s/cpp.cpp
>>> > + ? s/foobar.orig
>>> >
>>> > $ cd ..
>>> > _______________________________________________
>>> > Mercurial-devel mailing list
>>> > Mercurial-devel at selenic.com
>>> > http://selenic.com/mailman/listinfo/mercurial-devel
>>> _______________________________________________
>>> Mercurial-devel mailing list
>>> Mercurial-devel at selenic.com
>>> http://selenic.com/mailman/listinfo/mercurial-devel
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150304/e2784dfb/attachment.html>
More information about the Mercurial-devel
mailing list