Bug 259005

Summary: Mk/Scripts/qa.sh: Spurious SONAME warnings due to pipefail
Product: Ports & Packages Reporter: Henrik Gulbrandsen <henrik>
Component: Ports FrameworkAssignee: Port Management Team <portmgr>
Status: Closed FIXED    
Severity: Affects Some People CC: ports-bugs, tatsuki_makino
Priority: --- Keywords: needs-qa, regression
Version: LatestFlags: koobs: merge-quarterly?
Hardware: Any   
OS: Any   
URL: https://reviews.freebsd.org/D31211
See Also: https://reviews.freebsd.org/D31211
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=167009
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=258972
Attachments:
Description Flags
qa_soname_pipefail.diff none

Description Henrik Gulbrandsen 2021-10-08 13:07:38 UTC 
Created attachment 228517 [details]
qa_soname_pipefail.diff

The recently added "set -o pipefail" at the top of Mk/Scripts/qa.sh
breaks the exit status for pipelines where long texts are piped into
"grep -q", as explained in the comments for PR 167009 and elaborated
on elsewhere ( https://reviews.freebsd.org/D31211 ).

This produces lots of incorrect SONAME warnings during stage-qa for
at least some ports and systems. I have been waiting for someone to
patch this, but since the problem still exists in the current tree,
it is probably best to offer my own patch, if only to have an open
bug report for the issue.

There are other instances of the "| grep -q" pattern in qa.sh, but it
looks like the remaining examples have single-line input texts, which
are unlikely to trigger this bug. I think we can keep those unchanged.
Comment 1 Tatsuki Makino 2021-10-09 20:16:49 UTC 
Good morning, I sent an email yesterday, but it already existed. :)

audio/audacity will step on this issue.
If we can ignore the cost of launching a grep process, I can only think of double grep.

readelf -d ${dep_file} | grep SONAME | grep -q SONAME
Comment 2 Tatsuki Makino 2021-10-10 00:48:00 UTC 
readelf -d /usr/local/lib/libwx_gtk3u_core-3.1.so.5.0.0 | grep -q SONAME
 is not reproduced every time for some reason.

sync ; readelf -d /usr/local/lib/libwx_gtk3u_core-3.1.so.5.0.0 | grep -q SONAME
 increases the rate of reproduction.

readelf -d /usr/local/lib/libwx_gtk3u_core-3.1.so.5.0.0 | stdbuf -i 0 grep -q SONAME
 seems to be one of the workarounds.
Comment 3 Kubilay Kocak freebsd_committer freebsd_triage 2021-10-12 00:14:51 UTC 
This appears to be the the same issue as bug 259005 ?
Comment 4 Kubilay Kocak freebsd_committer freebsd_triage 2021-10-12 00:15:09 UTC 
Ugh, I mean bug 258972
Comment 5 Tatsuki Makino 2021-10-12 01:07:42 UTC 
(In reply to Kubilay Kocak from comment #4)

Maybe not.
I'm on 12.2-STABLE and can't get bug 258972 to occur.

This is due to the fact that grep -q only reads stdin up to the line it matches.

And... stdbuf -i 0 is also not a workaround.
We may have to discard unnecessary output to /dev/null to get exit code 0.
readelf -d "${dep_file}" | grep SONAME > /dev/null
Comment 6 Henrik Gulbrandsen 2021-10-12 06:01:45 UTC 
Nah, bug 258972 is something else. It only occurs on 14-CURRENT, and the qa.sh script uses ldd for the library list, so if anything I would guess it's related to bug 259069. I don't have a 14-CURRENT system to test on, but the ldd output must have changed so it's incompatible with the qa.sh awk expression.

"... | grep SONAME > /dev/null" should work, but there is a risk that someone will come along and try to optimize it back to "... | grep -q SONAME", which is why I used "set +o pipefail" instead. It is a bit more self-documenting.
Comment 7 Baptiste Daroussin freebsd_committer freebsd_triage 2021-11-19 21:05:16 UTC 
I did test the stdbuf solution (I have a test which reproduce the issue all the time) and stdbuf does not help here
Comment 8 Baptiste Daroussin freebsd_committer freebsd_triage 2021-11-19 21:25:02 UTC 
Note the specific case has been fixed in the tree!

Thank you for reporting and proposing a patch!
Comment 9 Henrik Gulbrandsen 2021-11-20 08:46:28 UTC 
(In reply to Baptiste Daroussin from comment #8)

Maybe the specific case, but what about the general case? Aren't you worried about the second instance of "readelf -d ... | grep -q SONAME" in the script?
My patch handled both lines, but commit 6ff48ecff only patched the first one.