From 7de7582fbf382f6dc0ae2b937b5ad6aef9c082c7 Mon Sep 17 00:00:00 2001 From: Fabio Valentini Date: Sep 25 2023 20:57:56 +0000 Subject: move scripts that use pipes into subshells and set -o pipefail --- diff --git a/macros.d/macros.cargo b/macros.d/macros.cargo index 7da260d..4bfc519 100644 --- a/macros.d/macros.cargo +++ b/macros.d/macros.cargo @@ -55,8 +55,10 @@ # -v - use vendored sources from instead of # %%cargo_registry # -N - Don't set up any registry. Only set up the build configuration. -%cargo_prep(v:N) %{-v:%{-N:%{error:-v and -N are mutually exclusive!}}}(\ -set -eu\ +%cargo_prep(v:N)\ +%{-v:%{-N:%{error:-v and -N are mutually exclusive!}}}\ +(\ +set -euo pipefail\ %{__mkdir} -p .cargo\ cat > .cargo/config << EOF\ [build]\ @@ -101,12 +103,12 @@ EOF}}\ # __cargo_parse_opts: function-like macro which parses common flags into the # equivalent command-line flags for cargo %__cargo_parse_opts(naf:) %{shrink:\ -%{-n:%{-a:%{error:Can't specify both -n and -a}}} \ -%{-f:%{-a:%{error:Can't specify both -f(%{-f*}) and -a}}} \ - %{-n:--no-default-features} \ - %{-a:--all-features} \ - %{-f:--features %{-f*}} \ - %{nil} + %{-n:%{-a:%{error:Can't specify both -n and -a}}} \ + %{-f:%{-a:%{error:Can't specify both -f(%{-f*}) and -a}}} \ + %{-n:--no-default-features} \ + %{-a:--all-features} \ + %{-f:--features %{-f*}} \ + %{nil} \ } # cargo_generate_buildrequires: prints BuildRequires that are needed for @@ -118,13 +120,13 @@ EOF}}\ %{__cargo_to_rpm} --path Cargo.toml buildrequires %{__cargo_parse_opts %{-n} %{-a} %{-f:-f%{-f*}}} %{-t:--with-check} %{!?-t:%{?with_check:--with-check}} # cargo_build: builds the crate with cargo with the specified feature flags -%cargo_build(naf:) \ -%{shrink:\ - %{__cargo} build \ - %{__cargo_common_opts} \ - --release \ - %{__cargo_parse_opts %{-n} %{-a} %{-f:-f%{-f*}}} \ - %* \ +%cargo_build(naf:)\ +%{shrink: \ + %{__cargo} build \ + %{__cargo_common_opts} \ + --release \ + %{__cargo_parse_opts %{-n} %{-a} %{-f:-f%{-f*}}} \ + %* \ } # cargo_test: runs the test suite with cargo with the specified feature flags @@ -134,14 +136,14 @@ EOF}}\ # macro argument parsing and "cargo test" argument parsing need to be bypassed, # i.e. "%%cargo_test -- -- --skip foo" for skipping all tests with names that # match "foo". -%cargo_test(naf:) \ -%{shrink:\ - %{__cargo} test \ - %{__cargo_common_opts} \ - --release \ - --no-fail-fast \ - %{__cargo_parse_opts %{-n} %{-a} %{-f:-f%{-f*}}} \ - %* \ +%cargo_test(naf:)\ +%{shrink: \ + %{__cargo} test \ + %{__cargo_common_opts} \ + --release \ + --no-fail-fast \ + %{__cargo_parse_opts %{-n} %{-a} %{-f:-f%{-f*}}} \ + %* \ } # cargo_install: install files into the buildroot @@ -157,8 +159,9 @@ EOF}}\ # "$CARGO_HOME/.crates.toml" file, which is used to keep track of which version # of a specific binary has been installed, but which conflicts between builds # of different Rust applications and is not needed when building RPM packages. -%cargo_install(t:naf:) ( \ -set -eu \ +%cargo_install(t:naf:)\ +(\ +set -euo pipefail \ if %{__cargo_is_lib} ; then \ CRATE_NAME=$(%{__cargo_to_rpm} --path Cargo.toml name) \ CRATE_VERSION=$(%{__cargo_to_rpm} --path Cargo.toml version) \ @@ -197,7 +200,9 @@ fi \ # The "cargo tree" command called by this macro will fail if there are missing # (optional) dependencies. %cargo_license(naf:)\ -%{shrink:\ +(\ +set -euo pipefail\ +%{shrink: \ %{__cargo} tree \ -Z avoid-dev-deps \ --workspace \ @@ -209,8 +214,9 @@ fi \ --prefix none \ --format "{l}: {p}" \ | sed -e "s: ($(pwd)[^)]*)::g" -e "s: / :/:g" -e "s:/: OR :g" \ - | sort -u -} + | sort -u \ +}\ +) # cargo_license_summary: print license summary for all crate dependencies # @@ -219,7 +225,9 @@ fi \ # in the dependency tree. This is useful for determining the correct License # tag for packages that contain compiled Rust binaries. %cargo_license_summary(naf:)\ -%{shrink:\ +(\ +set -euo pipefail\ +%{shrink: \ %{__cargo} tree \ -Z avoid-dev-deps \ --workspace \ @@ -232,7 +240,8 @@ fi \ --format "# {l}" \ | sed -e "s: / :/:g" -e "s:/: OR :g" \ | sort -u \ -} +}\ +) # cargo_vendor_manifest: write list of vendored crates and their versions # @@ -240,7 +249,9 @@ fi \ # that determines which crates are included when running "cargo vendor". # The results are written to "cargo-vendor.txt". %cargo_vendor_manifest()\ -%{shrink:\ +(\ +set -euo pipefail\ +%{shrink: \ %{__cargo} tree \ --workspace \ --offline \ @@ -254,5 +265,6 @@ fi \ | sed -e "s: (proc-macro)::" \ | sort -u \ > cargo-vendor.txt \ -} +}\ +) diff --git a/tests/test_macros_cargo.py b/tests/test_macros_cargo.py index d3438e3..a7916f6 100644 --- a/tests/test_macros_cargo.py +++ b/tests/test_macros_cargo.py @@ -54,7 +54,7 @@ def test_cargo_prep(evaluater): assert evaluater("%cargo_prep")[0].splitlines() == [ "(", - "set -eu", + "set -euo pipefail", "/usr/bin/mkdir -p .cargo", "cat > .cargo/config << EOF", "[build]", @@ -99,7 +99,7 @@ def test_cargo_prep_vendor(evaluater): assert evaluater("%cargo_prep -v vendor")[0].splitlines() == [ "(", - "set -eu", + "set -euo pipefail", "/usr/bin/mkdir -p .cargo", "cat > .cargo/config << EOF", "[build]", @@ -142,7 +142,7 @@ def test_cargo_prep_no_replacement(evaluater): assert evaluater("%cargo_prep -N")[0].splitlines() == [ "(", - "set -eu", + "set -euo pipefail", "/usr/bin/mkdir -p .cargo", "cat > .cargo/config << EOF", "[build]", @@ -247,7 +247,7 @@ def test_cargo_install(evaluater): assert [line.rstrip() for line in evaluater("%cargo_install")[0].splitlines()] == [ f"(", - f"set -eu", + f"set -euo pipefail", f"if {cargo_is_lib} ; then", f" CRATE_NAME=$({cargo_to_rpm} --path Cargo.toml name)", f" CRATE_VERSION=$({cargo_to_rpm} --path Cargo.toml version)", @@ -270,60 +270,75 @@ def test_cargo_install(evaluater): def test_cargo_license(evaluater): cargo = evaluater("%__cargo")[0] - assert evaluater("%cargo_license")[0] == " ".join( - [ - f"{cargo} tree", - "-Z avoid-dev-deps", - "--workspace", - "--offline", - "--edges no-build,no-dev,no-proc-macro", - "--no-dedupe", - "--target all", - "--prefix none", - '--format "{l}: {p}"', - '| sed -e "s: ($(pwd)[^)]*)::g" -e "s: / :/:g" -e "s:/: OR :g"', - "| sort -u", - ] - ) + assert evaluater("%cargo_license")[0].splitlines() == [ + "(", + "set -euo pipefail", + " ".join( + [ + f"{cargo} tree", + "-Z avoid-dev-deps", + "--workspace", + "--offline", + "--edges no-build,no-dev,no-proc-macro", + "--no-dedupe", + "--target all", + "--prefix none", + '--format "{l}: {p}"', + '| sed -e "s: ($(pwd)[^)]*)::g" -e "s: / :/:g" -e "s:/: OR :g"', + "| sort -u", + ] + ), + ")", + ] def test_cargo_license_summary(evaluater): cargo = evaluater("%__cargo")[0] - assert evaluater("%cargo_license_summary")[0] == " ".join( - [ - f"{cargo} tree", - "-Z avoid-dev-deps", - "--workspace", - "--offline", - "--edges no-build,no-dev,no-proc-macro", - "--no-dedupe", - "--target all", - "--prefix none", - '--format "# {l}"', - '| sed -e "s: / :/:g" -e "s:/: OR :g"', - "| sort -u", - ] - ) + assert evaluater("%cargo_license_summary")[0].splitlines() == [ + "(", + "set -euo pipefail", + " ".join( + [ + f"{cargo} tree", + "-Z avoid-dev-deps", + "--workspace", + "--offline", + "--edges no-build,no-dev,no-proc-macro", + "--no-dedupe", + "--target all", + "--prefix none", + '--format "# {l}"', + '| sed -e "s: / :/:g" -e "s:/: OR :g"', + "| sort -u", + ] + ), + ")", + ] def test_cargo_vendor_manifest(evaluater): cargo = evaluater("%__cargo")[0] - assert evaluater("%cargo_vendor_manifest")[0] == " ".join( - [ - f"{cargo} tree", - "--workspace", - "--offline", - "--edges normal,build,dev", - "--no-dedupe", - "--target all", - "--all-features", - "--prefix none", - '--format "{p}"', - '| grep -v "$(pwd)"', - '| sed -e "s: (proc-macro)::"', - "| sort -u", - "> cargo-vendor.txt", - ] - ) + assert evaluater("%cargo_vendor_manifest")[0].splitlines() == [ + "(", + "set -euo pipefail", + " ".join( + [ + f"{cargo} tree", + "--workspace", + "--offline", + "--edges normal,build,dev", + "--no-dedupe", + "--target all", + "--all-features", + "--prefix none", + '--format "{p}"', + '| grep -v "$(pwd)"', + '| sed -e "s: (proc-macro)::"', + "| sort -u", + "> cargo-vendor.txt", + ] + ), + ")", + ]