On 26/06/18 04:15, Assaf Gordon wrote: > Hello Pádraig and all, > > First, > thanks for all the hard work regarding the env-S test failure. > The shebang length limit is an interesting edge-case I haven't thought > about... > > > Second, > Attached few test failures (not env-S-script/shebang ones). > > ====== > > On Debian 8.10/i686 (this is gcc45.fsffrance.org from gcc copmile farm): > > FAIL: tests/cp/fiemap-perf > > This is a false-positive due to the test assuming that > copying 1TB of sparse file will finish in less than 10 seconds: > https://opengrok.housegordon.com/source/xref/coreutils/tests/cp/fiemap-perf.sh#42 > > Was previously reported for 8.29: > https://lists.gnu.org/archive/html/coreutils/2017-12/msg00059.html > > A bit more information this time: > > $ timeout 10 truncate -s1T f > $ ls -lh f* > -rw-r--r-- 1 agn agn 1.0T Jun 26 12:01 f > $ timeout 10 cp f f2 ; echo $? > > > 124 > > strace shows that the 'ioctl(FS_IOC_FIEMAP)' did not terminate within 10 > seconds. Perhaps it's a slow machine - gcc45 is "AMD Athlon(tm) II X4 > 640 Processor".
That's 3.16 kernel I think. Seems like a kernel bug, perhaps with 32 mode. There should only be minimal processing required for that ioctl. > On Debian 9.4/x86_64: > > FAIL: tests/df/df-symlink > > This is a false-positive, on my system /tmp is bind-mounted to /scratch > (which is mounted from /dev/sdb2): > > ++ grep -F /dev/sdb2 > ++ wc -l > + test 1 = 1 > + df --out=source,target . > + compare exp out > + compare_dev_null_ exp out > + test 2 = 2 > + test xexp = x/dev/null > + test xout = x/dev/null > + return 2 > + case $? in > + compare_ exp out > + diff -u exp out > --- exp 2018-06-25 05:16:53.034770806 -0600 > +++ out 2018-06-25 05:16:53.042771018 -0600 > @@ -1,2 +1,2 @@ > Filesystem Mounted on > -/dev/sdb2 /tmp > +/dev/sdb2 /scratch > + fail=1 > > ===== > > On OpenBSD 6.2/amd64, few test failures: > > FAIL: tests/misc/env > > seems like "env -" does not work. I will investigate further > (perhaps I've broken something with the changes to the command-line > processing?). > > $ ./src/env --version > env (GNU coreutils) 8.29.57-2ed7c2 > Copyright (C) 2018 Free Software Foundation, Inc. > License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses > /gpl.html>. > This is free software: you are free to change and redistribute it. > There is NO WARRANTY, to the extent permitted by law. > > Written by Richard Mlynarik, David MacKenzie, and Assaf Gordon. > > $ ./src/env - > env: invalid option -- '-' Looks like the hack of searching for '-' option only works on some getopt implementations. We should probably remove that character at least. Also in this area, the argc==3 is too restrictive. Patch attached to address these. > Also one ERROR: > > ERROR: tests/misc/usage_vs_getopt > ================================= > > using SHELL=/bin/sh with 'set -x' corrupts stderr > chroot: unknown option -- thisoptiondoesnotexist > usage_vs_getopt.sh: set-up failure: > ERROR tests/misc/usage_vs_getopt.sh (exit status: 99) > > I believe this is a false-positive. > The "framework_failure_" is triggered > when testing an invalid short option on line 37: > https://opengrok.housegordon.com/source/xref/coreutils/tests/misc/usage_vs_getopt.sh#37 > > Programs are tested like so: > > chroot -/ > > Then "grep" searches for "'/'" in STDERR (i.e. > single-quote,slash-single-quote). > > On this system (OpenBSD 6.2 with C locale), the error message does > not contain single-quotes and the 'grep' fails. Another assumption of getopt implementation. In your patch it's probably safer to not anchor your RE to $. I think the following `sed` also needs adjusting. I'll apply your patch with those tweaks. thanks! Pádraig
From 91597e550ba63d147768cff0ff9df0cfb602646a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]> Date: Tue, 26 Jun 2018 23:52:38 -0700 Subject: [PATCH] env: adjust diagnostics provide for shebang usage * src/env.c (main): Don't process '-' specially since that causes an issue on openbsd getopt implementation where a lone '-' is now processed as an option, and it doesn't particuarly help diagnose common shebang usage issues. Also don't restrict the extra diagnostics for shebang usage to the case with 3 arguments, as further arguments can be passed to the script. * tests/misc/env-S.pl: Adjust accordingly. --- src/env.c | 8 +++----- tests/misc/env-S.pl | 21 +++++++++------------ 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/src/env.c b/src/env.c index 97b6d6b..1915e79 100644 --- a/src/env.c +++ b/src/env.c @@ -48,7 +48,7 @@ static bool dev_debug; static char *varname; static size_t vnlen; -static char const shortopts[] = "+C:iS:u:v0 \t-"; +static char const shortopts[] = "+C:iS:u:v0 \t"; static struct option const longopts[] = { @@ -566,14 +566,12 @@ main (int argc, char **argv) break; case ' ': case '\t': - case '-': /* Space,tab,dash are undocumented options. Attempt to detect incorrect shebang usage with extraneous space, e.g.: #!/usr/bin/env -i command In which case argv[1] == "-i command". */ error (0, 0, _("invalid option -- '%c'"), optc); - if (argc == 3) - error (0, 0, _("use -[v]S to pass options in shebang lines")); + error (0, 0, _("use -[v]S to pass options in shebang lines")); usage (EXIT_CANCELED); case_GETOPT_HELP_CHAR; @@ -656,7 +654,7 @@ main (int argc, char **argv) int exit_status = errno == ENOENT ? EXIT_ENOENT : EXIT_CANNOT_INVOKE; error (0, errno, "%s", quote (argv[optind])); - if (argc == 3 && exit_status == EXIT_ENOENT && strchr (argv[optind], ' ')) + if (exit_status == EXIT_ENOENT && strchr (argv[optind], ' ')) error (0, 0, _("use -[v]S to pass options in shebang lines")); return exit_status; diff --git a/tests/misc/env-S.pl b/tests/misc/env-S.pl index d3bfc46..5c3715b 100755 --- a/tests/misc/env-S.pl +++ b/tests/misc/env-S.pl @@ -201,20 +201,18 @@ my @Tests = {ERR=>"$prog: only \${VARNAME} expansion is supported, " . "error at: \${9B}\n"}], - # Test incorrect shebang usage (extraneous sapce). - # With anything other than 3 args report invalid options (as before). - ['err_sp1', q['-v-S cat -n'], {EXIT=>125}, - {ERR=>"env: invalid option -- '-'\n" . - "Try 'env --help' for more information.\n"}], + # Test incorrect shebang usage (extraneous whitespace). ['err_sp2', q['-v -S cat -n'], {EXIT=>125}, {ERR=>"env: invalid option -- ' '\n" . + "env: use -[v]S to pass options in shebang lines\n" . "Try 'env --help' for more information.\n"}], ['err_sp3', q['-v -S cat -n'], {EXIT=>125}, # embedded tab after -v {ERR=>"env: invalid option -- '\t'\n" . + "env: use -[v]S to pass options in shebang lines\n" . "Try 'env --help' for more information.\n"}], - # With exactly 3 args, assume it's incorrect shebang usage, - # and report a different message. This typically happens with: + # Also diagnose incorrect shebang usage when failing to exec. + # This typically happens with: # # $ cat xxx # #!env -v -S cat -n @@ -225,12 +223,11 @@ my @Tests = # argv[0] = env # argv[1] = '-v -S cat -n' # argv[2] = './xxx' - ['err_sp5', q['-v -S cat -n' ./xxx], {EXIT=>125}, - {ERR=>"env: invalid option -- ' '\n" . - "env: use -[v]S to pass options in shebang lines\n" . - "Try 'env --help' for more information.\n"}], + ['err_sp5', q['cat -n' ./xxx], {EXIT=>127}, + {ERR=>"env: 'cat -n': No such file or directory\n" . + "env: use -[v]S to pass options in shebang lines\n"}], - ['err_sp6', q['cat -n' ./xxx], {EXIT=>127}, + ['err_sp6', q['cat -n' ./xxx arg], {EXIT=>127}, {ERR=>"env: 'cat -n': No such file or directory\n" . "env: use -[v]S to pass options in shebang lines\n"}], ); -- 2.9.3
