Re: [RFC] Automatic 'make check' in CI

[Han-Wen Nienhuys]

On Sun, Nov 22, 2020 at 1:55 PM Jonas Hahnfeld <hahnjo@hahnjo.de> wrote:

> Am Samstag, den 21.11.2020, 18:50 +0100 schrieb Jonas Hahnfeld:
> > Am Samstag, den 21.11.2020, 09:46 +0100 schrieb Jonas Hahnfeld:
> > > Am Freitag, den 20.11.2020, 17:55 -0500 schrieb Dan Eble:
> > > > On Nov 20, 2020, at 12:22, Jonas Hahnfeld <hahnjo@hahnjo.de> wrote:
> > > > > > > Despite these shortcomings, I think it would make sense to
> enable
> > > > > > > this first implementation in lilypond/lilypond. What do you
> think?
> > > > > > >
> > > > > >
> > > > > > yes, +1 . I am especially happy that this will likely have no
> extra
> > > > > > overhead.
> > > > >
> > > > > No extra overhead per pipeline in MRs, but you get an extra
> pipeline
> > > > > for every merge to master. We'll see if this poses a serious
> problem…
> > > >
> > > > Given the positive feedback this change has already received, can we
> > > > accelerate its progress?  I'm eager to have it in place because I've
> > > > got a branch with new some regression tests ready.
> > >
> > > Sure, I can merge !517 later today and then work on the second MR to
> > > actually enable 'make check' in the pipelines.
> >
> > Something's not working correctly here:
> >
> https://hahnjo.gitlab.io/-/lilypond/-/jobs/864846942/artifacts/test-results/index.html
> > The test-baseline was generated by frog (using parallelism), check
> > executed on a shared runner. I thought I had tested using a test-
> > baseline with different number of cores...
>
> What I probably checked is various values for CPU_COUNT, including just
> one core, and all works correctly AFAICT. What gives differences is not
> setting CPU_COUNT / job-count at all.
>
> I successfully traced this back to compiling, for example, footnote-
> volta-spanner.ly and skyline-point-extent.ly (in that order!) with an
> added '\include "lilypond-book-preamble.ly"' (attached). If I run
>  $ ./out/bin/lilypond footnote-volta-spanner.ly skyline-point-extent.ly
> the skyline-point-extent.pdf has some added whitespace that vanishes
> when passing -djob-count=1 or 2 (starting with 3, it won't fork because
> there are less input files than jobs).
>
> footnote-volta-spanner.ly was added by
> commit c1f44faf958aafe7802f432a5ae0f4c74a8c0146
> Author: Han-Wen Nienhuys <hanwenn@gmail.com>
> Date:   Tue Jun 16 08:59:35 2020 +0200
>
>     For footnotes on spanners, copy bounds from underlying spanner.
>
>     This is likely more correct for spanners that run from non-musical
>     columns (eg. volta brackets) or are created retroactively
>     (eg. automatic beams).
>
>     Demonstrate this by adding a regression test that puts a footnote on a
>     VoltaBracket.
>
>     Without adjustments of Footnote_engraver, the test produces the
> following:
>
>       programming error: Spanner `FootnoteSpanner' is not fully contained
> in parent spanner.  Ignoring orphaned part
>       continuing, cross fingers
>       programming error: bounds of this piece aren't breakable.
>       continuing, cross fingers
>
> (https://gitlab.com/lilypond/lilypond/-/merge_requests/138). During
> review, David had some concerns about Spanners that were dismissed on
> the basis that there was no test case that showed wrong results (is
> that really our standard for claiming that a change works correctly?!).
>
> I tried reverting that commit and all other differences also went away.
> Does this mean the change is indeed flawed?
>

I'll try to have a look this week (likely Friday). It may be prudent to
revert the change if 2.22 is imminent.

Any ideas, comments?
>

Let's not reverse the arg list after fork, to avoid -djob-count confusing
things more.

I'd like to get rid of the fork call -that would also solve the problem
with the options regtest. Let me try to have another look at !204. I am not
too optimistic, but it seems worth trying another time.


-- 
Han-Wen Nienhuys - hanwenn@gmail.com - http://www.xs4all.nl/~hanwen