ninja-build / ninja

a small build system with a focus on speed
https://ninja-build.org/
Apache License 2.0
11.17k stars 1.59k forks source link

Environment variable with a hyphen is dropped in ubuntu #2288

Open mserdarsanli opened 1 year ago

mserdarsanli commented 1 year ago

Feel free to close this as invalid, the bug is likely not in ninja, as it looks like ninja simply does a posix_spawn to launch a process without modifying the environment. Though interestingly I could not reproduce the issue without ninja (tried my own wrapper that does posix_spawn etc), so opening it here as someone else might have experienced the same issue.

populate buid.ninja with

rule environ
    command = env

build env.txt: environ

Run ninja with command:

env - FOO_bar=1 FOO_baz-baz=2 ninja

Expected output from env should include both FOO_bar and FOO_baz-baz variables, but on some systems you will only see the variable without hyphen in the name, those with hyphen are dropped somewhere.

So far I've tested: Ubuntu: Buggy Linux Mint: Buggy Fedora: Works Fine Arch: works Fine

I've also debugged by making ninja printing environ just before doing posix_spawn and I can see the variable FOO_baz-baz there, it just isn't visible in the launched process.

My guess is that ubuntu glibc does something wrong somewhere, but I couldn't find it

eli-schwartz commented 1 year ago

I can simplify the problem for you:

$ env - FOO_bar=1 FOO_baz-baz=2 bash -c env
FOO_baz-baz=2
PWD=/home/eschwartz/git/ninja
SHLVL=0
FOO_bar=1
_=/usr/bin/env
$ env - FOO_bar=1 FOO_baz-baz=2 dash -c env
PWD=/home/eschwartz/git/ninja
FOO_bar=1
$ env - FOO_bar=1 FOO_baz-baz=2 busybox sh -c env
SHLVL=1
FOO_baz-baz=2
PWD=/home/eschwartz/git/ninja
FOO_bar=1

Command lines in ninja are executed using sh -c, shells are not required to support environment variables whose names contain a hyphen -- that's not a portable name. Ninja preserves the variable just fine, until the shell itself drops it. Well, some shells anyway.

mserdarsanli commented 1 year ago

Thanks, that explains it. I my system sh is dash so it strips those variables. Ninja seems to hardcode /bin/sh path there so it can't simply be worked around.

shells are not required to support environment variables whose names contain a hyphen -- that's not a portable name.

Is this really the case? I can't find much about it. Looking at dash commit for the change, it seems to be added to fix the export use case, while bash instead keeps such env vars and not print them on export.

eli-schwartz commented 1 year ago

POSIX explicitly states that:

Bash is conformant to the specification; when faced with non-portable environment variables that it doesn't understand and cannot make into shell variables, it deletes them before forking subprocesses and proceeds normally.

Dash is conformant the same way.

BusyBox sh is also conformant, but a different way; it chooses to preserve them for subprocesses, as it is permitted to do.