[Mesa-dev] [PATCH] mesa/main: fixup make check after NV_conditional_render for gles

[Ilia Mirkin]

On Fri, Nov 9, 2018 at 11:43 AM Erik Faye-Lund
<erik.faye-lund at collabora.com> wrote:
>
> On Fri, 2018-11-09 at 11:17 -0500, Ilia Mirkin wrote:
> > On Fri, Nov 9, 2018 at 10:51 AM Erik Faye-Lund
> > <erik.faye-lund at collabora.com> wrote:
> > > On Fri, 2018-11-09 at 10:44 -0500, Ilia Mirkin wrote:
> > > > Errr ... why are these being added to the ABI now? Extensions are
> > > > generally not added in. Some are grandfathered in, but this one
> > > > would
> > > > be new.
> > > >
> > > > Or is this due to the fact that it's grandfathered in for GL, and
> > > > making that and GLES different would be tricky?
> > >
> > > I'm not sure, really. This feature missed the 18.3 release anyway,
> > > so
> > > we have time to fix this up, if anyone knows for sure.
> > >
> > > I don't think there's a good reason to cement this in the GLES ABI,
> > > so
> > > I think we should remove it, if that's possible. I don't really
> > > know
> > > what makes this appear in the ABI, though. If someone can explain,
> > > I'd
> > > be happy to fix things up.
> > >
> > > Anyway, I pushed the test-fixes for now; the ABI change seems to
> > > already have happened.
> >
> > That cements it as an OK thing to do.
>
> Please explain.
>
> > When you push a buggy change,
> > the solution isn't "make the test ignore the bug". The solution is to
> > either fix the bug or revert the offending change.
>
> Yes, and I thought I fixed the bug. I didn't see your message until
> *after* I pushed the fixup commit. I just thought I could fix this up

Ah OK. I wasn't checking the actual commits, just looking at email.

> further after the weekend, because I have dinner on the stove and
> guests about to arrive. Surely applications won't magically start
> depending on this symbol being visible until then.

Seems unlikely. And even the revert could have easily waited a day or
two. The problem happens when after a day or two you forget all about
it -- there's no indication of any failure, etc, so it's easy to let
it slip through. And now we've effectively expanded our ABI by
accident, and will never remember to fix it.

>
> Anyway, I've reverted both commits, and I'll retry this whole thing
> next week.

Thanks!

  -ilia