Skip to content

Commit fd813bf

Browse files
committed
Merge bitcoin/bitcoin#33002: ci: Only pass documented env vars
3333d3f ci: Only pass documented env vars (MarcoFalke) Pull request description: The CI currently inherits almost all env vars from the host. This was problematic in the past and causing non-determinism, e.g. the fix in commit fa12558. It is still problematic today, see e.g. bitcoin/bitcoin#31349 (comment), or bitcoin/bitcoin#32935 This fixes bitcoin/bitcoin#32935 by only passing env vars documented in `./ci/test/00_setup_env.sh`. Implementation-wise, instead of cramming the python code into the `python -c ""` statement, just start a fresh py file, which is easier to handle. ACKs for top commit: willcl-ark: ACK 3333d3f Tree-SHA512: f922e481a844128d7fbf773563278a3992c178ead60a3050eceb9ded2aad979afc815a5cbdb9f68494493c5d8d942cdd1111c21e32a5746d19505b87745cb84a
2 parents 9617a42 + 3333d3f commit fd813bf

File tree

3 files changed

+50
-7
lines changed

3 files changed

+50
-7
lines changed

ci/test/02_run_container.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
#!/usr/bin/env python3
2+
# Copyright (c) The Bitcoin Core developers
3+
# Distributed under the MIT software license, see the accompanying
4+
# file COPYING or https://opensource.org/license/mit/.
5+
6+
import os
7+
import shlex
8+
import subprocess
9+
import sys
10+
11+
12+
def run(cmd, **kwargs):
13+
print("+ " + shlex.join(cmd), flush=True)
14+
try:
15+
return subprocess.run(cmd, check=True, **kwargs)
16+
except Exception as e:
17+
sys.exit(e)
18+
19+
20+
def main():
21+
print("Export only allowed settings:")
22+
settings = run(
23+
["bash", "-c", "grep export ./ci/test/00_setup_env*.sh"],
24+
stdout=subprocess.PIPE,
25+
text=True,
26+
encoding="utf8",
27+
).stdout.splitlines()
28+
settings = set(l.split("=")[0].split("export ")[1] for l in settings)
29+
# Add this one manually, because it is the only one set inside the
30+
# container that also allows external overwrites
31+
settings.add("BASE_BUILD_DIR")
32+
33+
# Append $USER to /tmp/env to support multi-user systems and $CONTAINER_NAME
34+
# to allow support starting multiple runs simultaneously by the same user.
35+
env_file = "/tmp/env-{u}-{c}".format(
36+
u=os.getenv("USER"),
37+
c=os.getenv("CONTAINER_NAME"),
38+
)
39+
with open(env_file, "w", encoding="utf8") as file:
40+
for k, v in os.environ.items():
41+
if k in settings:
42+
file.write(f"{k}={v}\n")
43+
run(["cat", env_file])
44+
45+
run(["./ci/test/02_run_container.sh"]) # run the remainder
46+
47+
48+
if __name__ == "__main__":
49+
main()

ci/test/02_run_container.sh

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,6 @@ export CI_IMAGE_LABEL="bitcoin-ci-test"
1010
set -o errexit -o pipefail -o xtrace
1111

1212
if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then
13-
# Export all env vars to avoid missing some.
14-
# Though, exclude those with newlines to avoid parsing problems.
15-
python3 -c 'import os; [print(f"{key}={value}") for key, value in os.environ.items() if "\n" not in value and "HOME" != key and "PATH" != key and "USER" != key]' | tee "/tmp/env-$USER-$CONTAINER_NAME"
16-
1713
# Env vars during the build can not be changed. For example, a modified
1814
# $MAKEJOBS is ignored in the build process. Use --cpuset-cpus as an
1915
# approximation to respect $MAKEJOBS somewhat, if cpuset is available.
@@ -118,8 +114,6 @@ if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then
118114
# When detecting podman-docker, `--external` should be added.
119115
docker image prune --force --filter "label=$CI_IMAGE_LABEL"
120116

121-
# Append $USER to /tmp/env to support multi-user systems and $CONTAINER_NAME
122-
# to allow support starting multiple runs simultaneously by the same user.
123117
# shellcheck disable=SC2086
124118
CI_CONTAINER_ID=$(docker run --cap-add LINUX_IMMUTABLE $CI_CONTAINER_CAP --rm --interactive --detach --tty \
125119
--mount "type=bind,src=$BASE_READ_ONLY_DIR,dst=$BASE_READ_ONLY_DIR,readonly" \

ci/test_run_all.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,4 @@ export LC_ALL=C.UTF-8
88

99
set -o errexit; source ./ci/test/00_setup_env.sh
1010
set -o errexit
11-
"./ci/test/02_run_container.sh"
11+
"./ci/test/02_run_container.py"

0 commit comments

Comments
 (0)