Ubuntu
grep package

  1. Bug #1940999
  2. Comment #5

Comment 5 for bug 1940999

Revision history for this message
Robie Basak (racb) wrote :

Thank you for working on this!

> 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?

> + * 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.

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. 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.

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.

> +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 :)