FS#12251 - [initscripts] rc.d scripts cannot be called from cron

Attached to Project: Arch Linux
Opened by Gavin Bisesi (Daenyth) - Wednesday, 26 November 2008, 17:19 GMT
Last edited by Roman Kyrylych (Romashka) - Saturday, 13 February 2010, 16:15 GMT
Task Type Bug Report
Category Packages: Core
Status Closed
Assigned To Aaron Griffin (phrakture)
Thomas Bächler (brain0)
Architecture All
Severity Low
Priority Normal
Reported Version None
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

The rc.d scripts can't be called fron cron because of the way they use stty:

/bin/stty: standard input: Inappropriate ioctl for device
This task depends upon

Closed by  Roman Kyrylych (Romashka)
Saturday, 13 February 2010, 16:15 GMT
Reason for closing:  Fixed
Additional comments about closing:  patch applied, no response, assuming fixed
Comment by Thomas Bächler (brain0) - Sunday, 07 June 2009, 14:32 GMT
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.
Comment by Gavin Bisesi (Daenyth) - Monday, 08 June 2009, 14:29 GMT
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
Comment by Thomas Bächler (brain0) - Monday, 08 June 2009, 15:22 GMT
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.
Comment by Jim Pryor (Profjim) - Friday, 31 July 2009, 01:36 GMT
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.
Comment by Aaron Griffin (phrakture) - Friday, 31 July 2009, 03:16 GMT Comment by Aaron Griffin (phrakture) - Friday, 31 July 2009, 03:17 GMT
tput seems to work fine on a non-tty, yes?
Comment by Jim Pryor (Profjim) - Friday, 31 July 2009, 07:40 GMT
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.
Comment by Jim Pryor (Profjim) - Saturday, 01 August 2009, 13:18 GMT
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?
Comment by Thomas Bächler (brain0) - Saturday, 01 August 2009, 13:27 GMT
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).
Comment by Jim Pryor (Profjim) - Saturday, 01 August 2009, 13:34 GMT
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".

Comment by Jim Pryor (Profjim) - Saturday, 01 August 2009, 13:35 GMT
Calling netcfg from the command line, 0 is not a terminal but 1 is.
Comment by Jim Pryor (Profjim) - Saturday, 01 August 2009, 13:42 GMT
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.
Comment by Jim Pryor (Profjim) - Saturday, 01 August 2009, 13:51 GMT
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.

Comment by Aaron Griffin (phrakture) - Saturday, 01 August 2009, 17:29 GMT
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 :)
Comment by Jim Pryor (Profjim) - Saturday, 01 August 2009, 18:00 GMT
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).
Comment by Jim Pryor (Profjim) - Wednesday, 05 August 2009, 12:56 GMT
I found some corner cases where tput wasn't working, or stty worked better. So the attached patch should be more reliable.
Comment by Aaron Griffin (phrakture) - Wednesday, 05 August 2009, 17:12 GMT
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
Comment by Jim Pryor (Profjim) - Wednesday, 05 August 2009, 22:32 GMT
Thanks
Comment by Jim Pryor (Profjim) - Friday, 14 August 2009, 17:10 GMT
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.
Comment by Jim Pryor (Profjim) - Friday, 14 August 2009, 17:11 GMT
Forgot the patch.
Comment by Jim Pryor (Profjim) - Friday, 14 August 2009, 17:13 GMT
This version edits a stale comment that no longer applies.
Comment by Jim Pryor (Profjim) - Saturday, 15 August 2009, 15:54 GMT
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.
Comment by Jim Pryor (Profjim) - Saturday, 05 September 2009, 02:56 GMT
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.
Comment by Thomas Bächler (brain0) - Saturday, 05 September 2009, 04:19 GMT
Hah, Aaron, can you take this one?
Comment by Aaron Griffin (phrakture) - Saturday, 05 September 2009, 06:54 GMT
Going out of town in the morning, I can get to it Monday night if no one does before that
Comment by Aaron Griffin (phrakture) - Thursday, 24 September 2009, 21:38 GMT
Finally reviewed and pushed this. All is well?

Loading...