Bug#970678: Network preseeding using http is broken
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
Reply to: