[PATCH 2 of 2] subrepo: warn when adding already tracked files in gitsubrepo

[Mathias De Maré]

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)'.

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/20150303/1f77732a/attachment.html>