On 22/04/18 03:25, Assaf Gordon wrote: > Hello, > > Following recent discussion about shebang lines [1][2], > attached is a patch to add "env -S", with the same syntax as freebsd's > env(1). > > [1] https://lists.gnu.org/r/coreutils/2017-05/msg00020.html > [2] https://lists.gnu.org/r/bug-coreutils/2018-04/msg00031.html > > Also included a small patch adding "--debug/-v" option. > > It includes tests with close to %100 coverage, but no documentation yet. > If you think this is a good addition, I'll update the patch with > documentation.
This is quite useful functionality to have in a standard tool. Thanks a lot for implementing it. I'd rather be consistent with other utils and just use the long --debug form. Oh I see, the FreeBSD version supports -v. So in that case I agree with supporting the short option also. Please add an extra space after "--split-string=S " so that the man page will be formatted appropriately. There is no need to specify "inline" on functions generally (unless defining in headers). Best let the compiler sort it out. The error() for --debug may need translations or use a debug() wrapper to avoid the syntax check In scan_varname(), I think we should we restrict to: s/isalpha/c_isalpha/ s/isalnum/c_isalnum/ I see the test script identifies the case where the OS does split and handles spaces in the src dir. Excellent. I noticed some tweaks for the test which are attached. ------ some general notes ----- I see that if one wants to use other options in combo with -S they need to be part of the -S option. I.E. after -S. It would be good to illustrate that in the docs, or perhaps issue a warning if -S comes after other options? For example it looks buggy that -u TERM is just ignored in this example: src/env -v -u TERM -S sh -c "echo \$TERM" We should at least warn about this case, but maybe error out. Another gotcha is that portably specifying -v with -S (i.e. with only one option) requires: src/env '-vS sh -c "echo \$TERM"' So I would document two forms in the man page/--help like: env [OPTION]... [-] [NAME=VALUE]... [COMMAND [ARG]...] env -[v]S '[OPTION]... [-] [NAME=VALUE]... [COMMAND [ARG]...]' and make it clear usage() that -S is only needed with shebang lines. The quotes above are important to document too, as otherwise consistent processing (like interpolation etc.) wouldn't be done on operating systems that did split shebangs (or when used in a standard command invocation). thanks! Pádraig
diff --git a/tests/misc/env-S-script.sh b/tests/misc/env-S-script.sh old mode 100644 new mode 100755 index 0377191..d164be5 --- a/tests/misc/env-S-script.sh +++ b/tests/misc/env-S-script.sh @@ -37,12 +37,11 @@ chmod a+x env_test || framework_failure_ # support multiple arguments on the shebang line. # Ignoring the absolute paths, the script is: # #!env printf x%sx\n A B -printf "#!$dir/env $dir/printf "'x%%sx\\n A B\n' > env_bad || framework_failure_ +printf "#!$dir/env $dir/printf "'x%%sx\\n A B\n' > env_bad || + framework_failure_ chmod a+x env_bad || framework_failure_ -returns_ 127 ./env_bad \ - || warn_ "WARNING: OS natively accepts multiple arguments on shebang line" - - +returns_ 127 ./env_bad || + warn_ 'Note: OS natively accepts multiple arguments on shebang line' # env should execute 'printf' with 7 parameters: # 'x%sx\n', 'A', 'B' from the "-S" argument, @@ -51,7 +50,7 @@ returns_ 127 ./env_bad \ # #!env -S printf x%sx\n A B printf "#!$dir/env -S $dir/printf "'x%%sx\\n A B\n' > env1 || framework_failure_ chmod a+x env1 || framework_failure_ -cat<<EOF>exp1 || framework_failure_ +cat<<\EOF>exp1 || framework_failure_ xAx xBx x./env1x @@ -67,10 +66,10 @@ compare exp1 out1 || fail=1 # 'A B' and not two paramaters 'A','B'. # Ignoring the absolute paths, the script is: # #!env -S printf x%sx\n "A B" -printf "#!$dir/env -S $dir/printf "'x%%sx\\n "A B"\n' > env2 \ - || framework_failure_ +printf "#!$dir/env -S $dir/printf "'x%%sx\\n "A B"\n' > env2 || + framework_failure_ chmod a+x env2 || framework_failure_ -cat<<EOF>exp2 || framework_failure_ +cat<<\EOF>exp2 || framework_failure_ xA Bx x./env2x EOF @@ -81,9 +80,10 @@ compare exp2 out2 || fail=1 # backslash-underscore instead of spaces. # Ignoring the absolute paths, the script is: # #!env -Sprintf\_x%sx\n\_Y -printf "#!$dir/env -S$dir/printf"'\\_x%%sx\\n\\_Y\n' > env3 || framework_failure_ +printf "#!$dir/env -S$dir/printf"'\\_x%%sx\\n\\_Y\n' > env3 || + framework_failure_ chmod a+x env3 || framework_failure_ -cat<<EOF>exp3 || framework_failure_ +cat<<\EOF>exp3 || framework_failure_ xYx x./env3x xWx @@ -99,7 +99,7 @@ compare exp3 out3 || fail=1 printf "#!$dir/env -S$dir/printf"' x%%sx\\n A#B #C D\n' > env4 \ || framework_failure_ chmod a+x env4 || framework_failure_ -cat<<EOF>exp4 || framework_failure_ +cat<<\EOF>exp4 || framework_failure_ xA#Bx x./env4x xZx @@ -116,7 +116,7 @@ compare exp4 out4 || fail=1 { printf "#!$dir/env -S perl -w -T\n" ; printf 'print "hello\\n";\n' ; } > env5 || framework_failure_ chmod a+x env5 || framework_failure_ -cat<<EOF>exp5 || framework_failure_ +cat<<\EOF>exp5 || framework_failure_ hello EOF ./env5 > out5 || fail=1 @@ -138,7 +138,4 @@ printf "env6" > exp6 || framework_failure_ compare exp6 out6 || fail=1 - - - Exit $fail