PR#25: add macros for overriding installation of crate sources and binaries - fedora-rust/rust-packaging

fedora-rust / rust-packaging

#25 add macros for overriding installation of crate sources and binaries

Merged 3 months ago by decathorpe. Opened 3 months ago by decathorpe.

Unknown source main into main

add macros for overriding installation of crate sources and binaries

Fabio Valentini • 3 months ago

fa7e229

macros.d/macros.cargo

file modified

+18 -2

		`@@ -42,6 +42,22 @@`
		`# the current working directory is a "binary" crate`
		`%__cargo_is_bin() [ $(%{__cargo_to_rpm} --path Cargo.toml is-bin) -eq 1 ]`

		`+ # cargo_install_lib: macro to control whether library sources are installed`
		`+ # by the cargo_install macro`
		`+ #`
		`+ # If this macro is set to 0 in a spec file, then crate sources will not be`
		`+ # installed to crate_instdir even if a library target is auto-detected or the`
		`+ # crate defines a [lib] target explicitly.`
		`+ %cargo_install_lib 1`
		`+`
		`+ # cargo_install_bin: macro to control whether binary targets are installed`
		`+ # by the cargo_install macro`
		`+ #`
		`+ # If this macro is set to 0 in a spec file, then no executables will be`
		`+ # installed to _bindir even if binary targets are auto-detected or the crate`
		`+ # defines one or more [[bin]] targets explicitly.`
		`+ %cargo_install_bin 1`
		`+`
		`# cargo_prep: macro to set up build environment for cargo projects`
		`#`
		`# This involves four steps:`
		`@@ -171,7 +187,7 @@`
		`%cargo_install(t:naf:)\`
		`(\`
		`set -euo pipefail \`
		`- if %{__cargo_is_lib} ; then \`
		`+ 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) \`
		`REG_DIR=%{buildroot}%{cargo_registry}/$CRATE_NAME-$CRATE_VERSION \`
		`@@ -183,7 +199,7 @@`
		`%{__rm} -f $REG_DIR/Cargo.toml.deps \`
		`echo '{"files":{},"package":""}' > $REG_DIR/.cargo-checksum.json \`
		`fi \`
		`- if %{__cargo_is_bin} ; then \`
		`+ if %{__cargo_is_bin} && [ %{cargo_install_bin} -eq 1 ] ; then \`
		`%{shrink: \`
		`%{__cargo} install \`
		`%{__cargo_common_opts} \`

tests/test_macros_cargo.py

file modified

+13 -13

		`@@ -144,7 +144,7 @@`
		`"",`
		`"cat >> .cargo/config << EOF",`
		`"[source.vendored-sources]",`
		`- f'directory = "vendor"',`
		`+ 'directory = "vendor"',`
		`"",`
		`"[source.crates-io]",`
		`'registry = "https://crates.io"',`
		`@@ -281,24 +281,24 @@`
		`awk = evaluater("%__awk")[0]`

		`assert [line.rstrip() for line in evaluater("%cargo_install")[0].splitlines()] == [`
		`- f"(",`
		`- f"set -euo pipefail",`
		`- f"if {cargo_is_lib} ; then",`
		`+ "(",`
		`+ "set -euo pipefail",`
		`+ 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)",`
		`f" REG_DIR={buildroot}{cargo_registry}/$CRATE_NAME-$CRATE_VERSION",`
		`- f" /usr/bin/mkdir -p $REG_DIR",`
		`+ " /usr/bin/mkdir -p $REG_DIR",`
		`f" {awk} -i inplace -v INPLACE_SUFFIX=.deps '/^\\[((.+\\.)?((dev\|build)-)?dependencies\|features)/{{f=1;next}} /^\\[/{{f=0}}; !f' Cargo.toml",`
		`f" {cargo} package -l \| grep -w -E -v 'Cargo.(lock\|toml.orig)' \| xargs -d '\\n' /usr/bin/cp --parents -a -t $REG_DIR",`
		`- f" /usr/bin/mv Cargo.toml{{.deps,}}",`
		`- f" /usr/bin/cp -a Cargo.toml $REG_DIR/Cargo.toml",`
		`- f" /usr/bin/rm -f $REG_DIR/Cargo.toml.deps",`
		`- f' echo \'{{"files":{{}},"package":""}}\' > $REG_DIR/.cargo-checksum.json',`
		`- f"fi",`
		`- f"if {cargo_is_bin} ; then",`
		`+ " /usr/bin/mv Cargo.toml{.deps,}",`
		`+ " /usr/bin/cp -a Cargo.toml $REG_DIR/Cargo.toml",`
		`+ " /usr/bin/rm -f $REG_DIR/Cargo.toml.deps",`
		`+ ' echo \'{"files":{},"package":""}\' > $REG_DIR/.cargo-checksum.json',`
		`+ "fi",`
		`+ f"if {cargo_is_bin} && [ 1 -eq 1 ] ; then",`
		`f" {cargo} install {cargo_common_opts} --profile rpm --no-track --path .",`
		`- f"fi",`
		`- f")",`
		`+ "fi",`
		`+ ")",`
		`]`

decathorpe commented 3 months ago

I'm trying to address https://pagure.io/fedora-rust/rust-packaging/issue/22.

This PR add two new macros to allow modifying the default behaviour of the %cargo_install macro:

%cargo_install_lib 1
%cargo_install_bin 1

Setting the first macro to 0 prevents installation of crate sources to %{crate_instdir} even if the crate provides a [lib] interface. Setting the second macro to 0 prevents installation of executables even of the crate defines any [[bin]] targets explicitly or implicitly.

This should address two use cases that are currently not correctly supported:

Disable installation of binaries that need to be built but should not be installed (for example, binaries that are required for running tests).
Disable installation of library sources in packages that are not Rust crates / published on crates.io.

Support for setting both macros will be added to rust2rpm separately once this lands in cargo-rpm-macros.

salimma commented on line 29 of macros.d/macros.cargo 3 months ago

isn't %{__cargo_is_lib} going to expand to either 0 or 1 as well?

bash-5.2$ if 1; then echo true-ish; fi
bash: 1: command not found
bash-5.2$

We should probably compare them both to 1 explicitly

e.g.

bash-5.2$ if [ 1 == 1 ] && [ 0 == 1 ]; then echo true-ish; fi
bash-5.2$ if [ 1 == 1 ] && [ 1 == 1 ]; then echo true-ish; fi
true-ish

decathorpe commented 3 months ago

No, %__cargo_is_lib expands to [ $(/usr/bin/cargo2rpm --path Cargo.toml is-lib) -eq 1 ], so that should be fine. I can add the -eq 1 to the second one too if you think that would be better?

rebased onto fa7e229

3 months ago

salimma commented 3 months ago

I think we do need -eq 1

michel@michel-fedora-PC198L6J:~$ [ 0 ] && echo true
true

michel@michel-fedora-PC198L6J:~$ [ 0 -eq 1 ] && echo true
michel@michel-fedora-PC198L6J:~$

decathorpe commented 3 months ago

Ok, I tested the new version with a package, and setting either %global cargo_install_lib 0 and %global cargo_install_bin 0 both had the intended effect (and not setting either did not change the default behaviour), so I'm going to merge this.

Pull-Request has been merged by decathorpe

3 months ago