On Wed, Aug 25, 2021 at 12:00:44PM -0000, Robie Basak wrote:
> Thank you for working on this!

And thank you for the thoughtful review.

> > To address this I propose picking up the new version of grep.
> But you're picking up the new version of grep and then also disabling
> the failing test in that update. So why is the new version of grep
> necessary at all? Can't you just disable the failing test in the current
> version of grep in Impish?

I have been unclear.  With grep 3.6 + glibc 2.34, the failing test is a
different test - the 'stack-overflow' test fails with the following log
output:

  stack-overflow: failed test: grep never printed "stack overflow"
  FAIL: stack-overflow

More verbose log attached.

So my general plan here is to add grep 3.7 to pick up the new version,
which addresses the 'stack-overflow' test, but does regress on
'test-regex', which I then disable.  There's another option here, to use
the built-in regex for grep, but I presumed that would be generally
undesired.

> > +  * New upstream version 3.7 (LP: #1940999)
> > +  * Add d/p/06-disable-test-regex.patch to disable failing test-regex
> This bug is entitled "FTBFS against glibc 2.34" so the bug reference
> should go against the second item there, not the first. Placing the
> reference against the first entry makes it sound like this bug is
> tracking the update of grep to 3.7, but according to the bug title as it
> is at the moment, that isn't what this bug is tracking.

With the new information above, I think the way I have structured it
matches the problem I'm trying to address - what do you think?

> Please avoid refreshing quilt patches unnecessarily. Refreshing is only
> needed for conflicts and if fuzz appears. Otherwise, not refreshing them
> minimises diff noise and thus helps with review.

Sounds reasonable to me.  I'll fix it.

> Additionally, if forced
> to refresh I always use QUILT_REFRESH_ARGS="-p ab --no-timestamps --no-
> index" as documented at https://wiki.debian.org/UsingQuilt to reduce the
> noise for next time.

Right, I used to hide the suggested quilt config behind a dquilt alias
but the use case for me doing that in the first place is no longer
relevant.  An easy fix.

> It looks like fundamentally you're resolving the FTBFS by disabling the
> failing test. But have you checked that the test is actually a false
> positive? What if the test is working and picking up something that is
> actually wrong with the new glibc? If you think it's a false positive
> then I'd expect to see some explanation that it's a false positive and
> why you think that, but I couldn't find anything. And if it is a false
> positive, then a bug report to the test's origin (presumably upstream)
> and a dep3 header linking to that would be appropriate.

I believe the issue with the stack-overflow test is consistent with this
description from upstream:
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=46227
Admittedly I have not analyzed further other than comparing the verbose
log output I have to that and concluding that it was the same problem.
I will dig further if you believe it worthwhile.

> > +Forwarded:   no - Debian not yet on glibc 2.34
> It's still useful to forward, as Debian is expected to need that anyway.
> I usually file such bugs in the Debian BTS explaining that it isn't
> necessary to apply right now but will be useful in the future. However,
> before forwarding, maybe we should get the fix right here first :)

OK, when we have agreement on the fix I will forward.

> > +Last-Update: Aug-24-2021
> This should use ISO format according to the spec please
> (https://dep.debian.net/deps/dep3/)

Will do.

-Dan