Re: Getting rid of the msvc branch

[Stefano Lattarini]

On 03/07/2012 08:32 PM, Peter Rosin wrote:
> Stefano Lattarini skrev 2012-03-07 16:39:
>> On 03/07/2012 04:00 PM, Peter Rosin wrote:
>>> Peter Rosin skrev 2012-03-07 14:08:
>>>> Do you think it would be at all possible to start with a msvc and maint
>>>> that is freshly merged into both master and branch-1.11.  Then merge msvc
>>>> into maint in such a way that maint resembles branch-1.11, then do dummy
>>>> merges of maint into master and branch-1.11.  I.e. for the master case:
>>>>     git checkout master; git merge --strategy=ours maint
>>>> and similar for branch-1.11.  And then, finally, get rid of the obnoxious
>>>> msvc branch...
>>>>
>>>> After that, we should be able to go back to the old simple rule of only
>>>> updating the scripts on maint.
>>>
>>> I tried, and it looks good, methinks.
>>>
>> To me as well, but with an important nit below.
>>
>>> I did this:
>>>
>>> git checkout maint
>>> git merge --no-ff msvc
>>> git diff maint branch-1.11 HACKING | patch -p1
>>> git diff maint branch-1.11 lib/Automake/ChannelDefs.pm | patch -p1
>>> git diff maint branch-1.11 tests/ar-lib3.test | patch -p1
>>> git diff maint branch-1.11 tests/ar-lib4.test | patch -p1
>>> git diff maint branch-1.11 tests/extra-portability.test | patch -p1
>>> git diff maint branch-1.11 tests/extra-portability2.test | patch -p1
>>> git diff maint branch-1.11 tests/extradep.test | patch -p1
>>> git diff maint branch-1.11 tests/extradep2.test | patch -p1
>>> git diff maint branch-1.11 tests/list-of-tests.mk | patch -p1
>>> git add HACKING
>>> git add lib/Automake/ChannelDefs.pm
>>> git add tests/ar-lib3.test
>>> git add tests/ar-lib4.test
>>> git add tests/extra-portability.test
>>> git add tests/extra-portability2.test
>>> git add tests/extradep.test
>>> git add tests/extradep2.test
>>> git add tests/list-of-tests.mk
>>> git commit --amend -C HEAD
>>>
>> This commit message won't be nearly good enough, unfortunately.  I think 
>> that,
>> for the sake of the future readers of the history, you should give a detailed
>> and motivated explanation of why this merge is needed.  Hopefully, you might
>> be able to condense it from our past (and present) discussion.  In 
>> particular,
>> you should explain:
>>
>>   - why the msvc merged into branch-1.11 and the one merged into master have
>>     diverged (that issue about backward-compatibility for new warnings);
>>
>>   - that the decision to merge msvc into branch-1.11, rather than into maint,
>>     has been a bad call (in hindsight);
>>
>>   - that the merge you're doing, while complicating the history, will help
>>     us to simplify our workflow and get rid of a messy situation that is
>>     truly hindering development;
>>
>>   - any other pertinent rationale, fact and/or explanation you can think of.
>>
>> Could you try to write such a commit message, and post if for review before
>> pushing?  Otherwise, I'll try to do so myself -- but I strongly believe that
>> having such a "merge message", either from you or from me, is a necessary
>> prerequisite for pushing these changes.
> 
> Yes, that commit message was a whee bit sketchy :-)
> 
> How about replacing "git commit --amend -C HEAD" with the below?
> 
> Cheers,
> Peter
> 
> cat <<EOF | git commit --amend --file=-
> Merge branch 'msvc' into maint
> 
> This merge remedies the confusing situation that some changes
> destined for both the master branch and the release branch, a.k.a.
> branch-1.11,
>
Having parentheses instead of commas would make the sentence reads
better IMHO:

  destined for both the master branch and the release branch (a.k.a.
  branch-1.11)

WDYT?

> currently needs to be made on the non-obvious msvc
> branch instead of on the more natural maint branch.
>
"instead of on" sounds a little forced to me (but I'm not a native, so
this might be a wrong impression); what about s/instead of/rather than/?

> This has caused a seemingly endless string of less that optimal
> commits.
> 
> The reason for the confusion stems from the fact that the changes made
> on the msvc branch became too radical and was considered only suitable
> for the master branch, and was thus written in a form suitable for
> master and then merged there.  Later, the msvc branch was merged
> directly into branch-1.11, in order to rush the new features to the
> market and to keep the released scripts (lib/ar-lib, lib/compile and
> lib/depcomp) consistent with those on the master branch.  However,
> some changes had to be made to the features added by the msvc branch
> in order for them to fit the requirements of branch-1.11, notably that
> the warnings issued in the extra-portability class cannot be enabled
> by -Wall in the 1.11.x maintenance releases.
> 
> In retrospect, it would have been better to not merge msvc directly
> into branch-1.11, but instead do it via the maint branch (followed up
> with a dummy merge from maint into master) the moment it was decided
> that the msvc changes should make it into branch-1.11.
> 
> All in all, this merge is not going to affect neither the master
>
s/ not// (but I see in your follow-up you've already caught and fixed
that -- thanks!)

> branch nor branch-1.11, since it is followed up with dummy merges
> masking all changes.  The merge is made to maintain the sanity of the
> poor developers, who wishes to once again have a working maint branch.
> 
> Discussion about merging the msvc branch into branch-1.11:
> http://lists.gnu.org/archive/html/automake/2011-10/msg00030.html
> 
> Discussion about why this merge hasn't happened before:
> http://lists.gnu.org/archive/html/automake/2011-12/msg00051.html
> 
> Extra edits below.
> 
> * lib/Automake/ChannelDefs.pm: Use the version from branch-1.11.
> * tests/ar-lib3.test: Likewise.
> * tests/ar-lib4.test: Likewise.
> * tests/extra-portability.test: Likewise.
> * tests/extra-portability2.test: Likewise.
> * tests/extradep.test: Likewise.
> * tests/extradep2.test: Likewise.
> * tests/list-of-tests.mk: Likewise.
> * HACKING: Backport the version from branch-1.11 while at it (as the
> change on branch-1.11 is also present on master via an unrelated
> commit), even though this change has nothing to do with the changes
> on the msvc branch.
> EOF

Thanks for your thoroughness and patience.

Regards,
  Stefano