| Message ID | 20220801172620.34547-1-mark@mentovai.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | [libmnl] build: doc: refer to bash as bash, not /bin/bash | expand |
Hi Mark, On Mon, Aug 01, 2022 at 01:26:20PM -0400, Mark Mentovai wrote: > This locates bash according to its presence in the PATH, not at a > hard-coded path which may not exist or may not be the most suitable bash > to use. > > Signed-off-by: Mark Mentovai <mark@mentovai.com> > --- > doxygen/Makefile.am | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/doxygen/Makefile.am b/doxygen/Makefile.am > index 29078dee122a..189a233f3760 100644 > --- a/doxygen/Makefile.am > +++ b/doxygen/Makefile.am > @@ -21,7 +21,7 @@ doxyfile.stamp: $(doc_srcs) Makefile.am > # The command has to be a single line so the functions work > # and so `make` gives all lines to `bash -c` > # (hence ";\" at the end of every line but the last). > - /bin/bash -p -c 'declare -A renamed_page;\ > + bash -p -c 'declare -A renamed_page;\ > main(){ set -e; cd man/man3; rm -f _*;\ > count_real_pages;\ > rename_real_pages;\ > -- > 2.37.1 > I would not apply this patch unless it's actually necessary for some distribution. If you have discovered a distribution where /bin/bash doesn't work, please let us know. Scripts that start "#!/bin/bash" are not uncommon, and Netfilter already has a couple of these, in libnetfilter_queue and libnetfilter_log. I somehow omitted to update libmnl to replace the cumbersome embedded script in Makefile.am with a stand-alone script, but you've reminded me. Cheers ... Duncan.
Duncan Roe wrote: > On Mon, Aug 01, 2022 at 01:26:20PM -0400, Mark Mentovai wrote: >> This locates bash according to its presence in the PATH, not at a >> hard-coded path which may not exist or may not be the most suitable bash >> to use. >> >> Signed-off-by: Mark Mentovai <mark@mentovai.com> >> --- >> doxygen/Makefile.am | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/doxygen/Makefile.am b/doxygen/Makefile.am >> index 29078dee122a..189a233f3760 100644 >> --- a/doxygen/Makefile.am >> +++ b/doxygen/Makefile.am >> @@ -21,7 +21,7 @@ doxyfile.stamp: $(doc_srcs) Makefile.am >> # The command has to be a single line so the functions work >> # and so `make` gives all lines to `bash -c` >> # (hence ";\" at the end of every line but the last). >> - /bin/bash -p -c 'declare -A renamed_page;\ >> + bash -p -c 'declare -A renamed_page;\ >> main(){ set -e; cd man/man3; rm -f _*;\ >> count_real_pages;\ >> rename_real_pages;\ >> -- >> 2.37.1 >> > I would not apply this patch unless it's actually necessary for some > distribution. > > If you have discovered a distribution where /bin/bash doesn't work, please let > us know. > > Scripts that start "#!/bin/bash" are not uncommon, and Netfilter already has a > couple of these, in libnetfilter_queue and libnetfilter_log. > > I somehow omitted to update libmnl to replace the cumbersome embedded script in > Makefile.am with a stand-alone script, but you've reminded me. > > Cheers ... Duncan. The context here is in OpenWrt, https://github.com/openwrt/openwrt/commit/beeb49740bb4. The use of /bin/bash is a problem during a cross build of libmnl, with a build system running macOS or BSD. /bin/bash on macOS is an unsuitably old version, and the OpenWrt build ensures that a recent bash is available in PATH. BSD derivatives tend not to have /bin/bash at all, although bash may be present elsewhere in PATH. Again, the OpenWrt build ensures this. I would not expect the same treatment to be strictly necessary for scripts like libnetfilter_queue or libnetfilter_log, which run on the target system, but the reliance on /bin/bash is a problem for cross builds and in particular non-Linux build systems. Considering that these cross builds are otherwise perfectly clean given an appropriate toolchain, it seems unnecessary to leave them broken for something like this, when a simple reliance on locating bash via PATH ought to suffice for everyone. Mark
On Mon, Aug 01, 2022 at 01:26:20PM -0400, Mark Mentovai wrote: > This locates bash according to its presence in the PATH, not at a > hard-coded path which may not exist or may not be the most suitable bash > to use. > > Signed-off-by: Mark Mentovai <mark@mentovai.com> > --- > doxygen/Makefile.am | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/doxygen/Makefile.am b/doxygen/Makefile.am > index 29078dee122a..189a233f3760 100644 > --- a/doxygen/Makefile.am > +++ b/doxygen/Makefile.am > @@ -21,7 +21,7 @@ doxyfile.stamp: $(doc_srcs) Makefile.am > # The command has to be a single line so the functions work > # and so `make` gives all lines to `bash -c` > # (hence ";\" at the end of every line but the last). > - /bin/bash -p -c 'declare -A renamed_page;\ > + bash -p -c 'declare -A renamed_page;\ > main(){ set -e; cd man/man3; rm -f _*;\ > count_real_pages;\ > rename_real_pages;\ > -- > 2.37.1 > Acked-by: Duncan Roe <duncan_roe@optusnet.com.au>
On Tue, Aug 02, 2022 at 02:14:29PM -0400, Mark Mentovai wrote: [...] > > The context here is in OpenWrt, > https://github.com/openwrt/openwrt/commit/beeb49740bb4. The use of /bin/bash > is a problem during a cross build of libmnl, with a build system running > macOS or BSD. /bin/bash on macOS is an unsuitably old version, and the > OpenWrt build ensures that a recent bash is available in PATH. BSD > derivatives tend not to have /bin/bash at all, although bash may be present > elsewhere in PATH. Again, the OpenWrt build ensures this. > > I would not expect the same treatment to be strictly necessary for scripts > like libnetfilter_queue or libnetfilter_log, which run on the target system, > but the reliance on /bin/bash is a problem for cross builds and in > particular non-Linux build systems. Considering that these cross builds are > otherwise perfectly clean given an appropriate toolchain, it seems > unnecessary to leave them broken for something like this, when a simple > reliance on locating bash via PATH ought to suffice for everyone. > > Mark Good enough for me - I acked the original patch. Cheers ... Duncan.
diff --git a/doxygen/Makefile.am b/doxygen/Makefile.am index 29078dee122a..189a233f3760 100644 --- a/doxygen/Makefile.am +++ b/doxygen/Makefile.am @@ -21,7 +21,7 @@ doxyfile.stamp: $(doc_srcs) Makefile.am # The command has to be a single line so the functions work # and so `make` gives all lines to `bash -c` # (hence ";\" at the end of every line but the last). - /bin/bash -p -c 'declare -A renamed_page;\ + bash -p -c 'declare -A renamed_page;\ main(){ set -e; cd man/man3; rm -f _*;\ count_real_pages;\ rename_real_pages;\
This locates bash according to its presence in the PATH, not at a hard-coded path which may not exist or may not be the most suitable bash to use. Signed-off-by: Mark Mentovai <mark@mentovai.com> --- doxygen/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)