First of all, this should not be taken as a complete review.
It just reflects my first impression of --enable-single-binary.
On 06/19/2014 10:24 PM, Alex Deymo wrote:
> diff --git a/src/coreutils.c b/src/coreutils.c
> new file mode 100644
> index 0000000..70c2d21
> --- /dev/null
> +++ b/src/coreutils.c
> @@ -0,0 +1,138 @@
...
> +
> + /* 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);
This should go to stderr. Why not error (...)?
> diff --git a/tests/misc/env.sh b/tests/misc/env.sh
> index c4b9737..f9400c7 100755
> --- a/tests/misc/env.sh
> +++ b/tests/misc/env.sh
> @@ -107,7 +107,8 @@ export PATH
> # '/bin/sh -i', rather than '/bin/sh -- -i', which doesn't do what we want.
> # Avoid the issue by using an executable rather than a script.
> # Test -u, rather than -i, to minimize PATH problems.
> -ln -s "$abs_top_builddir/src/echo" ./-u || framework_failure_
> +echo "#!$abs_top_builddir/src/echo" > ./-u || framework_failure_
> +chmod +x ./-u || framework_failure_
This looks problematic - as the change contradicts the explicit comment
about avoiding a script. Why should a symlink to src/echo (which in turn
is a symlink to coreutils) not work?
Further comments:
a) The syntax-check, check-very-expensive and the root tests pass ...
but some of them are skipped because the check on $built_programs fails:
+ require_built_ chroot
+ skip_=no
+ for i in '"$@"'
+ case " $built_programs " in
+ echo 'chroot: not built'
chroot: not built
+ skip_=yes
+ test yes = yes
+ skip_ 'required program(s) not built'
The variable $build_programs now only contains "coreutils".
b) As space is the argument for using --enable-single-binary,
shouldn't a stripped install be the default there?
$ ls -ldog coreutils
-rwxr-xr-x 1 4816636 Jun 20 00:19 coreutils
c) Finally, it'd be good to have a test case for the new
'coreutils' binary. ;-)
Have a nice day,
Berny
