Philip Hands @ 2020-09-21 (Monday), 22:38 (+0200)
BTW The example I pasted was just busybox running on my laptop running full Debian, so was not supposed to be demonstrating it working under d-i.
I could likely have been more precise from the beginning about the exact cause. Sorry for making you required to ask for clarification.
This mixes the stdout of wget with the %OK% %EOF% stuff, and then puts it all into the sed, which seems flawed.
And that is why you're a Debian developer and I'm merely a user. I agree.
I'd have thought something like this would do the trick (not tested yet): local RETVAL=$( { { wget "$@" 2>&1 >/dev/null && echo 0 >&3 } | { grep -q "$file_not_found_pattern" && echo 4 >&3 } || echo 1 >&3 } 3>&1 | head -1 )
Looks good to me. As in, this code I can actually follow and understand. Which was a bit of a stretch for me to claim about for the sed construct.
I don't really understand why the tail would be needed, but am not objecting to it.
Replacing the RETVAL assignment expression as above works for me. Both from debian-installer and when running this simple test script in a shell:
#!/bin/sh fetch-url http://pxeserver./preseed/base.txt base.txt >/dev/null [ $? = 0 ] || exit echo "Existing file downloaded" fetch-url http://pxeserver./non-existant.txt non-existant.txt >/dev/null [ $? = 4 ] || exit echo "Non-existing file returned 404" fetch-url http://invalid-host./whatever.txt invalid-host.txt >/dev/null [ $? = 1 ] || exit echo "Unresolvable host return general error"
One could of course use stderr for that (>&2 ... 2>&1) for getting the echo-ed return codes out, rather than fd #3, but I think this is clearer (since one really isn't trying to produce stderr output) and AFAIK it should work fine even if /dev/fd/ is missing.
I agree. Nicely done!
I suspect there may be a way of getting this to work without the need for a local variable and the echoing for the result codes, so I will ponder on that...
Maybe it is, and if so it might include early return statements and thus save spawning grep on successful fetches. I reckon you could be happy and proud with the code you've posted already though.
BTW I just saw Ben's comment that we should just fix the missing /dev/fd which strikes me as entirely sensible.
It's entirely sensible to also attempt fixing the root cause, but my opinion is that it wouldn't hurt to treat missing fd-nodes and an overcomplex wget404() as two separate issues. Please feel free to fork the bug report.
One could also envision adding a test case for detecting missing fd:s within some test suite for d-i. Are there any tests run on nightly builds, which could help catching regressions like these?
-- /Martin