[Buildroot] [PATCH 1/1] package/ntp: remove host shell check

[Buildroot] [PATCH 1/1] package/ntp: remove host shell check

[James Byrne]

This adds a patch which removes the autoconf check for which shell is
installed on the host and fixes the shell as '/bin/sh'. Since we are
cross-compiling, we cannot assume that the target uses the same shell.
Also, it can prevent builds being reproducible because a different host
environment will result in a different target binary.

Signed-off-by: James Byrne <james.byrne@origamienergy.com>
---
 package/ntp/0003-force-sh.patch | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 package/ntp/0003-force-sh.patch

diff --git a/package/ntp/0003-force-sh.patch b/package/ntp/0003-force-sh.patch
new file mode 100644
index 0000000000..4c2628e04a
--- /dev/null
+++ b/package/ntp/0003-force-sh.patch
@@ -0,0 +1,30 @@
+We're cross compiling, don't use the presence of shells on the host to decide
+anything about the target...
+
+diff --git a/sntp/libopts/m4/libopts.m4 b/sntp/libopts/m4/libopts.m4
+--- a/sntp/libopts/m4/libopts.m4
++++ b/sntp/libopts/m4/libopts.m4
+@@ -112,22 +112,7 @@
+   AC_CHECK_FUNCS([mmap canonicalize_file_name snprintf strdup strchr \
+                  strrchr strsignal fchmod fstat chmod])
+   AC_PROG_SED
+-  [while :
+-  do
+-      POSIX_SHELL=`which bash`
+-      test -x "$POSIX_SHELL" && break
+-      POSIX_SHELL=`which dash`
+-      test -x "$POSIX_SHELL" && break
+-      POSIX_SHELL=/usr/xpg4/bin/sh
+-      test -x "$POSIX_SHELL" && break
+-      POSIX_SHELL=`/bin/sh -c '
+-          exec 2>/dev/null
+-          if ! true ; then exit 1 ; fi
+-          echo /bin/sh'`
+-      test -x "$POSIX_SHELL" && break
+-      ]AC_MSG_ERROR([cannot locate a working POSIX shell])[
+-  done]
+-  AC_DEFINE_UNQUOTED([POSIX_SHELL], ["${POSIX_SHELL}"],
++  AC_DEFINE_UNQUOTED([POSIX_SHELL], ["/bin/sh"],
+            [define to a working POSIX compliant shell])
+   AC_SUBST([POSIX_SHELL])
+ ])
-- 
2.24.0

[Buildroot] [PATCH 1/1] package/ntp: remove host shell check

[Yann E. MORIN]

James, All,

