Re: 2.15.8 Regtests

[David Kastrup]

"Phil Holmes" <address@hidden> writes:

>> "Phil Holmes" <address@hidden> writes:
>>
>>> One major change from 2.15.7 - in nested-property-revert.ly.  I'm
>>> assuming this is down to David's reversion of an earlier commit.  I'm
>>> not expert enough to say what should happen here, although it doesn't
>>> look right to me.
>>
>> This "should not" is not achievable with the current data structures
>> without causing other regressions even harder to explain (which already
>> happened).  So I picked the less complex code and behavior as a starting
>> point for first fixing the data structures, then the code.
>>
>> The test case is quite artificial and, in my opinion, not worth fixing
>> temporarily at the cost of breaking more straightforward code.
>
> I think this needs to be added to the tracker, and would normally
> count as critical regression.  I'm tempted to mark it regression -
> high - since it's only there because of another critical that you
> fixed (IIRC) and you are working on it.  You OK with that?

I have a hard time counting the removal of a band aid for an artificial
test case with undefined behavior (try finding a place in the user
documentation that declares this kind of code as producing predictable
results) as a regression because the original code did not fix the
underlying problem, but merely masked it.

As a rather silly analogy: imagine that I put exit(0) right at the start
of the main program.  Somebody takes it out eventually, and I demand
that every segfault needs now to be labelled as a regression since
Lilypond did not segfault with my fix installed.

Bluntly: unmatched nested overrides/reverts never ever worked sensibly,
and actually their behavior never has been defined in the user
documentation either.  That we temporarily had a "fix" that was
hand-crafted to match the only test-case in our regression tests in
order to make just this test produce more intuitive results, does not
change that.

So I don't see the regression, and frankly, I don't see the high
importance as the feature could not be used predictably at any point of
time.  It is a rather new and, obviously, largely untested feature.  If
you tread carefully (and this test case doesn't), you are likely to get
what you expect.  There is a cheap workaround: don't revert a nested
property that you did not override in the same manner and order.

-- 
David Kastrup