# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 2015-02-28 04:30:42
# Node ID 932de135041fc944af1c3df21ff251522f8236b6
# Parent  00ef3edcf1d5d0b1bb7215e785460769f48acfe8

subrepo: warn when adding already tracked files in gitsubrepo

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 @@ class gitsubrepo(abstractsubrepo):
         (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)
+
         # 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 @@ class gitsubrepo(abstractsubrepo):
                 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 @@ correctly do a dry run
   ? 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 ..