On 07/02/2014 01:44 AM, Alex Deymo wrote:
> Hi again,
>
> Sorry for the delay with this patch. Here is patch v7 attached.
>
> *** symlinks vs shebangs:
>
> * Other projects use symlinks to have a multicall binary; including busybox
> and toolbox which cover similar functionality as coreutils; but not limited
> to them: lvm (vgcreate, pvcreate, etc), xz (xzcat, unxz point to "xz") and
> others I'm not aware of.
>
> * People using symlinks do not protect against the symlink to symlink case;
> they just rely on argv[0]. symlink to symlink is not the only way to provide
> a different argv[0]; you can pass whatever you want as the first argument of
> argv when you call exec().
>
> * The scheme to follow symlinks to determine which one was the last symlink
> requires to accurately reproduce the path search algorithm used by exec() on
> the platform we are running; and requires the make a couple of syscalls just
> to know what's the tool to call, yet still doesn't solve the whole case.
>
> * Shebangs allow you to have a symlink to them and it still works without
> extra work. On some systems, we can change the value of /proc/$PID/cmdline
> and other files to not break use cases like "killall foo" or the result of
> "ps".
>
>
> In general, if you want to use --enable-single-binary is because size is a
> concern; and in those cases you know your environment (you are deploying an
> embedded system with a particular kernel). You might not care about 4MB in a
> multi GB linux distro and there's people out there still doing symlink to
> symlink that eventually will get their scripts right; but not yet.
>
> So, I added an option where you can choose at configure time if you want to
> install symlinks or shebangs (--enable-single-binary=shebangs for example). I
> tested this patch on the ChromiumOS builders and it worked (we managed to
> build a builder environment and images for amd64, x86 and arm).
>
> In the shebang case, the program will attempt to modify /proc/$PID/cmdline by
> using prctl() if available. There are other ways to modify argv[] on other
> platforms (look at the sendmail or postgresql case for example), so
> eventually we should migrate that functionality to a gnulib file and make it
> work on all the supported platforms... but for now, you still have the choice
> to use symlinks if that works for you.
>
> Let me know if you have other questions about this patch and happy canada day
> for the canadians out there on this list.
>
> deymo.
>
Excellent work.
One small issue I noticed (in symlinks mode at least)
was that `make syntax-check` causes all the
tools to be remade as stand-alone binaries.
This is related to `make src/$binary` replacing
the symlink with a stand-alone binary.
Note that isn't a supported end user mode anyway,
so it's not blocking, though nice to avoid.
As an aside, the reason end users can not do:
make src/$binary
is due to our use of BUILT_SOURCES of which the automake manual says:
"you cannot use BUILT_SOURCES if the ability to run
‘make foo’ on a clean tree is important to you."
coreutils uses BUILT_SOURCES, as the mandatory configure
step is expensive compared to the build itself,
so there is not much advantage to supporting that.
A more problematic issue is that program prefix isn't supported.
i.e.: ./configure --enable-single-binary --program-prefix=g
BTW I've attached some spelling fixes which I've rolled in here.
thanks!
Pádraig.
diff --git a/NEWS b/NEWS
index 16b9fa3..6fc2471 100644
--- a/NEWS
+++ b/NEWS
@@ -105,7 +105,7 @@ GNU coreutils NEWS -*- outline -*-
selected programs in a single binary called "coreutils". The selected
programs can still be called directly using symlinks to "coreutils" or
shebangs with the option --coreutils-prog= passed to this program. The
- install behaviour is determined by the option --enable-single-binary=symlinks
+ install behavior is determined by the option --enable-single-binary=symlinks
or --enable-single-binary=shebangs (the default). With the symlinks option,
you can't make a second symlink to any program because that will change the
name of the called program, which is used by coreutils to determine the
diff --git a/configure.ac b/configure.ac
index 7e30f55..27caffd 100644
--- a/configure.ac
+++ b/configure.ac
@@ -503,7 +503,7 @@ man1_MANS=`
# a distribution tarball.
EXTRA_MANS=`for p in $no_install_progs_default; do echo man/$p.1; done`
-# Replace all the programs by the single binary and simlinks if specified.
+# Replace all the programs by the single binary and symlinks if specified.
single_binary_progs=
single_binary_libs=
single_binary_deps=
diff --git a/src/coreutils-arch.c b/src/coreutils-arch.c
index a93f484..899cc93 100644
--- a/src/coreutils-arch.c
+++ b/src/coreutils-arch.c
@@ -21,7 +21,7 @@
#include "uname.h"
/* Ensure that the main for uname is declared even if the tool is not being
- build in this single-binary. */
+ built in this single-binary. */
int _single_binary_main_uname (int argc, char** argv) ATTRIBUTE_NORETURN;
int _single_binary_main_arch (int argc, char** argv) ATTRIBUTE_NORETURN;
diff --git a/src/coreutils-dir.c b/src/coreutils-dir.c
index 8166b0e..4b488f4 100644
--- a/src/coreutils-dir.c
+++ b/src/coreutils-dir.c
@@ -20,7 +20,7 @@
#include "system.h"
#include "ls.h"
-/* Ensure that the main for ls is declared even if the tool is not being build
+/* Ensure that the main for ls is declared even if the tool is not being built
in this single-binary. */
int _single_binary_main_ls (int argc, char** argv) ATTRIBUTE_NORETURN;
int _single_binary_main_dir (int argc, char** argv) ATTRIBUTE_NORETURN;
diff --git a/src/coreutils-vdir.c b/src/coreutils-vdir.c
index 2dbcd70..036367f 100644
--- a/src/coreutils-vdir.c
+++ b/src/coreutils-vdir.c
@@ -20,7 +20,7 @@
#include "system.h"
#include "ls.h"
-/* Ensure that the main for ls is declared even if the tool is not being build
+/* Ensure that the main for ls is declared even if the tool is not being built
in this single-binary. */
int _single_binary_main_ls (int argc, char** argv) ATTRIBUTE_NORETURN;
int _single_binary_main_vdir (int argc, char** argv) ATTRIBUTE_NORETURN;
diff --git a/src/coreutils.c b/src/coreutils.c
index d836782..f54f85d 100644
--- a/src/coreutils.c
+++ b/src/coreutils.c
@@ -31,7 +31,7 @@
/* 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
- byt gen-single-binary.sh). */
+ by gen-single-binary.sh). */
#define SINGLE_BINARY_PROGRAM(prog_name_str, main_name) \
int _single_binary_main_##main_name (int, char**) ATTRIBUTE_NORETURN;
#include "coreutils.h"
@@ -110,8 +110,8 @@ launch_program (const char *prog_name, int prog_argc, char **prog_argv)
prctl (PR_SET_NAME, prog_name);
#endif
#if HAVE_PRCTL && defined PR_SET_MM_ARG_START
- /* Shift the begining of the command line to prog_argv[0] (if set) so
- /proc/pid/cmdline reflects the reight value. */
+ /* Shift the beginning of the command line to prog_argv[0] (if set) so
+ /proc/pid/cmdline reflects the right value. */
prctl (PR_SET_MM_ARG_START, prog_argv[0]);
#endif