On 2019-11-20 15:55 +0000, James Byrne spake thusly:
> This adds a patch which removes the autoconf check for which shell is
> installed on the host and fixes the shell as '/bin/sh'. Since we are
> cross-compiling, we cannot assume that the target uses the same shell.
> Also, it can prevent builds being reproducible because a different host
> environment will result in a different target binary.
> 
> Signed-off-by: James Byrne <james.byrne@origamienergy.com>
> ---
>  package/ntp/0003-force-sh.patch | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>  create mode 100644 package/ntp/0003-force-sh.patch
> 
> diff --git a/package/ntp/0003-force-sh.patch b/package/ntp/0003-force-sh.patch
> new file mode 100644
> index 0000000000..4c2628e04a
> --- /dev/null
> +++ b/package/ntp/0003-force-sh.patch
> @@ -0,0 +1,30 @@
> +We're cross compiling, don't use the presence of shells on the host to decide
> +anything about the target...
> +
> +diff --git a/sntp/libopts/m4/libopts.m4 b/sntp/libopts/m4/libopts.m4
> +--- a/sntp/libopts/m4/libopts.m4
> ++++ b/sntp/libopts/m4/libopts.m4
> +@@ -112,22 +112,7 @@
> +   AC_CHECK_FUNCS([mmap canonicalize_file_name snprintf strdup strchr \
> +                  strrchr strsignal fchmod fstat chmod])
> +   AC_PROG_SED
> +-  [while :
> +-  do
> +-      POSIX_SHELL=`which bash`
> +-      test -x "$POSIX_SHELL" && break
> +-      POSIX_SHELL=`which dash`
> +-      test -x "$POSIX_SHELL" && break
> +-      POSIX_SHELL=/usr/xpg4/bin/sh
> +-      test -x "$POSIX_SHELL" && break
> +-      POSIX_SHELL=`/bin/sh -c '
> +-          exec 2>/dev/null
> +-          if ! true ; then exit 1 ; fi
> +-          echo /bin/sh'`
> +-      test -x "$POSIX_SHELL" && break
> +-      ]AC_MSG_ERROR([cannot locate a working POSIX shell])[
> +-  done]
> +-  AC_DEFINE_UNQUOTED([POSIX_SHELL], ["${POSIX_SHELL}"],
> ++  AC_DEFINE_UNQUOTED([POSIX_SHELL], ["/bin/sh"],

This is not nice, as this patch can't be upstreamed, and we'll have tio
live with it for the eons to come.

What about using AC_CACHE_CHECK [0] to do the check, which can be
overriden on the command line. For example:

    [
    AC_CACHE_CHECK([for a posix-compliant shell], [ntp_ac_cv_posix_shell],
    [while :
    do
        blablabla
    done
    ])
    AC_DEFINE_UNQUOTED([POSIX_SHELL], ["${ntp_ac_cv_posix_shell}"],
      [define to a working POSIX compliant shell])
    ]

And then in ntp.mk we could do (in addition to the existing one, of
course):

    NTP_CONF_ENV = ntp_ac_cv_posix_shell=/bin/sh

And such a patch will probably be more acceptable for upstream.

[0] https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/autoconf.html#Caching-Results

Regards,
Yann E. MORIN.

> +            [define to a working POSIX compliant shell])
> +   AC_SUBST([POSIX_SHELL])
> + ])
> -- 
> 2.24.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/1] package/ntp: remove host shell check

[James Byrne]

Hi Yann,

On 23/11/2019 10:30, Yann E. MORIN wrote:
> This is not nice, as this patch can't be upstreamed, and we'll have tio
> live with it for the eons to come.
> 
> What about using AC_CACHE_CHECK [0] to do the check, which can be
> overriden on the command line. For example:
> 
>      [
>      AC_CACHE_CHECK([for a posix-compliant shell], [ntp_ac_cv_posix_shell],
>      [while :
>      do
>          blablabla
>      done
>      ])
>      AC_DEFINE_UNQUOTED([POSIX_SHELL], ["${ntp_ac_cv_posix_shell}"],
>        [define to a working POSIX compliant shell])
>      ]
> 
> And then in ntp.mk we could do (in addition to the existing one, of
> course):
> 
>      NTP_CONF_ENV = ntp_ac_cv_posix_shell=/bin/sh
> 
> And such a patch will probably be more acceptable for upstream.

Your comments prompted me to do some more digging, and it turns out that 
the issue has been addressed in a later version of AutoGen, which is 
where the issue comes from - see the change to libopts.def in:

https://sourceforge.net/p/autogen/code/ci/e51f2a781500236b2ba92a4ccd854771a7c28b61/

I will resubmit a V2 patch based on pulling in this change, then if the 
upstream ntp package is updated to use a newer version of AutoGen at 
some point in the future, the patch can go away.

Regards,

James

[Buildroot] [PATCH v2] package/ntp: override host shell check

[James Byrne]

Add a patch from the upstream AutoGen package that allows POSIX_SHELL to
be taken from the environment, then define that to be '/bin/sh'.

Since we are cross-compiling, the original behaviour of detecting the
host shell is not useful as we cannot assume that the target uses the
same shell, and it can prevent builds being reproducible because a
different host environment will result in a different target binary.

