[PATCH] subrepos: support adding files in git subrepos
Mathias De Maré
mathias.demare at gmail.com
Thu Feb 26 12:50:50 CST 2015
On Wed, Feb 25, 2015 at 2:46 AM, Matt Harbison <mharbison72 at gmail.com>
wrote:
> On Tue, 24 Feb 2015 02:49:49 -0500, Mathias De Maré <
> mathias.demare at gmail.com> wrote:
>
> # HG changeset patch
>> # User Mathias De Maré <mathias.demare at gmail.com>
>> # Date 1424764162 -3600
>> # Tue Feb 24 08:49:22 2015 +0100
>> # Node ID 391c33821c0399e40979ef2440289844f8e12756
>> # Parent ff5caa8dfd993680d9602ca6ebb14da9de10d5f4
>> subrepos: support adding files in git subrepos
>>
>> This support includes correct matching, so includes,
>> excludes and patterns are all supported.
>>
>> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
>> --- a/mercurial/subrepo.py
>> +++ b/mercurial/subrepo.py
>> @@ -1524,6 +1524,24 @@
>> return False
>> @annotatesubrepoerror
>> + def add(self, ui, match, prefix, explicitonly, **opts):
>> + rev = self._state[1]
>> + if match.files():
>> + files = match.files()
>> + elif match.always():
>> + self._gitcommand(["add", "--all"])
>> + return []
>> + else:
>> + (modified, added, removed,
>> + deleted, unknown, ignored, clean) = self.status(None)
>> + files = unknown
>> + if match:
>> + files = [f for f in files if match(f)]
>>
>
> Will match ever be None, since .files() was called on it first?
>
Turns out it won't, going back to commands::add(), it looks to me like this
is always created. I've removed the check (and like you say, it doesn't
make sense to have this check after calling .files() already).
>
> + for f in files:
>> + self._gitcommand(["add", f])
>> + return []
>> +
>> + @annotatesubrepoerror
>> def remove(self):
>> if self._gitmissing():
>> return
>>
>
> Do we need this _gitmissing() check for add() too? Don't forget to update
> mercurial/help/subrepos.txt.
>
We do indeed. I've also checked other commands and I have a separate patch
to fix those.
>
> I was surprised that the matcher has all of the files in it, but your
> tests seem to prove it does. Looking at the code, it looks like it goes
> cmdutil.add() -> wctx.walk(match) -> dirstate.walk(), which I guess picks
> up the (non-hg?) subrepo files. But that dirstate walk is called in a way
> to ignore the files in the parent repo's .hgignore. And since it doesn't
> know git's ignore file, will it end up adding files git wants to ignore if
> a pattern _other_ than an exact file is supplied, like a glob? (IDK if git
> will add ignored files if explicitly named like hg. Maybe it isn't an
> issue.)
>
cmdutil first does wctx.walk(match), but afterwards it goes over all of the
subrepos :-)
>
> The .gitignore issue you can probably solve by always calling
> git.status(), and only acting on the intersection of unknown and
> match.files(), unless a file is match.exact() (or match.always()). I'm not
> sure what to suggest about bypassing .hgignore, other than to walk again
> with the correct parameter? Maybe _always_ iterating over unknown from
> git.status() and calling the matcher on each entry? That may fix both
> cases, but opens the door to silently ignoring the error for adding an
> already tracked file. That may not be worth worrying about.
>
I've implemented this by checking using match.exact() for each file if I
should add '-f'.
I don't do this for 'match.always()', since Mercurial doesn't add ignored
files with a no-args 'hg add'. In fact, I don't treat 'always()' like a
special case, since the regular case allows me to add the "adding filename"
text in the output.
I think the only unhandled case is already tracked files, I decided to do
like you mentioned and not worry about it :-)
Greetings,
Mathias
>
> --Matt
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150226/2f60513c/attachment.html>
More information about the Mercurial-devel
mailing list