[PATCH 0 of 4 stable] fix diff/status -S for added/removed subrepos (issue3056)

[Dov Feldstern]

On Fri, Apr 27, 2012 at 21:54, Matt Mackall <mpm at selenic.com> wrote:
> On Wed, 2012-04-25 at 13:02 +0300, Dov Feldstern wrote:
>> In trying to generate a patch containing all the changes needed for
>> bringing a non-mercurial copy of a repo from state 1 to state 2, I ran
>> into the issue where the generated patch did not include any of the
>> files from a subrepo which had been added between state 1 and state 2.
>
> This may be the right thing, but it's too ambitious / too much to think
> about this close to the release. Please resend this early in the 2.3
> cycle and hopefully we can get some opinions and review comments from
> subrepo users.
>
> Also, please include a minimal example of what's being changed: it's
> taken me about 10 minutes to page in enough context to get a handle on
> what's being proposed by reading through bug reports and list threads
> and I'll probably have paged that all back out by the time you resubmit
> next week. Saying "I did X and got Y, but it should be Z" gives the
> reviewer a framework to hang the rest of your description on.
>
> Regarding patch 4: tests should be updated in the changeset that breaks
> them, otherwise tracking down breakage in a stack of changes gets quite
> tricky.
>

Will do, thanks!

>> Searching the bts, I found that this is issue 3056 [1]. That issue
>> seems to have culminated in a thread [2] in which everyone agrees that
>> the issue should be fixed, but that's where things were left.
>>
>> So here are a few patches which solve the issue.
>>
>> I've added some tests for this to test-subrepo.t. I've also fixed some
>> existing tests which inadvertently display this issue; note, though,
>> that not all cases covered by my new tests are covered by the existing
>> ones. If reducing the number of new tests is an issue, I would suggest
>> perhaps removing the existing tests for issue3153 [3] (which is
>> actually a subset of issue3056). All other tests pass, except for
>> test-gendoc.t, which currently fails even before my patches.
>>
>> Finally, in reviewing these patches, I sort of feel that with a little
>> more refactoring --- something along the lines of having itersubrepos
>> also return the state of each subrepo in ctx2 --- things would be much
>> simpler from the caller's point of view; however, I'm not sure how
>> clean that is (it starts becoming hard to explain "what does
>> itersubrepos (and the more internal functions it calls) actually
>> do?"); and I wanted to get some feedback before venturing into more
>> serious refactoring.
>>
>> Comments welcome.
>>
>> [1] http://mercurial.selenic.com/bts/issue3056
>> [2] http://www.selenic.com/pipermail/mercurial-devel/2011-October/035194.html
>> [3] http://mercurial.selenic.com/bts/issue3153
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel at selenic.com
>> http://selenic.com/mailman/listinfo/mercurial-devel
>
>
> --
> Mathematics is the supreme nostalgia of our time.
>
>