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

login
register
mail settings
Submitter Matt Harbison
Date March 3, 2015, 3:43 a.m.
Message ID <24c36aaa69efe05e36a9.1425354221@Envy>
Download mbox | patch
Permalink /patch/7884/
State Accepted
Commit 932de135041fc944af1c3df21ff251522f8236b6
Headers show

Comments

Matt Harbison - March 3, 2015, 3:43 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@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

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.
Augie Fackler - March 3, 2015, 4:26 p.m.
On Mon, Mar 02, 2015 at 10:43:41PM -0500, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@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)
> +
>          # 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@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Mathias De Maré - March 3, 2015, 5:38 p.m.
On Tue, Mar 3, 2015 at 5:26 PM, Augie Fackler <raf@durin42.com> wrote:

> On Mon, Mar 02, 2015 at 10:43:41PM -0500, Matt Harbison wrote:
> > # HG changeset patch
> > # User Matt Harbison <matt_harbison@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@selenic.com
> > http://selenic.com/mailman/listinfo/mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Matt Harbison - March 4, 2015, 2:34 a.m.
On Tue, 03 Mar 2015 12:38:56 -0500, Mathias De Maré  
<mathias.demare@gmail.com> wrote:

> On Tue, Mar 3, 2015 at 5:26 PM, Augie Fackler <raf@durin42.com> wrote:
>
>> On Mon, Mar 02, 2015 at 10:43:41PM -0500, Matt Harbison wrote:
>> > # HG changeset patch
>> > # User Matt Harbison <matt_harbison@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).

--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@selenic.com
>> > http://selenic.com/mailman/listinfo/mercurial-devel
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@selenic.com
>> http://selenic.com/mailman/listinfo/mercurial-devel
Mathias De Maré - March 4, 2015, 6:14 a.m.
On Wed, Mar 4, 2015 at 3:34 AM, Matt Harbison <mharbison72@gmail.com> wrote:

> On Tue, 03 Mar 2015 12:38:56 -0500, Mathias De Maré <
> mathias.demare@gmail.com> wrote:
>
>  On Tue, Mar 3, 2015 at 5:26 PM, Augie Fackler <raf@durin42.com> wrote:
>>
>>  On Mon, Mar 02, 2015 at 10:43:41PM -0500, Matt Harbison wrote:
>>> > # HG changeset patch
>>> > # User Matt Harbison <matt_harbison@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@selenic.com
>>> > http://selenic.com/mailman/listinfo/mercurial-devel
>>> _______________________________________________
>>> Mercurial-devel mailing list
>>> Mercurial-devel@selenic.com
>>> http://selenic.com/mailman/listinfo/mercurial-devel
>>>
>>

Patch

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)
+
         # 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 ..