|Summary:||assert update to handle pipe's|
|Product:||Portage Development||Reporter:||SpanKY <vapier>|
|Component:||Core||Assignee:||Portage team <dev-portage>|
|Package list:||Runtime testing required:||---|
Description SpanKY 2003-08-04 20:17:31 UTC
currently unpack() and other places are broken due to piping the output of programs into tar the assert function needs to be updated to utilize the PIPESTATUS variable aron has put together the required changes to assert ... the next step would be to filter the updates into unpack() and any other affected packages (quickpkg also comes to mind as does emerge-websync)
Comment 1 SpanKY 2003-08-04 20:19:23 UTC
Created attachment 15520 [details, diff] ebuild.sh.patch adds PIPESTATUS support to assert ... also nice to point out that this will (or rather should not) break any package that already uses assert ... assert before checked the return of the last program in the pipe ... this just makes assert check the returns of all programs in the pipe change (i.e. extending the algo from 1 to 1...N piped programs)
Comment 2 Aron Griffis (RETIRED) 2003-08-04 20:23:20 UTC
fyi, here is what I did to verify that no ebuilds will be negatively affected by this change: find . -name \*.ebuild | xargs perl -0777 -ne '$ARGV=~s,.*/,,; s/[\\]\n/ /g; s/\|\n/| /g; /^.*\|.*\n.*\bassert.*/m || next; print "$ARGV: $&\n"' This shows all ebuilds that appear to have a pipeline preceding a call to assert. If you run the above command, about 5 appear, all of which are false positives. The result is that we can apply this patch ASAP without hurting anything, and providing some really nice functionality.
Comment 3 Nicholas Jones (RETIRED) 2003-08-06 01:45:08 UTC
What is this fixing exactly?
Comment 4 SpanKY 2003-08-06 04:55:13 UTC
well, i laid out the bug on the -dev e-mail list ... i dont have it anymore because i cleaned out the list ... maybe someone else can post it ...
Comment 5 Aron Griffis (RETIRED) 2003-08-06 05:44:39 UTC
Nick, it solves a problem that has been present in ebuilds for a long time. Namely, the following command will appear to work just fine: false | false | false | true || die Only the last item in the pipeline is checked for status. The change that Mike and I are proposing is that "assert" will check the status of all the items in the pipeline. For Mike, I think this pertains to a specific bug. For myself, this is part of a general project to make ebuilds catch more errors. I've seen a number of ebuilds lately that don't check properly for errors, and the result is that compile-time errors become run-time errors because the ebuild doesn't abort. As Mr. Bones is famous for saying, ebuilds should check status on every single disk write, otherwise we don't know if it failed due to a transient problem like a full disk. Does all this make sense?
Comment 6 Nicholas Jones (RETIRED) 2003-08-06 14:45:38 UTC
Yes. Added for 49_pre18