PR#28: [DO-NOT-MERGE] Only build in release mode / with optimizations when shipping binaries - fedora-rust/rust-packaging

fedora-rust / rust-packaging

#28 [DO-NOT-MERGE] Only build in release mode / with optimizations when shipping binaries

Opened 2 months ago by decathorpe. Modified 2 months ago

fedora-rust/ decathorpe/rust-packaging main into main

Only build in release mode / with optimizations when shipping binaries

Fabio Valentini • 2 months ago

44f39cf

macros.d/macros.cargo

file modified

+26 -16

		`@@ -6,7 +6,7 @@`
		`# features that have not been stabilized yet, i.e. the`
		`# "-Z avoid-dev-deps" flag which is passed to cargo by the cargo_build,`
		`# cargo_install, and cargo_test macros.`
		`- %__cargo /usr/bin/env CARGO_HOME=.cargo RUSTC_BOOTSTRAP=1 RUSTFLAGS='%{build_rustflags}' /usr/bin/cargo`
		`+ %__cargo /usr/bin/env CARGO_HOME=.cargo RUSTC_BOOTSTRAP=1 /usr/bin/cargo`

		`# __cargo_common_opts: common command line flags for cargo`
		`#`
		`@@ -152,13 +152,17 @@`

		`# cargo_build: builds the crate with cargo with the specified feature flags`
		`%cargo_build(naf:)\`
		`- %{shrink: \`
		`- %{__cargo} build \`
		`- %{__cargo_common_opts} \`
		`- --profile rpm \`
		`- %{__cargo_parse_opts %{-n} %{-a} %{-f:-f%{-f*}}} \`
		`- %* \`
		`- }`
		`+ (\`
		`+ set -euo pipefail \`
		`+ export RUSTFLAGS="--cap-lints=warn" \`
		`+ %{shrink: \`
		`+ %{__cargo} build \`
		`+ %{__cargo_common_opts} \`
		`+ %{expr:"%{debug_package}" != "%{nil}" ? "--profile rpm" : ""} \`
		`+ %{__cargo_parse_opts %{-n} %{-a} %{-f:-f%{-f*}}} \`
		`+ %* \`
		`+ }\`
		`+ )`

		`# cargo_test: runs the test suite with cargo with the specified feature flags`
		`#`
		`@@ -168,14 +172,19 @@`
		`# i.e. "%%cargo_test -- -- --skip foo" for skipping all tests with names that`
		`# match "foo".`
		`%cargo_test(naf:)\`
		`- %{shrink: \`
		`- %{__cargo} test \`
		`- %{__cargo_common_opts} \`
		`- --profile rpm \`
		`- --no-fail-fast \`
		`- %{__cargo_parse_opts %{-n} %{-a} %{-f:-f%{-f*}}} \`
		`- %* \`
		`- }`
		`+ (\`
		`+ set -euo pipefail \`
		`+ export RUSTFLAGS="--cap-lints=warn" \`
		`+ export RUSTDOCFLAGS="--cap-lints=warn" \`
		`+ %{shrink: \`
		`+ %{__cargo} test \`
		`+ %{__cargo_common_opts} \`
		`+ %{expr:"%{debug_package}" != "%{nil}" ? "--profile rpm" : ""} \`
		`+ --no-fail-fast \`
		`+ %{__cargo_parse_opts %{-n} %{-a} %{-f:-f%{-f*}}} \`
		`+ %* \`
		`+ }\`
		`+ )`

		`# cargo_install: install files into the buildroot`
		`#`
		`@@ -193,6 +202,7 @@`
		`%cargo_install(t:naf:)\`
		`(\`
		`set -euo pipefail \`
		`+ export RUSTFLAGS="--cap-lints=warn" \`
		`if %{__cargo_is_lib} && [ %{cargo_install_lib} -eq 1 ] ; then \`
		`CRATE_NAME=$(%{__cargo_to_rpm} --path Cargo.toml name) \`
		`CRATE_VERSION=$(%{__cargo_to_rpm} --path Cargo.toml version) \`

tests/test_macros_cargo.py

file modified

+25 -8

		`@@ -2,8 +2,7 @@`


		`def test_cargo(evaluater):`
		`- build_rustflags = evaluater("%build_rustflags")[0]`
		`- assert evaluater("%__cargo")[0] == f"/usr/bin/env CARGO_HOME=.cargo RUSTC_BOOTSTRAP=1 RUSTFLAGS='{build_rustflags}' /usr/bin/cargo"`
		`+ assert evaluater("%__cargo")[0] == "/usr/bin/env CARGO_HOME=.cargo RUSTC_BOOTSTRAP=1 /usr/bin/cargo"`


		`def test_cargo_to_rpm(evaluater):`
		`@@ -263,24 +262,41 @@`
		`cargo = evaluater("%__cargo")[0]`
		`cargo_common_opts = evaluater("%__cargo_common_opts")[0]`

		`- assert evaluater("%cargo_build")[0] == f"{cargo} build {cargo_common_opts} --profile rpm"`
		`+ assert [line.rstrip() for line in evaluater("%cargo_build")[0].splitlines()] == [`
		`+ "(",`
		`+ "set -euo pipefail",`
		`+ "export RUSTFLAGS=\"--cap-lints=warn\"",`
		`+ " ".join([f"{cargo} build", cargo_common_opts, "--profile rpm"]),`
		`+ ")",`
		`+ ]`


		`def test_cargo_test(evaluater):`
		`cargo = evaluater("%__cargo")[0]`
		`cargo_common_opts = evaluater("%__cargo_common_opts")[0]`

		`- assert evaluater("%cargo_test")[0] == f"{cargo} test {cargo_common_opts} --profile rpm --no-fail-fast"`
		`+ assert [line.rstrip() for line in evaluater("%cargo_test")[0].splitlines()] == [`
		`+ "(",`
		`+ "set -euo pipefail",`
		`+ "export RUSTFLAGS=\"--cap-lints=warn\"",`
		`+ "export RUSTDOCFLAGS=\"--cap-lints=warn\"",`
		`+ " ".join([f"{cargo} test", cargo_common_opts, "--profile rpm", "--no-fail-fast"]),`
		`+ ")",`
		`+ ]`


		`def test_cargo_test_with_args(evaluater):`
		`cargo = evaluater("%__cargo")[0]`
		`cargo_common_opts = evaluater("%__cargo_common_opts")[0]`

		`- assert (`
		`- evaluater("%cargo_test -- -- --exact --skip foo")[0]`
		`- == f"{cargo} test {cargo_common_opts} --profile rpm --no-fail-fast -- --exact --skip foo"`
		`- )`
		`+ assert [line.rstrip() for line in evaluater("%cargo_test -- -- --exact --skip foo")[0].splitlines()] == [`
		`+ "(",`
		`+ "set -euo pipefail",`
		`+ "export RUSTFLAGS=\"--cap-lints=warn\"",`
		`+ "export RUSTDOCFLAGS=\"--cap-lints=warn\"",`
		`+ " ".join([f"{cargo} test", cargo_common_opts, "--profile rpm", "--no-fail-fast", "--", "--exact", "--skip", "foo"]),`
		`+ ")",`
		`+ ]`


		`def test_cargo_install(evaluater):`
		`@@ -297,6 +313,7 @@`
		`assert [line.rstrip() for line in evaluater("%cargo_install")[0].splitlines()] == [`
		`"(",`
		`"set -euo pipefail",`
		`+ "export RUSTFLAGS=\"--cap-lints=warn\"",`
		`f"if {cargo_is_lib} && [ 1 -eq 1 ] ; then",`
		`f" CRATE_NAME=$({cargo_to_rpm} --path Cargo.toml name)",`
		`f" CRATE_VERSION=$({cargo_to_rpm} --path Cargo.toml version)",`

decathorpe commented 2 months ago

This required to no longer set RUSTFLAGS explicitly in a few places to ensure they are not getting set three times:

The "%__cargo" macro no longer defines RUSTFLAGS explicitly.
The "%cargo_build" and "%cargo_test" macros now explicitly unset RUSTFLAGS from the build environment. The "rpm" profile already contains settings that are equivalent to the default RUSTFLAGS.

Since RUSTFLAGS are already defined in the default build environment, no packages that ship binaries should be impacted by this change (except maybe for rare cases of environments where RUSTFLAGS is not set and the package uses the private "%__cargo" macro directly).

decathorpe commented 2 months ago

I've built rust-packaging with this change applied in COPR to make test builds easier:
https://copr.fedorainfracloud.org/coprs/decathorpe/rust-packaging-make-it-faster/

decathorpe commented 2 months ago

Hm, thinking about it, unset RUSTFLAGS is not enough. It should probably be RUSTFLAGS=--cap-lints=warn to match the current errors-as-warnings behaviour.

rebased onto 44f39cf

2 months ago

decathorpe commented 2 months ago

I think my previous comment(s) and also https://pagure.io/fedora-rust/rust-packaging/issue/29 should be addressed with the latest version.

jistone commented 2 months ago

The "rpm" profile already contains settings that are equivalent to the default RUSTFLAGS.

It contains opt-level, codegen-units, debug, and strip, but what about -Cforce-frame-pointers=yes and -Clink-arg=%_package_note_flags? And any custom %build_rustflags that a package may set?