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

[Han-Wen Nienhuys]

On Mon, Nov 23, 2020 at 9:11 PM Jonas Hahnfeld <hahnjo@hahnjo.de> wrote:

> Am Sonntag, den 22.11.2020, 22:49 +0100 schrieb David Kastrup:
> > Jonas Hahnfeld <hahnjo@hahnjo.de> writes:
> >
> > > Nope, that was a red herring: The reason is that the footnote creation
> > > process in footnote-volta-spanner.ly messes with (point-stencil),
> which
> > > is used by skyline-point-extent.ly and also the BendSpanner. As far as
> > > I understand the Stencil class, its objects are not immutable...
> > >
> > > One of the following two changes fixes the problem in my local test
> > > scenario:
> > >
> > > [...]
> > >
> > > My gut feeling is that we should fix null-markup because nothing should
> > > ever translate / rotate / modify the global point-stencil, thoughts?
> >
> > To me that seems crazy.  Instead \footnote should not modify the stencil
> > generated from its argument in place.  If it needs something with
> > different dimensions, it needs to create a new stencil, copying the
> > stencil expression and changing the dimensions.
>
> Yes indeed, this seems to be the contract for stencils: Make a copy
> whenever a Stencil is passed in from Scheme. I managed to find the code
> path for Balloon grobs that violate this rule, here's a fix:
> https://gitlab.com/lilypond/lilypond/-/merge_requests/522
> (Note that the test case shows that this was already broken before this
> unstable series, at least 2.20.0 but probably older versions too).
>

thanks for tracking this down!

point-stencil is from 2004, and the balloon code from thereabouts too. It's
surprising that this took so long to turn up.


> If possible, I'd like to merge this rather sooner than later so I can
> finally post the MR to enable 'make check'..
>

fine to skip the countdown for me.



> > ly:stencil-set-extent! has been removed in 2.5.17.  Extents are not
> > intended to be mutable as far as I can discern, so if our C++ code does
> > violate that assumption, it is that code we should fix instead of giving
> > up on behavior guaranteed from the Scheme side of things.
>
> In fact, the C++ code does and many methods of the Stencil class modify
> the object, but this is ok as long as we stick to the rule above. I'd
> be happy to assert in some way that stencil representations coming from
> Scheme are not altered, but I can't think of a good way to do this...
>

We'd have to make unsmob<Stencil> return a Stencil const *.  Maybe that is
appropriate for more simple smobs?

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