Signed-off-by: James Byrne <james.byrne@origamienergy.com>

---
Changes v1 -> v2:
  - Simplify by using upstream AutoGen change

Signed-off-by: James Byrne <james.byrne@origamienergy.com>
---
 package/ntp/0003-override-shell.patch | 14 ++++++++++++++
 package/ntp/ntp.mk                    |  2 +-
 2 files changed, 15 insertions(+), 1 deletion(-)
 create mode 100644 package/ntp/0003-override-shell.patch

diff --git a/package/ntp/0003-override-shell.patch b/package/ntp/0003-override-shell.patch
new file mode 100644
index 0000000000..823dedf028
--- /dev/null
+++ b/package/ntp/0003-override-shell.patch
@@ -0,0 +1,14 @@
+Pull in fix from upstream AutoGen package to accept POSIX_SHELL from the
+environment during the configure step
+
+diff --git a/sntp/libopts/m4/libopts.m4 b/sntp/libopts/m4/libopts.m4
+--- a/sntp/libopts/m4/libopts.m4
++++ b/sntp/libopts/m4/libopts.m4
+@@ -114,6 +114,7 @@
+   AC_PROG_SED
+   [while :
+   do
++      test -x "$POSIX_SHELL" && break
+       POSIX_SHELL=`which bash`
+       test -x "$POSIX_SHELL" && break
+       POSIX_SHELL=`which dash`
diff --git a/package/ntp/ntp.mk b/package/ntp/ntp.mk
index d53fcc5d0b..56050f4fe1 100644
--- a/package/ntp/ntp.mk
+++ b/package/ntp/ntp.mk
@@ -10,7 +10,7 @@ NTP_SITE = https://www.eecis.udel.edu/~ntp/ntp_spool/ntp4/ntp-$(NTP_VERSION_MAJO
 NTP_DEPENDENCIES = host-pkgconf libevent
 NTP_LICENSE = NTP
 NTP_LICENSE_FILES = COPYRIGHT
-NTP_CONF_ENV = ac_cv_lib_md5_MD5Init=no
+NTP_CONF_ENV = ac_cv_lib_md5_MD5Init=no POSIX_SHELL=/bin/sh
 NTP_CONF_OPTS = \
 	--with-shared \
 	--program-transform-name=s,,, \
-- 
2.24.0

[Buildroot] [PATCH v2] package/ntp: override host shell check

[Thomas Petazzoni]

On Tue, 26 Nov 2019 11:33:01 +0000
James Byrne <james.byrne@origamienergy.com> wrote:

> Add a patch from the upstream AutoGen package that allows POSIX_SHELL to
> be taken from the environment, then define that to be '/bin/sh'.
> 
> Since we are cross-compiling, the original behaviour of detecting the
> host shell is not useful as we cannot assume that the target uses the
> same shell, and it can prevent builds being reproducible because a
> different host environment will result in a different target binary.
> 
> Signed-off-by: James Byrne <james.byrne@origamienergy.com>
> 
> ---
> Changes v1 -> v2:
>   - Simplify by using upstream AutoGen change

Applied to master, thanks. Could you talk with the upstream NTP
developers to get this change merged ?

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v2] package/ntp: override host shell check

[Peter Korsgaard]

>>>>> "James" == James Byrne <james.byrne@origamienergy.com> writes:

 > Add a patch from the upstream AutoGen package that allows POSIX_SHELL to
 > be taken from the environment, then define that to be '/bin/sh'.

 > Since we are cross-compiling, the original behaviour of detecting the
 > host shell is not useful as we cannot assume that the target uses the
 > same shell, and it can prevent builds being reproducible because a
 > different host environment will result in a different target binary.

 > Signed-off-by: James Byrne <james.byrne@origamienergy.com>

 > ---
 > Changes v1 -> v2:
 >   - Simplify by using upstream AutoGen change

Committed to 2019.02.x and 2019.11.x, thanks.

-- 
Bye, Peter Korsgaard