Re: [PATCH v2] build: Option for building all tools in a single binary

[Mike Frysinger]

> --- a/Makefile.am
> +++ b/Makefile.am
>
> +# If we are building a single-binary, create symlinks for them when 
installing.
> +install-exec-hook:
> +     $(AM_V_at)for i in $(single_binary_progs); do                   \
> +             rm -f $(DESTDIR)$(bindir)/$$i$(EXEEXT);                 \
> +             $(LN_S) -s coreutils$(EXEEXT) $(DESTDIR)$(bindir)/$$i$(EXEEXT);\
> +     done

i think you need `|| exit $$?` after the rm & ln statements since this doesn't 
run in set -e mode

> --- /dev/null
> +++ b/src/coreutils-arch.c
> @@ -0,0 +1,14 @@
> +#include <config.h>
> +#include "system.h"

each of these stub files need a one line blurb/copyright/license comment 
block.  see kill.c as an example.

> +/* Ensure that the main for uname is declared even if the tool is not being
> + * build in this single-binary. */

GNU style says to omit the * at the start of line, and two spaces after the 
period.

> --- /dev/null
> +++ b/src/coreutils.c
>
> +/* coreutils.c aggregates the functionality of every other tool into a 
single
> +   binary multiplexed by the value of argv[0]. This is enabled by passing

two spaces after the period

> +   By Alex Deymo <address@hidden> */

two spaces at the end, and a period.

> +/* Declare the main() function on each one of the selected tools. This name

GNU style says to just use main -- omit the ().

> +  if (STREQ (prog_name, "coreutils"))
> +  {
> +    prog_argv++;

GNU style says to indent the braces with two spaces
  if (...)
    {
      ...
    }

> +    prog_name = prog_argv[0]; /* Don't use last_component() in this case. 
*/

i think the preference is to not do inline comments, so move it to above

> +  /* Only print the "program not found" message when no options have been
> +   * passed to coreutils. */
> +  if (optind == 1 && prog_name && !STREQ (prog_name, "coreutils"))
> +    printf ("%s: program not found.\n", prog_name);

shouldn't this write to stderr w/fprintf ?

how about the message also taking the form:
        coreutils: program not found: foo

> --- a/src/local.mk
> +++ b/src/local.mk
>
> +src/coreutils_symlinks:
> +     $(AM_V_GEN)touch $@
> +     $(AM_V_at)for i in $(single_binary_progs); do \
> +             rm -f $(top_srcdir)/src/$$i$(EXEEXT); \
> +             $(LN_S) -s coreutils$(EXEEXT) $(top_srcdir)/src/$$i$(EXEEXT); \
> +     done
> +
> +clean-local:
> +     rm -f src/coreutils_symlinks
> +     $(AM_V_at)for i in $(single_binary_progs); do \
> +             rm -f $(top_srcdir)/src/$$i$(EXEEXT); \
> +     done

same comment wrt `|| exit $$?`
-mike

signature.asc
Description: This is a digitally signed message part.