Re: [PATCH] tests: add AM_PROG_AR to help losing archivers

[Stefano Lattarini]

On 01/31/2012 09:30 PM, Peter Rosin wrote:
> Stefano Lattarini skrev 2012-01-31 14:31:
>> On 01/31/2012 01:55 PM, Peter Rosin wrote:
>>> Hi,
>>>
>>> Here's a couple of missing AM_PROG_AR lines, ok for maint?
>>>
>> Alas no, because msvc is *not* merged into maint, so that we don't even
>> have AM_PROG_AR in maint  :-(
> 
> Arrgh.  Good catch!
> 
>> You might instead want to merge maint into msvc, then apply this patch
>> to msvc, then merge msvc back into branch-1.11 and master.
>>
>> (I know, the present organization of branches sucks in some respects;
>> we might rethink it after the 1.11.3 release, OK?)
> 
> I would love to send maint, msvc and branch-1.11 to some dump
> somewhere far away with a one-way ticket.
>
Oh no!  maint and branch-1.11 are were we cut the 1.11.x releases from,
so until 1.12 is out, they are here to stay.  Sorry.

In retrospective, though, it should have been better to dispense with
branch-1.11 and simply use maint as base for the 1.11.x releases (that
would have allowed us to have the 'msvc' branch dropped at this point,
since it's already merged in the maintenance branch). The only potential
problem with that approach is: how do we deal with "divergent" changes in
maint and master then?  (As a concrete example of such a situation, think
of the 'extra-portability' warnings, that are enabled by '-Wall' in
master *but* not in the 1.11.x series).  Luckily, after some experience
with git, I think I have a solution for this problem.

If you need a divergent change for master ad maint (which is a very rare
occurrence anyway), proceed as follows:

  1. merge maint into master;
  2. create a temporary branch out of maint (say "tmp-branch");
  3. implement in 'tmp-branch' the behaviour you want for 'maint';
  4. merge 'tmp-branch' into 'maint';
  5. change the implementation in 'tmp-branch' to match the behaviour
     you want for 'master';
  5. merge 'tmp-branch' into 'master';
  6. merge 'maint' into 'master'; if we have done things right, this
     should cause *no* merge conflicts;
  7. If 'tmp-branch' is not meant to be a permanent topic branch,
     delete it.

I'll try this approach after 1.11.3, so that we move 'branch-1.11' to
be the new 'maint', keep it merged into 'master', and delete the (by
then obsolete) 'msvc' branch.  WDYT?

> They have simply diverged too much.  At least for me, I'm only somewhat up
> to speed on the ar-lib mess, but you might be in a different position having
> written most (all?) of the other diverging changes.
> 
>>> Subject: [PATCH] tests: add AM_PROG_AR to help losing archivers
>>>
>>> * tests/extradep.test (configure.in): Add AM_PROG_AR.
>>> * tests/extradep2.test (configure.in): Likewise.
>>>
>> By only reading this, I'm not sure whether the change is needed to pacify
>> some automake warning, to improve coverage, or to make the tests runnable
>> with Microsoft lib as the archiver.  Could you add a paragraph after the
>> summary line that explicitly specifies what is the patch motivation?
>>
>> ACK with that addressed.
> 
> Turns out these AM_PROG_ARs are already on master.  So, the merge from
> maint into msvc should probably include them with a manual adjustment.
> I tried doing the merges you describe above, but it's too rich for my
> stomach.  I get something that works, sort of, by I don't feel good
> about it and I have this feeling that some changes leaked into branch-1.11
> that are not supposed to be there.  I simply don't feel qualified and
> can't assess if my conflict resolutions are good or bad without further
> digging.  Which I don't have time for, so I'm leaving those merges for
> someone else<tm>.  Sorry.
>
Given the plan outlined above, I say you can just apply your changes
to branch-1.11.  I will handle the post-1.11.3 branches reorganization
myself, so you should not worry about "merging pains" :-)

> Regarding the requested extra paragraph in the commit message, is that
> really needed?
>
Not *really* needed for such a simple change -- but still highly
preferred.

> I think it would be quite a bit of extra work to get an
> accurate description of the various failure cases,
>
Oh, but I didn't ask for the accurate description of the failures --
only for their "kind".  Let me rephrase what I said:

 - "to improve coverage"
  This means no real failure; you only made the changes to ensure better
  coverage when you run the testsuite with MSVC.

 - "to make the tests runnable with Microsoft lib as the archiver"
  The tests failed when you run them with Microsoft lib being the
  archiver; you patch fixes this failure.  So the change is not just
  an improvement, but a real bug fix (albeit for a testsuite bug).

 - "the change is needed to pacify some automake warning"
  This means a failure happening at automake time (maybe only on MSYS),
  but even *without Microsoft lib involved*.  This too is a real bug
  fix (albeit again, for a testsuite bug).

> as I haven't kept the test suite results and would need to rerun the
> tests with/without patches and with varying compiler settings etc.
> It's a trivial bug, and as I said, the code is already there on master.
> 
Then it's acceptable to dispense with the "explanatory paragraph" in
this case.  Still, if in future you manage keep track of the details
of a bug before fixing it, and report such details in the commit
message, you'll make an happy man :-)

Thanks,
  Stefano