On 07/07/2014 12:41 AM, Bernhard Voelker wrote: > On 07/05/2014 03:40 PM, Pádraig Brady wrote: >> On 07/04/2014 05:06 PM, Pádraig Brady wrote: >> Rolled up for easy application on master: >> http://www.pixelbeat.org/cu/single-binary_v8.patch > > Thanks for squashing into one patch. > Here some comments - the cases are marked with either > "[{normal,shebang,symlinks} case]" depending on whether > configure was invoked without --enable-single-binary, > with --s...=shebang or with ...=symlinks.
Excellent review again. Details below... > > > 1. "make syntax-check" is boken [normal case] > > $ git am single-binary_v8.patch > $ git clean -xdfq > $ ./bootstrap > $ ./configure --quiet > $ make > $ make syntax-check > make: *** No rule to make target `src/coreutils_', needed by > `src/coreutils'. Stop. fixed > > > 2. Typo in commit msg: > >> When installing, the makefile will create etiher symlinks or > > s/etiher/either/ fixed > > 3. build-aux/gen-single-binary.sh: > >>> # We need to duplicate the specific rules to build each program into a new >> # static library target. We can't reuse the existing target since we need to >> # create a .a file instead of linking the program. We can't do this at >> # ./configure since the file names need to available when automake runs to >> let >> # it generate all the required rules in Makefile.in. > > s/to available/to be available/ fixed > > 4. Dependencies don't seem to be tracked 100% yet [shebang case] > > $ git clean -xdfq > $ ./bootstrap > $ ./configure --quiet --enable-single-binary=shebangs > $ make -j > ... > src/coreutils.c:37:23: fatal error: coreutils.h: No such file or directory > #include "coreutils.h" > ^ > compilation terminated. > make[2]: *** [src/src_coreutils-coreutils.o] Error 1 fixed > > After a second 'make -j', the build is okay. > > > 5. Some remainders are not yet in .gitignore [shebang case] > > $ git status > # On branch allinone > # Untracked files: > # (use "git add <file>..." to include in what will be committed) > # > # build-aux/git-version-gen > # intl/ > # man/dynamic-deps.mk > # src/coreutils_shebangs > nothing added to commit but untracked files present (use "git add" to track) fixed > > 6. "make" dist complains about duplicate name [normal case] > This one is very likely independent of the single-binary patch. > > GEN THANKS > ./thanks-gen: THANKS.in: duplicate name: P�draig Brady separate issue > > > 7. Several tests skipped [shebang case] > E.g. > > chroot-fail.sh: skipped test: required program(s) not built > SKIP: tests/misc/chroot-fail.sh > > coreutils.sh: skipped test: multicall binary is not built > SKIP: tests/misc/coreutils.sh fixed > > Reason: the variable $built_programs now only contains "coreutils", > thus leading to a wrong result for e.g. require_built_ function. > > 8. Test case tests/misc/coreutils.sh skipped [shebang case] > Typo: > >> test -x "$abs_top_builddir}/src/coreutils" \ >> || skip_ "multicall binary is not built" > > s/}// > > 9. Test case tests/misc/coreutils.sh: non-standard compare_ call > >> compare_ out exp || fail=1 > > s/compare_/compare/ > s/out exp/exp out/ > > It should read > compare_ out exp || fail=1 various issues with that test fixed up > > 10. man/coreutils.1 doesn't get built [symlinks case] > ... at least not with a plain 'make'. fixed > 11. "make install" doesn't complain when man/coreutils.1 is missing [shebang > case] fixed > 12. man/coreutils.1: should avoid having the full path here: > >> Try: '/tmp/coreutils-8.22.142-d6383/src/coreutils PROGRAM_NAME --help' for >> help on the particular program. > > I noticed the same in several man/*.1 files now. Separate issue: $ grep -l $PWD man/*.1 man/basename.1 man/cat.1 man/chgrp.1 man/chown.1 man/comm.1 man/coreutils.1 man/dirname.1 man/nohup.1 man/numfmt.1 man/rm.1 > > 13. man/coreutils.1: hint about info page wrong: > That node does not exist: fixed >> info coreutils 'coreutils invocation' > > 14. man/local.mk: writing non-atomically into target: > >> +man/dynamic-deps.mk: Makefile >> + $(AM_V_GEN)rm -f $@ >> + $(AM_V_at)for man in $(ALL_MANS); do \ >> ... >> + done > $@ > > It should write to temporary file and do a final mv(1) instead. fixed > > 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 > 16. src/coreutils.c: search for right main program should have 'else' > >> /* Lookup the right main program. */ >> #define SINGLE_BINARY_PROGRAM(prog_name_str, main_name) \ >> if (STREQ (prog_name_str, prog_name)) \ >> prog_main = _single_binary_main_##main_name; >> #include "coreutils.h" >> #undef SINGLE_BINARY_PROGRAM > > This will STREQ thru all ~100 program names strings, even if the first > one already matched. fixed > 17. Re calling exit() instead of return in main function: >> * src/kill.c: Changes to call exit() in main. > > Shouldn't we have a syntax-check rule to ensure this? leaving as is for now > Although this is quite a big list, I think you're on a good way! 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? 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. thanks again, Pádraig.
>From cd99ef63483989fd89b85e68c1b6fac21cc902d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com> Date: Mon, 7 Jul 2014 09:46:12 +0100 Subject: [PATCH 1/2] build: fix multicall issues identified by Bernhard * src/local.mk (EXTRA_src_coreutils_DEPS): Only set for multicall. (built_SOURCES): Add coreutils.h so generated always. (src_coreutils_SOURCES): Leave at default so that set appropriately when --enable-single-binary is not set (significant with syntax-check). (src_coreutils_CFLAGS): Add -DSINGLE_BINARY to allow building coreutils.c without warnings. * coreutils.c: Avoid using the SINGLE_BINARY_PROGRAM without the SINGLE_BINARY define to avoid an unused macro warning. * build-aux/gen-single-binary.sh: s/to available/to be available/. * tests/misc/coreutils.sh: Fix typos. Now not always skipped. Also skip correctly even after make syntax-check when all binaries are built. * tests/local.mk: Add $single_binary_progs to support require_built_() from init.cfg * man/local.mk: Update dynamic-deps.mk atomically. * build-aux/gen-lists-of-programs.sh: Move "coreutils" from disabled_by_default_progs to build_if_possible_progs, so that "coreutils" is not listed in the list that can be enabled by the configure user with --enable-install-program --- .gitignore | 2 ++ build-aux/gen-single-binary.sh | 8 ++++---- configure.ac | 1 + doc/coreutils.texi | 19 +++++++++++++++++++ man/local.mk | 6 ++++-- src/coreutils.c | 29 ++++++++++++++++++----------- src/local.mk | 23 ++++++++++++----------- tests/local.mk | 2 +- tests/misc/coreutils.sh | 6 +++--- 9 files changed, 64 insertions(+), 32 deletions(-) diff --git a/.gitignore b/.gitignore index 5869791..02a61e8 100644 --- a/.gitignore +++ b/.gitignore @@ -134,6 +134,7 @@ /m4/xsize.m4 /maint.mk /man/*.1 +/man/dynamic-deps.mk /po/*.gmo /po/*.po /po/.gitignore @@ -157,6 +158,7 @@ /po/stamp-po /src/coreutils.h /src/coreutils_symlinks +/src/coreutils_shebangs /src/cu-progs.mk /src/fs-latest-magic.h /src/libsinglebin_*.a diff --git a/build-aux/gen-single-binary.sh b/build-aux/gen-single-binary.sh index f3958bb..4635706 100755 --- a/build-aux/gen-single-binary.sh +++ b/build-aux/gen-single-binary.sh @@ -6,10 +6,10 @@ # We need to duplicate the specific rules to build each program into a new # static library target. We can't reuse the existing target since we need to # create a .a file instead of linking the program. We can't do this at -# ./configure since the file names need to available when automake runs to let -# it generate all the required rules in Makefile.in. The configure step will -# select which ones will be used to build, but they need to be generated -# beforehand. +# ./configure since the file names need to be available when automake runs +# to let it generate all the required rules in Makefile.in. The configure +# step will select which ones will be used to build, but they need to be +# generated beforehand. # # Instead of maintaining a duplicated list of rules, we generate the # single-binary required rules based on the normal configuration found on diff --git a/configure.ac b/configure.ac index 27caffd..7a098a9 100644 --- a/configure.ac +++ b/configure.ac @@ -509,6 +509,7 @@ single_binary_libs= single_binary_deps= single_binary_install_type= if test "$gl_single_binary" != no; then + man1_MANS="$man1_MANS man/coreutils.1" # Convert the list to a space separated list gl_single_binary_exceptions=`echo $gl_single_binary_exceptions | tr ',' ' '` diff --git a/doc/coreutils.texi b/doc/coreutils.texi index 96220c3..19a523d 100644 --- a/doc/coreutils.texi +++ b/doc/coreutils.texi @@ -47,6 +47,7 @@ * chroot: (coreutils)chroot invocation. Specify the root directory. * cksum: (coreutils)cksum invocation. Print POSIX CRC checksum. * comm: (coreutils)comm invocation. Compare sorted files by line. +* coreutils: (coreutils)coreutils invocation. Multi-call program. * cp: (coreutils)cp invocation. Copy files. * csplit: (coreutils)csplit invocation. Split by context. * cut: (coreutils)cut invocation. Print selected parts of lines. @@ -229,6 +230,7 @@ Common Options * Traversing symlinks:: Traversing symlinks to directories * Treating / specially:: Treating / specially * Standards conformance:: Standards conformance +* coreutils invocation:: Multi-call binary invocation Output of entire files @@ -771,6 +773,7 @@ name. * Treating / specially:: --preserve-root and --no-preserve-root. * Special built-in utilities:: @command{break}, @command{:}, @dots{} * Standards conformance:: Conformance to the POSIX standard. +* coreutils invocation:: Multi-call binary invocation. @end menu @@ -1494,6 +1497,22 @@ that assumes an older version of POSIX and uses @samp{sort +1} or @samp{tail +10}, you can work around any compatibility problems by setting @samp{_POSIX2_VERSION=199209} in your environment. +@node coreutils invocation +@section @command{coreutils}: Multi-call binary + +@pindex multicall +@cindex combined +@cindex calling combined multi-call binary + +@command{coreutils} invokes an individual utility, either +implicitly selected by the last component of @samp{argv[0]}, +or by explicitly calling @command{coreutils} with the +@option{--coreutils-prog} option. Synopsis: + +@example +coreutils @option{--coreutils-prog=PROGRAM} @dots{} +@end example + @node Output of entire files @chapter Output of entire files diff --git a/man/local.mk b/man/local.mk index 3b8fd6d..bd18f90 100644 --- a/man/local.mk +++ b/man/local.mk @@ -59,7 +59,7 @@ $(ALL_MANS): $(mandeps) # being installed will have the right dependency for the manpages. CLEANFILES += man/dynamic-deps.mk man/dynamic-deps.mk: Makefile - $(AM_V_GEN)rm -f $@ + $(AM_V_GEN)rm -f $@ $@-t $(AM_V_at)for man in $(ALL_MANS); do \ name=$${man:4: -2} ; # Space is important \ case $$name in \ @@ -74,7 +74,9 @@ man/dynamic-deps.mk: Makefile *) \ echo $$man: src/$$prog$(EXEEXT);; \ esac \ - done > $@ + done > $@-t \ + && chmod a-w $@-t \ + && mv $@-t $@ # Include the generated man dependencies. -include man/dynamic-deps.mk diff --git a/src/coreutils.c b/src/coreutils.c index 9bb3ab4..40342bf 100644 --- a/src/coreutils.c +++ b/src/coreutils.c @@ -29,13 +29,15 @@ #include "system.h" #include "error.h" +#ifdef SINGLE_BINARY /* Declare the main function on each one of the selected tools. This name needs to match the one passed as CFLAGS on single-binary.mk (generated by gen-single-binary.sh). */ -#define SINGLE_BINARY_PROGRAM(prog_name_str, main_name) \ +# define SINGLE_BINARY_PROGRAM(prog_name_str, main_name) \ int _single_binary_main_##main_name (int, char**) ATTRIBUTE_NORETURN; -#include "coreutils.h" -#undef SINGLE_BINARY_PROGRAM +# include "coreutils.h" +# undef SINGLE_BINARY_PROGRAM +#endif /* The official name of this program (e.g., no 'g' prefix). */ #define PROGRAM_NAME "coreutils" @@ -72,14 +74,16 @@ Execute the PROGRAM_NAME built-in program with the given PARAMETERS.\n\ printf ("\n\ Built-in programs:\n" -#define SINGLE_BINARY_PROGRAM(prog_name_str, main_name) " " prog_name_str -#include "coreutils.h" -#undef SINGLE_BINARY_PROGRAM +#ifdef SINGLE_BINARY +# define SINGLE_BINARY_PROGRAM(prog_name_str, main_name) " " prog_name_str +# include "coreutils.h" +# undef SINGLE_BINARY_PROGRAM +#endif "\n"); printf (_("\ \n\ -Try: '%s PROGRAM_NAME --help' for help on the particular program.\n"), +Use: '%s --coreutils-prog=PROGRAM_NAME --help' for individual program help.\n"), program_name); emit_ancillary_info (); } @@ -95,12 +99,15 @@ launch_program (const char *prog_name, int prog_argc, char **prog_argv) if (!prog_argc || !prog_argv || !prog_argv[0] || !prog_name) return; +#ifdef SINGLE_BINARY + if (false); /* Lookup the right main program. */ -#define SINGLE_BINARY_PROGRAM(prog_name_str, main_name) \ - if (STREQ (prog_name_str, prog_name)) \ +# define SINGLE_BINARY_PROGRAM(prog_name_str, main_name) \ + else if (STREQ (prog_name_str, prog_name)) \ prog_main = _single_binary_main_##main_name; -#include "coreutils.h" -#undef SINGLE_BINARY_PROGRAM +# include "coreutils.h" +# undef SINGLE_BINARY_PROGRAM +#endif if (! prog_main) return; diff --git a/src/local.mk b/src/local.mk index e1a5632..e35d286 100644 --- a/src/local.mk +++ b/src/local.mk @@ -106,6 +106,9 @@ src_cksum_LDADD = $(LDADD) src_comm_LDADD = $(LDADD) src_nproc_LDADD = $(LDADD) src_cp_LDADD = $(LDADD) +if !SINGLE_BINARY +src_coreutils_LDADD = $(LDADD) +endif src_csplit_LDADD = $(LDADD) src_cut_LDADD = $(LDADD) src_date_LDADD = $(LDADD) @@ -397,22 +400,20 @@ src_libstdbuf_so_LDADD = $(LIBINTL) src_libstdbuf_so_LDFLAGS = -shared src_libstdbuf_so_CFLAGS = -fPIC $(AM_CFLAGS) +BUILT_SOURCES += src/coreutils.h if SINGLE_BINARY # Single binary dependencies -src_coreutils_SOURCES = src/coreutils.c src/coreutils.h -src_coreutils_CFLAGS = $(AM_CFLAGS) -src_coreutils_LDFLAGS = $(AM_LDFLAGS) +src_coreutils_CFLAGS = -DSINGLE_BINARY $(AM_CFLAGS) +#src_coreutils_LDFLAGS = $(AM_LDFLAGS) src_coreutils_LDADD = $(single_binary_deps) $(LDADD) $(single_binary_libs) src_coreutils_DEPENDENCIES = $(LDADD) $(single_binary_deps) include $(top_srcdir)/src/single-binary.mk -endif SINGLE_BINARY - # Creates symlinks or shebangs to the installed programs when building -# coreutils single binary. This doesn't do anything when $(single_binary_progs) -# is empty. +# coreutils single binary. EXTRA_src_coreutils_DEPENDENCIES = src/coreutils_$(single_binary_install_type) +endif SINGLE_BINARY CLEANFILES += src/coreutils_symlinks src/coreutils_symlinks: Makefile @@ -567,10 +568,10 @@ src/version.h: Makefile CLEANFILES += src/coreutils.h src/coreutils.h: Makefile $(AM_V_GEN)rm -f $@ - $(AM_V_at)for prog in $(single_binary_progs); do \ - prog=`basename $$prog` ; \ - main=`echo $$prog | tr '[' '_'` ; \ - echo "SINGLE_BINARY_PROGRAM(\"$$prog\", $$main)" ; \ + $(AM_V_at)for prog in $(single_binary_progs); do \ + prog=`basename $$prog`; \ + main=`echo $$prog | tr '[' '_'`; \ + echo "SINGLE_BINARY_PROGRAM(\"$$prog\", $$main)"; \ done | sort > $@t $(AM_V_at)chmod a-w $@t $(AM_V_at)mv $@t $@ diff --git a/tests/local.mk b/tests/local.mk index d1e6f59..e0f1f84 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -65,7 +65,7 @@ TESTS_ENVIRONMENT = \ abs_top_builddir='$(abs_top_builddir)' \ abs_top_srcdir='$(abs_top_srcdir)' \ abs_srcdir='$(abs_srcdir)' \ - built_programs='$(built_programs)' \ + built_programs='$(built_programs) $(single_binary_progs)' \ host_os=$(host_os) \ host_triplet='$(host_triplet)' \ srcdir='$(srcdir)' \ diff --git a/tests/misc/coreutils.sh b/tests/misc/coreutils.sh index ad39c8b..2935bbe 100755 --- a/tests/misc/coreutils.sh +++ b/tests/misc/coreutils.sh @@ -20,12 +20,12 @@ . "${srcdir=.}/tests/init.sh"; path_prepend_ ./src print_ver_ coreutils -test -x "$abs_top_builddir}/src/coreutils" \ - || skip_ "multicall binary is not built" +test -s "$abs_top_builddir/src/coreutils.h" \ + || skip_ "multicall binary is disabled" # Yes outputs all its params so is good to verify argv manipulations echo 'y' > exp coreutils --coreutils-prog=yes | head -n10 | uniq > out -compare_ out exp || fail=1 +compare exp out || fail=1 Exit $fail -- 1.7.7.6 >From 2fcc5ffa9f6bb3217297feb245e1844a2e601a4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com> Date: Mon, 7 Jul 2014 16:31:01 +0100 Subject: [PATCH 2/2] build: disallow --enable-single-binary=symlinks --program-prefix combo * configure.ac: Since we currently only match on internal names, disallow this combination for now. --- configure.ac | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/configure.ac b/configure.ac index 7a098a9..b48beae 100644 --- a/configure.ac +++ b/configure.ac @@ -235,6 +235,12 @@ 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 + AC_MSG_ERROR([program name transformations are not currently supported + with --enable-single-binary=symlinks.]) + fi +fi AM_CONDITIONAL([SINGLE_BINARY], [test "$gl_single_binary" != no]) AC_FUNC_FORK -- 1.7.7.6