[Openvpn-devel,1/1] configure.ac: replace set with env

Message ID 20200106080426.40701-1-list@eworm.de
State Rejected
Headers show
Series [Openvpn-devel,1/1] configure.ac: replace set with env | expand

Commit Message

Christian Hesse Jan. 5, 2020, 9:04 p.m. UTC
From: Christian Hesse <mail@eworm.de>

The shell builtin `set` produces different output for different shells:

bash$ set | grep '^TERM='
TERM=xterm
dash$ set | grep '^TERM='
TERM='xterm'

This may break reproducible builds depending on what shell is used.

Let's replace `set` with `env`, which is a real command and always
produces identical output.

Signed-off-by: Christian Hesse <mail@eworm.de>
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Tom Yan Jan. 5, 2020, 9:48 p.m. UTC | #1
How about printenv (without grep)?
Christian Hesse Jan. 5, 2020, 9:55 p.m. UTC | #2
Tom Yan <tom.ty89@gmail.com> on Mon, 2020/01/06 08:48:
> How about printenv (without grep)?

The variables are not known in advance. This needs to match all variables
starting with "enable_" and "with_".
Gert Doering Jan. 5, 2020, 10:01 p.m. UTC | #3
Hi,

On Mon, Jan 06, 2020 at 08:48:00AM +0000, Tom Yan wrote:
> How about printenv (without grep)?

Portability.  

"env |grep" is POSIX standardized so should work everywhere.

printenv is a BSD invention, which not all unix variants are required
to have.

(Besides, printenv on its own cannot do "give me all environment variables
that start with 'with_'", which is exactly what we want here... so you'd
end up with "printenv | grep", which is less portable and more characters)

gert
David Sommerseth June 23, 2022, 8:39 a.m. UTC | #4
On 06/01/2020 09:04, Christian Hesse wrote:
> From: Christian Hesse <mail@eworm.de>
> 
> The shell builtin `set` produces different output for different shells:
> 
> bash$ set | grep '^TERM='
> TERM=xterm
> dash$ set | grep '^TERM='
> TERM='xterm'
> 
> This may break reproducible builds depending on what shell is used.
> 
> Let's replace `set` with `env`, which is a real command and always
> produces identical output.

Hi,

Spotted this one now, lingering for way too long.  And it looked 
trivial.  Except it doesn't give the expected outcome on my RHEL-8 
machine.  The CONFIGURE_DEFINES macro in config.h ends up empty. 
Reverting this patch alone, and it comes back again.

So, I'm sorry, I can't ack this one.

Patch

diff --git a/configure.ac b/configure.ac
index a47e0a06..f13ff7b6 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1326,7 +1326,7 @@  if test "${enable_async_push}" = "yes"; then
 	)
 fi
 
-CONFIGURE_DEFINES="`set | grep '^enable_.*=' ; set | grep '^with_.*='`"
+CONFIGURE_DEFINES="`env | grep '^enable_.*=' ; env | grep '^with_.*='`"
 AC_DEFINE_UNQUOTED([CONFIGURE_DEFINES], ["`echo ${CONFIGURE_DEFINES}`"], [Configuration settings])
 
 TAP_WIN_COMPONENT_ID="PRODUCT_TAP_WIN_COMPONENT_ID"