Arch Linux

Damn this bug is old (I missed lots of bugs last fall, sorry for that).

I'm looking at the code now. Is this error critical? From what I see, the scripts should run anyway. This can probably be solved by some reordering of commands, so stty isn't called on a non-terminal.

Don't think it's terribly important, but it does pollute cron logs pretty badly. I think the `isatty` tool would work for this check, but no repo package seems to have it, even though man-pages provides the man for it. Strange

It's just a matter of reordering code: the [ -t 1 ] and [ ! -t 1 ] lines do what isatty should do. The relevant piece of code in rc.d/functions can be organized a bit differently to completely avoid this error. And if we want to be lazy, a 2>/dev/null at the right place should also work.

Just another vote for fixing this. I keep running up against it in various contexts (acpi handlers, etc). It's easy to mod /etc/rc.d/functions oneself, but I'd rather see the fix in the official version so that I don't have to track my patch against it.

Does this work?
http://projects.archlinux.org/?p=initscripts.git;a=commitdiff;h=b05933da4de82fe515c1eee5e5a7690072b99c79

tput seems to work fine on a non-tty, yes?

Yes, I think that patch works. [ -t 1 ] works fine in the non-terminal contexts and doesn't output anything to stderr (like stty does).

Also, my testing shows that tput works fine on non-tty and doesn't spit to stderr either.

Correction: stty is complaining because its stdin isn't a terminal, it doesn't care whether stdout isn't a terminal. So the "[ -t 1 ]" test in git should be changed to "[ -t 1 -a -t 0 ]." That's the only way to avoid calling stty when it'll be unhappy.

Conceivably, you might want to output color when stdout is a terminal but stdin isn't. But then you'd need to find some non-stty-based method for determing the column size. Maybe use $COLUMNS?

Rather use -o here. Anyway, it doesn't really matter as usually either both 0 and 1 will be a terminal or both won't (except for some rare cases).

Hmm, further testing shows that in many contexts where /etc/rc.d/functions is sourced, stdin isn't a terminal. So making it happy (and so keeping your logs clean if you're logging the output) means you'll lose lots of the pretty colors. Scratch the proposal in the previous comment. I propose instead: (preferred) stop using "stty size" and instead use, e.g., "tput cols". Or (fallback) continue using stty size but add a "2>/dev/null".

Calling netcfg from the command line, 0 is not a terminal but 1 is.

Actually, I'm not sure how much of what I just said is true. I misdiagnosed the problem. What's really going on is that the patch phrakture pointed to is unconditionally zapping USECOLORS.

How about this for the relevant block?

if [ -t 1 ]; then
STAT_COL=$(tput cols)
if [ "$STAT_COL" = "0" ]; then
# if output was 0 (serial console), set default width to 80
STAT_COL=80
fi
else
STAT_COL=80
USECOLOR=""
fi
# we use 13 characters for our own stuff
...etc

I don't know how many terminals define the cols attribute. I can attest that at least linux and rxvt-unicode do.

If we want to continue using stty, then as I said please avoid calling it when input is not a terminal (I hit a problem case where input wasn't a terminal but output was minutes after implementing the patch), or please add "2>/dev/null" to the stty call.

Would someone mind submitting a git format-patch for the proposed changes? It seems like lots of different things are being suggested, so it's easier to speak in code :)

I'm a git dummy, so I don't know how. But attached is a diff -aup against the version of functions in testing (2009.07-3).

I found some corner cases where tput wasn't working, or stty worked better. So the attached patch should be more reliable.

Ah crap... I applied this here:
http://projects.archlinux.org/?p=initscripts.git;a=commitdiff;h=0e36fec20e3c27eaa459e1083833cb85ccf2b34b

But wasn't paying attention and snuck in an additional patch... fixing that now

Thanks

Sorry, but patch I sent on Aug 5 doesn't work. I had changed a STAT_COL="$(/bin/tput cols)"
to STAT_COL="$(/bin/tput cols 2>/dev/null)". The motivation for doing that was there were times at startup where tput might fail (if, e.g., you have /usr on a separate partition and /etc/rc.d/functions is being sourced before /usr/share/terminfo is mounted. That motivation is good, but the solution I proposed is dumb. When you do VAR=$(cmd), stdout isn't going to the terminal. When you do VAR=$(cmd 2>/dev/null), then stderr isn't going to the terminal either. So VAR=$(tput cols 2>/dev/null) is always going to return 80.

I've attached another fix, which first tries to call tput cols silently to see whether there's an error. If no error, then it calls tput cols for real, no longer redirecting stderr, to get the real width of the terminal.

This patch is in git format, which I've now learned how to do. The current version of functions in git is broken---it looks like these fixes were applied twice, or two versions of them were applied, or something like that. This patch should clean it all up.

Forgot the patch.

This version edits a stale comment that no longer applies.

Sorry for the multiple postings. But ignore the previous two patches; here's a third version, against current git master, which includes the previous patches and also ensures that USECOLOR is off when stdout isn't to a terminal.

Bump...I know devs are probably busy with the bugtracker now back up. But /etc/rc.d/functions got pushed to core with the misapplied patches. I don't think it'll crash for anyone, but the functionality is messed up. My patch from Aug 15 should clean the file up.

Hah, Aaron, can you take this one?

Going out of town in the morning, I can get to it Monday night if no one does before that

Finally reviewed and pushed this. All is well?