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

[Alex Deymo]

On Mon, Jun 16, 2014 at 2:36 AM, Pádraig Brady <address@hidden> wrote:
>> int main (int argc, char **argv) ATTRIBUTE_NORETURN;
>
> Yes there are probably special cases for main().
> Adding that at the define at the top of coreutils.c
> like I suggested is fine.

Ok. I added ATTRIBUTE_NORETURN to both in v4, the coreutils.c
declaration and the -D while compiling the foo.c main that has the
rename. We technically need both, but the -D is the most important.

>> 3. Still no clue what's supposed to happen in that test with env, and can't 
>> reproduce any difference between the single-binary and the normal mode 
>> outside the test.
>
> tests/misc/env.sh

If I run this with the link to "coreutils" or with the static binary I
get the same results... which are different from what "make check"
does so I don't really now what should happen here.


>> 4. "make" and "make check" problem
>> The all-am target depends on the manpages $(MANS), which has all the 
>> man/$prog.1 files. Those man/$prog.1 files depend on the src/$prog, which 
>> should not be build in the single-binary case. Because of this dependency, 
>> both src/coreutils and all the src/$prog are built. I added a PHONY target 
>> after all-am that creates the symlinks on src/, but that doesn't work the 
>> first time because makefile will evaluate the dependencies before we create 
>> the symlinks, and it will use the rule it knows to generate the src/$prog 
>> file (overriding the symlink).
>> Ideas on how to solve this?
>
> Maybe the dependency list could be generated and then included,
> or perhaps use a dynamic depedency like.
>
>   man/timeout.1:   $(get_binary_name $@)

hmm... get_binary_name should return "src/coreutils" if the $1 is on
the list of programs and "src/$1" otherwise? I'll try that.


>> diff --git a/src/coreutils.c b/src/coreutils.c
>> +  /* Ensure that at least a parameter was passed to coreutils. */
>> +  if (prog_argc < 1 || !prog_argv[0])
>> +  {
>> +    initialize_main (&argc, &argv);
>> +    set_program_name (argv[0]);
>> +    setlocale (LC_ALL, "");
>> +    bindtextdomain (PACKAGE, LOCALEDIR);
>> +    textdomain (PACKAGE);
>> +    atexit (close_stdout);
>> +
>> +    usage (EXIT_FAILURE);
>> +  }
>
> Rather than repeating the above block it would be better
> to adjust the following conditional to just skip
> the _single_binary_...() call in that case and fall
> through to internal handling then.

Reworked this part.

>> diff --git a/src/kill.c b/src/kill.c
>
> Your editor stripped out form feeds from this file at least. Please reinstate.

Wait what? It wasn't my editor, I did remove those dirty ^L chars from
the C file... but that's your code, so if you want them you can keep
them :-)

>> diff --git a/src/local.mk b/src/local.mk
>
> Rather than having separate ..._main() functions for the above,
> would it be possible to add special cases in coreutils.c to:

I added these as new ..._main() functions that set the ls_mode and
call the ..._ls_main() function instead of rebuilding the ls.c main
with the rename. I did the same for uname/arch. This works even if you
want to build ls separated and dir or vdir in src/coreutils. The
..._ls_main() will be there in that case, but coreutils.c will pretend
it doesn't know =).

> $ ./configure --enable-single-binary --enable-single-binary-exceptions=sort 
> && make
> src/libsinglebin_dir.a(src_libsinglebin_dir_a-ls.o): In function 
> `decode_switches':
> src/ls.c:1532: undefined reference to `libsinglebin_dir_ls_mode'

This probably because I renamed _libsinglebin_dir_ls_mode to
libsinglebin_dir_ls_mode on the generated src/single-binary.mk define
and automake is not smart enough to recompile src/libsinglebin_dir.a
when there's a change on the CFLAGS. make clean should fix that, but I
implemented this in a different way anyway.

I'll review the dependency problem tomorrow.

Thanks,
deymo.

single-binary_v5.patch
Description: Text Data