On 07/08/2014 11:50 AM, Pádraig Brady wrote: > On 07/08/2014 04:23 AM, Alex Deymo wrote: >> Hi! >> I'm back. >> >> On Mon, Jul 7, 2014 at 8:40 AM, Pádraig Brady <[email protected] >> <mailto:[email protected]>> wrote: >> >> On 07/07/2014 12:41 AM, Bernhard Voelker wrote: >> > On 07/05/2014 03:40 PM, Pádraig Brady wrote: >> > >> > 15. src/coreutils-{arch,dir,vdir}.c wrapper: >> > Why don't we do this also in non-single-binary case? ;-) >> >> leaving as is for now >> >> >> Doing this in the non-single-binary case doesn't help much. In the current >> code, these three binaries differ only in the value of a variable in the >> .data section. Using the single-binary code, they would differ is some code >> that runs and sets that value accordingly. >> >> >> Changes are attached to this email. >> >> Rolled up patch is at: >> http://www.pixelbeat.org/cu/single-binary_v9.patch >> >> One other change to consider is that we might >> change `coreutils --coreutils-prog=` to `coreutils --program` >> as the former is a bit long/awkward/redundant? >> >> >> The idea of the awkward/redundant flag is that it is unique. The current >> coreutils.c checks both the argv[0] and the --coreutils-prog= flag as >> argv[1]. The flag is only used internally and users should not pass that >> flag to any other program, so the code works either for symlinks o shebangs. >> If you want to change the flag to something shorter like "--program" then we >> should also pass a compile-time value to coreutils.c to tell it where to >> read the program name from (argv[0] basename or a suffix of argv[1]) >> >> Another thing I just thought of is that we should change the ENOENT >> warning in coreutils.c to something explicit as the error >> pertains to internal functionality, rather than optional >> external links/scripts etc. >> >> >> Let me know if you want me to work on these changes (or other) so we don't >> duplicate the work. > > If you could look at it it would be great thanks. > There's the above --coreutils-prog possible adjustment and the > man page issues Bernhard mentioned. > > Re the --coreutils-prog adjustment, if we were adding a compile time > adjustment, > then it might be possible to do away with an option altogether. i.e. support: > > coreutils ls ... > > I'm not pushed either way TBH as I mainly see the explicit coreutils > invocation as a means to support the shebangs method.
I'm itching to get coreutils-8.23 released, so completed the attached few final fixes. I didn't think this would impact significantly any further worth you might be doing. Here is the complete rolled up patch, which I'm about ready to push. http://www.pixelbeat.org/cu/single-binary_v10.patch thanks, Pádraig.
>From 8a81dbfbfdf512ca7b118f991fafe0e9f1a79700 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]> Date: Tue, 8 Jul 2014 15:23:53 +0100 Subject: [PATCH] build: various multicall fixes * configure.ac: Support --enable-single-binary=symlinks again. * man/local.mk: Adjust so absolute paths on longer in man pages. * src/coreutils.c: (main): In symlinks mode, ensure 'install' is recognized as a program. Change the program not found error message away from ENOENT as that's confusing when the program is specified with a parameter, in which case the optional external link is irrelevant. Always output this error if --coreutils-prog=$unknown was passed. * tests/misc/coreutils.sh: Ensure this error is always output when appropriate. --- configure.ac | 4 ++-- man/local.mk | 10 +++++----- src/coreutils.c | 10 +++++++++- tests/misc/coreutils.sh | 5 +++++ 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/configure.ac b/configure.ac index b48beae..b50f6e1 100644 --- a/configure.ac +++ b/configure.ac @@ -235,8 +235,8 @@ AC_ARG_ENABLE([single-binary-exceptions], [gl_single_binary_exceptions=$enableval], [gl_single_binary_exceptions=] ) -if test "$gl_single_binary" = "symlinks"; then - if ! test "x$program_transform_name" = "x"; then +if test "$gl_single_binary" = 'symlinks'; then + if ! test "`echo ls | sed \"$program_transform_name\"`" = 'ls'; then AC_MSG_ERROR([program name transformations are not currently supported with --enable-single-binary=symlinks.]) fi diff --git a/man/local.mk b/man/local.mk index bd18f90..0f0b66e 100644 --- a/man/local.mk +++ b/man/local.mk @@ -87,9 +87,9 @@ man/dynamic-deps.mk: Makefile ## creating 'install.1'. Similarly, ensure that it uses the 'src/[' binary ## to create 'test.1'. case $$name in \ - install) prog='ginstall';; \ - test) prog='[';; \ - *) prog=$$name;; \ + install) prog='ginstall'; argv=$$name;; \ + test) prog='['; argv='[';; \ + *) prog=$$name; argv=$$prog;; \ esac; \ ## Note the use of $$t/$*, rather than just '$*' as in other packages. ## That is necessary to avoid failures for programs that are also shell @@ -98,11 +98,11 @@ man/dynamic-deps.mk: Makefile && t=$*.td \ && rm -rf $$t \ && $(MKDIR_P) $$t \ - && (cd $$t && $(LN_S) '$(abs_top_builddir)/src/'$$prog $$name) \ + && (cd $$t && $(LN_S) '$(abs_top_builddir)/src/'$$prog $$argv) \ && $(run_help2man) \ --source='$(PACKAGE_STRING)' \ --include=$(srcdir)/man/$$name.x \ - --output=$$t/$$name.1 '$(abs_top_builddir)/src/'$$prog \ + --output=$$t/$$name.1 $$t/$$argv \ --info-page='coreutils \(aq'$$name' invocation\(aq' \ && sed \ -e 's|$*\.td/||g' \ diff --git a/src/coreutils.c b/src/coreutils.c index 40342bf..c459b1d 100644 --- a/src/coreutils.c +++ b/src/coreutils.c @@ -28,6 +28,7 @@ #include "system.h" #include "error.h" +#include "quote.h" #ifdef SINGLE_BINARY /* Declare the main function on each one of the selected tools. This name @@ -75,6 +76,7 @@ Execute the PROGRAM_NAME built-in program with the given PARAMETERS.\n\ printf ("\n\ Built-in programs:\n" #ifdef SINGLE_BINARY +/* XXX: Ideally we#d like to present "install" here, not "ginstall". */ # define SINGLE_BINARY_PROGRAM(prog_name_str, main_name) " " prog_name_str # include "coreutils.h" # undef SINGLE_BINARY_PROGRAM @@ -131,6 +133,11 @@ main (int argc, char **argv) char *prog_name = last_component (argv[0]); int optc; + /* Map external name to internal name. */ + char ginstall[] = "ginstall"; + if (STREQ (prog_name, "install")) + prog_name = ginstall; + /* If this program is called directly as "coreutils" or if the value of argv[0] is an unknown tool (which "coreutils" is), we proceed and parse the options. */ @@ -168,6 +175,7 @@ main (int argc, char **argv) { argv[nskip] = arg_name; /* XXX: Discards any specified path. */ launch_program (prog_name, argc - nskip, argv + nskip); + error (EXIT_FAILURE, 0, _("unknown program %s"), quote (prog_name)); } } @@ -191,7 +199,7 @@ main (int argc, char **argv) /* Only print the error message when no options have been passed to coreutils. */ if (optind == 1 && prog_name && !STREQ (prog_name, "coreutils")) - error (0, ENOENT, _("program %s"), prog_name); + error (0, 0, _("unknown program %s"), quote (prog_name)); usage (EXIT_FAILURE); } diff --git a/tests/misc/coreutils.sh b/tests/misc/coreutils.sh index 2935bbe..a22bc9f 100755 --- a/tests/misc/coreutils.sh +++ b/tests/misc/coreutils.sh @@ -28,4 +28,9 @@ echo 'y' > exp coreutils --coreutils-prog=yes | head -n10 | uniq > out compare exp out || fail=1 +# Ensure if incorrect program passed, we diagnose +echo "coreutils: unknown program 'blah'" > exp +coreutils --coreutils-prog='blah' --help 2>err && fail=1 +compare exp err || fail=1 + Exit $fail -- 1.7.7.6
