PR#1: [RFC] Add devel and feature subpackage generation and testing aparatus - fedora-rust/rust-packaging

fedora-rust / rust-packaging

#1 [RFC] Add devel and feature subpackage generation and testing aparatus

Opened 2 years ago by gotmax23. Modified a year ago

fedora-rust/ gotmax23/rust-packaging subpackages into main

Add testing apparatus

Maxwell G • a year ago

aca0910

Add devel and feature subpackage generation

Maxwell G • a year ago

0000671

macros.d/macros.cargo-srpm

file added

+55

		`@@ -0,0 +1,55 @@`
		`+ # Generate a %%description for a devel subpackage`
		`+ #`
		`+ # %%1 - The of the feature`
		`+ # [-n] - Crate name (defaults to %%{crate})`
		`+ %cargo_devel_description(n:) \`
		`+ This package contains library source intended for building other packages which\`
		`+ use the "%{?-n:%{-n*}}%{!?-n:%{crate}}" crate.`
		`+`
		`+ # Generate a %%description for a feature subpackage`
		`+ #`
		`+ # %%1 - The of the feature`
		`+ # [-n] - Crate name (defaults to %%{crate})`
		`+ %cargo_feature_description(n:) %{!?1:%{error:No feature name was passed}}\`
		`+ This package contains library source intended for building other packages which\`
		`+ use the "%{1}" feature of the "%{?-n:%{-n*}}%{!?-n:%{crate}}" crate.`
		`+`
		`+`
		`+ # Generate a -devel subpackage`
		`+ #`
		`+ # [-n] - Crate name (defaults to %%{crate})`
		`+ # [-p] - Extra preamble to include in the subpackage definition`
		`+ %cargo_devel_subpackage(n:p:) %{expand:`
		`+ %{?-n:%define _crate %{-n*}}`
		`+ %{!?-n:%define _crate %{crate}}`
		`+ %package -n rust-%{_crate}-devel`
		`+ Summary: %{summary}`
		`+ BuildArch: noarch`
		`+ %{?-p:%{-p*}}`
		`+`
		`+ %description -n rust-%{_crate}-devel %{_description}`
		`+`
		`+ %{cargo_devel_description -n %{_crate}}`
		`+`
		`+ %files -n rust-%{_crate}-devel`
		`+ %{cargo_registry}/%{_crate}-%{version_no_tilde}/}`
		`+`
		`+ # Generate a feature subpackage`
		`+ #`
		`+ # %%1 - The of the feature`
		`+ # [-n] - Crate name (defaults to %%{crate})`
		`+ # [-p] - Extra preamble to include in the subpackage definition`
		`+ %cargo_feature_subpackage(n:p:) %{expand:`
		`+ %{!?1:%{error:No feature name was passed}}`
		`+ %{?-n:%define _crate %{-n*}}`
		`+ %{!?-n:%define _crate %{crate}}`
		`+`
		`+ %package -n rust-%{_crate}+%{1}-devel`
		`+ Summary: %{summary}`
		`+ BuildArch: noarch`
		`+ %{?-p:%{-p*}}`
		`+`
		`+ %description -n rust-%{_crate}+%{1}-devel %{cargo_feature_description -n %{_crate} %{1}}`
		`+`
		`+ %files -n rust-%{_crate}+%{1}-devel`
		`+ %ghost %{cargo_registry}/%{_crate}-%{version_no_tilde}/Cargo.toml}`

tests/__init__.py

file added

empty or binary file added

tests/conftest.py

file added

+47

		`@@ -0,0 +1,47 @@`
		`+ import os`
		`+ import subprocess`
		`+ from collections.abc import Callable, Sequence`
		`+ from pathlib import Path`
		`+`
		`+ import pytest`
		`+`
		`+ PARENT = Path.cwd().parent`
		`+ # e.g. MACRO_DIR=%{buildroot}%{_rpmmacrodir} %pytest`
		`+ MACRO_DIR = Path(os.environ.get("MACRO_DIR") or PARENT.joinpath("macros.d"))`
		`+`
		`+`
		`+ @pytest.fixture(scope="session")`
		`+ def macros_path() -> str:`
		`+ default_macros_path = subprocess.check_output(`
		`+ # Don't judge. It works.`
		`+ "rpm --showrc \| grep 'Macro path' \| awk -F ': ' '{print $2}'",`
		`+ shell=True,`
		`+ text=True,`
		`+ check=True,`
		`+ ).strip()`
		`+ return f"{default_macros_path}:{MACRO_DIR}/macros.*"`
		`+`
		`+`
		`+ @pytest.fixture`
		`+ def evaluater(macros_path: str) -> Callable[..., tuple[str, str]]:`
		`+ def runner(`
		`+ exp: str,`
		`+ should_fail: bool = False,`
		`+ defines: dict[str, str] \| None = None,`
		`+ undefines: Sequence[str] = (),`
		`+ ) -> tuple[str, str]:`
		`+ cmd: list[str] = ["rpm", "--macros", macros_path]`
		`+ defines = defines or {}`
		`+ for name, value in defines.items():`
		`+ cmd.extend(("--define", f"{name} {value}"))`
		`+ for name in undefines:`
		`+ cmd.extend(("-E", f"%undefine {name}"))`
		`+ cmd.extend(("-E", exp))`
		`+ proc = subprocess.run(cmd, text=True, capture_output=True)`
		`+ if should_fail:`
		`+ assert proc.returncode != 0`
		`+ else:`
		`+ assert proc.returncode == 0, proc.stderr`
		`+ return proc.stdout.strip(), proc.stderr.strip()`
		`+`
		`+ return runner`

tests/test_srpm_macros.py

file added

+151

		`@@ -0,0 +1,151 @@`
		`+ def test_skip_build(evaluater):`
		`+ assert evaluater("%__cargo_skip_build")[0] == "0"`
		`+`
		`+`
		`+ def test_rust_arches(evaluater):`
		`+ ix86 = evaluater("%ix86")[0]`
		`+ assert (`
		`+ evaluater("%rust_arches")[0]`
		`+ == f"x86_64 {ix86} armv7hl aarch64 ppc64 ppc64le riscv64 s390x"`
		`+ )`
		`+`
		`+`
		`+ def strip_empty(text: str) -> str:`
		`+ """`
		`+ Remove empty lines in macro output to ease comparisons.`
		`+ """`
		`+ return "\n".join(line.strip() for line in text.splitlines() if line.strip())`
		`+`
		`+`
		`+ def test_cargo_feature_subpackage_simple(evaluater):`
		`+ output = evaluater(`
		`+ "%{cargo_feature_subpackage -n foo feature}",`
		`+ False,`
		+ # Ensure that `-n foo` overrides `%{crate}`
		`+ dict(version="0.0.1", summary="Hello world", crate="notfoo"),`
		`+ )`
		`+ expected = """\`
		`+ %package -n rust-foo+feature-devel`
		`+ Summary: Hello world`
		`+ BuildArch: noarch`
		`+`
		`+ %description -n rust-foo+feature-devel`
		`+ This package contains library source intended for building other packages which`
		`+ use the "feature" feature of the "foo" crate.`
		`+`
		`+ %files -n rust-foo+feature-devel`
		`+ %ghost /usr/share/cargo/registry/foo-0.0.1/Cargo.toml"""`
		`+ assert strip_empty(output[0]) == strip_empty(expected)`
		`+`
		`+`
		`+ def test_cargo_feature_subpackage_invalid(evaluater):`
		`+ output = evaluater("%{cargo_feature_subpackage -n testcrate}", True)`
		`+ assert output[1] == "error: No feature name was passed"`
		`+`
		`+`
		`+ def test_cargo_feature_subpackage_preamble(evaluater):`
		`+ output = evaluater(`
		`+ "%{cargo_feature_subpackage -p %{quote:Requires: python3-devel} feature}",`
		`+ False,`
		`+ dict(version="0.0.1", summary="Hello world", crate="foo"),`
		`+ )`
		`+`
		`+ expected = """\`
		`+ %package -n rust-foo+feature-devel`
		`+ Summary: Hello world`
		`+ BuildArch: noarch`
		`+ Requires: python3-devel`
		`+`
		`+ %description -n rust-foo+feature-devel`
		`+ This package contains library source intended for building other packages which`
		`+ use the "feature" feature of the "foo" crate.`
		`+`
		`+ %files -n rust-foo+feature-devel`
		`+ %ghost /usr/share/cargo/registry/foo-0.0.1/Cargo.toml"""`
		`+ assert strip_empty(output[0]) == strip_empty(expected)`
		`+`
		`+`
		`+ def test_cargo_feature_subpackage_preamble_multiline(evaluater):`
		`+ preamble = """\`
		`+ Requires: python3-devel`
		`+ Provides: abcd\`
		`+ """`
		`+ # %{cargo_feature_subpackage -n foo -p %{quote:PREAMBLE_HERE} feature}`
		`+ exp = "%{cargo_feature_subpackage -n foo"`
		`+ exp += " -p %{quote: " + preamble + "} feature}"`
		`+ output = evaluater(`
		`+ exp,`
		`+ False,`
		`+ dict(version="0.0.1", summary="Hello world"),`
		`+ )`
		`+`
		`+ expected = """\`
		`+ %package -n rust-foo+feature-devel`
		`+ Summary: Hello world`
		`+ BuildArch: noarch`
		`+ Requires: python3-devel`
		`+ Provides: abcd`
		`+`
		`+ %description -n rust-foo+feature-devel`
		`+ This package contains library source intended for building other packages which`
		`+ use the "feature" feature of the "foo" crate.`
		`+`
		`+ %files -n rust-foo+feature-devel`
		`+ %ghost /usr/share/cargo/registry/foo-0.0.1/Cargo.toml"""`
		`+ assert strip_empty(output[0]) == strip_empty(expected)`
		`+`
		`+`
		`+ def test_cargo_devel_subpackage(evaluater):`
		`+ output = evaluater(`
		`+ "%cargo_devel_subpackage",`
		`+ False,`
		`+ dict(`
		`+ version="1.0.1",`
		`+ summary="Hopefully, this code doesn't get too rusty",`
		`+ crate="abc",`
		`+ ),`
		`+ )`
		`+ expected = """`
		`+ %package -n rust-abc-devel`
		`+ Summary: Hopefully, this code doesn't get too rusty`
		`+ BuildArch: noarch`
		`+`
		`+`
		`+ %description -n rust-abc-devel %{_description}`
		`+`
		`+`
		`+ This package contains library source intended for building other packages which`
		`+ use the "abc" crate.`
		`+`
		`+ %files -n rust-abc-devel`
		`+ /usr/share/cargo/registry/abc-1.0.1/`
		`+ """`
		`+ assert strip_empty(output[0]) == strip_empty(expected)`
		`+`
		`+`
		`+ def test_cargo_devel_subpackagen(evaluater):`
		`+ _description = "\\\nBeep boop is the best piece of software since sliced bread."`
		`+ output = evaluater(`
		`+ "%cargo_devel_subpackage -n xyz -p %{quote:Provides: beep-boop}",`
		`+ False,`
		`+ dict(`
		`+ version="1.0.1", summary="Beep boop", crate="abc", _description=_description`
		`+ ),`
		`+ )`
		`+ expected = """`
		`+ %package -n rust-xyz-devel`
		`+ Summary: Beep boop`
		`+ BuildArch: noarch`
		`+ Provides: beep-boop`
		`+`
		`+`
		`+ %description -n rust-xyz-devel`
		`+ Beep boop is the best piece of software since sliced bread.`
		`+`
		`+ This package contains library source intended for building other packages which`
		`+ use the "xyz" crate.`
		`+`
		`+ %files -n rust-xyz-devel`
		`+ /usr/share/cargo/registry/xyz-1.0.1/`
		`+ """`
		`+ assert strip_empty(output[0]) == strip_empty(expected)`

gotmax23 commented 2 years ago

Add devel and feature subpackage generation

%cargo_feature_subpackage

Simplest invocation:

%cargo_feature_subpackage default

Explicitly specify the crate name instead of using %{crate}:

%cargo_feature_subpackage default -n foo

Add extra preamble from a macro:

%global py39preamble %{expand:
Requires: python3-devel >= 3.9
Provides: useless-provides-to-show-that-multi-lines-work}

%cargo_extras_subpackage abi3-py39 -p %{quote:%py39preamble}

Add extra preamble inline:

%cargo_extras_subpackage abi3-py39 -p %{quote:Requires: python3-devel >= 3.9}

Add extra multiline preamble inline:

%{cargo_extras_subpackage abi3-py39 -p %{quote:
Requires: python3-devel >= 3.9
Provides: useless-provides-to-show-that-multi-lines-work
}}

%cargo_devel_subpackage

Simplest invocation:

%cargo_devel_subpackage
%license %{crate_instdir}/LICENSE
%doc %{crate_instdir}/README.md

Extra preamble and overriding the crate name work the same way.

gotmax23 commented 2 years ago

Note that this doesn't use the new dynamic subpackage generation feature, so it's compatible with older rpm versions. It would be possible to get a list of features from cargo2rpm during %build and then generate dynamic definitions there without having to manually keep track of %cargo_feature_subpackage invocations.

decathorpe commented 2 years ago

Interesting. I wanted to work on something like this.

How is this supposed to work in rust2rpm?

Also, I don't particularly like the need to define extra macros and / or using %extend and %quote macros just to add additional Requires ... would it be possible to just add an -r foo flag for specifying additional Requires? We don't need support for any other RPM tags, so making this generic is not necessary.

For example, using something like this would be 100% enough for everything we do in Fedora:

%cargo_feature_subpackage default -r "pkgconfig(curl)"

gotmax23 commented 2 years ago

Interesting. I wanted to work on something like this.

I hope I didn't step on your toes :).

How is this supposed to work in rust2rpm?

Not much differently. Replace the full subpackage definitions with calls to these macros.

would it be possible to just add an -r foo flag for specifying additional Requires?

Yes, but you wouldn't be able to specify if multiple times. You'd still need %{quote:...} if the dependency spec has a space. I think it makes sense to add -r in addition to -p.

gotmax23 commented 2 years ago

Another thought: it would be possible to add a %cargo_feature_subpackages macro that accepts a multi-item list of features but no -p or -r and uses a lua for loop to call %cargo_feature_subpackage for each one.

decathorpe commented 2 years ago

I hope I didn't step on your toes :).

Not at all!

would it be possible to just add an -r foo flag for specifying additional Requires?

Yes, but you wouldn't be able to specify if multiple times. You'd still need %{quote:...} if the dependency spec has a space. I think it makes sense to add -r in addition to -p.

That's unfortunate ...

So something like this can't work even if the argument for the -r flag is quoted?

%cargo_feature_subpackage default -r "pkgconfig(curl) >= 7.88.1"

decathorpe commented 2 years ago

Another thought: it would be possible to add a %cargo_feature_subpackages macro that accepts a multi-item list of features but no -p or -r and uses a lua for loop to call %cargo_feature_subpackage for each one.

That's probably a step too far. I'd rather wait with fully automating this once the subpackage generation is available across all Fedora releases. And even then, RHEL 9 RPM will probably be too old for it forever ...

gotmax23 commented 2 years ago

That's unfortunate ...

So something like this can't work even if the argument for the -r flag is quoted?

%cargo_feature_subpackage default -r "pkgconfig(curl) >= 7.88.1"

Yeah, you need to use %{quote:}. It's not like shell. The double quotes are just parsed as normal characters. With your example, the literal value of %{-r*} is "pkgconfig(curl) and the value of %{*} (positional args) is default >= 7.88.1".

decathorpe commented 2 years ago

Oh that's just awful. Aren't there two different argparse implementations in RPM? Are they both broken like that?

gotmax23 commented 2 years ago

That's probably a step too far. I'd rather wait with fully automating this once the subpackage generation is available across all Fedora releases.

I agree.

And even then, RHEL 9 RPM will probably be too old for it forever ...

It might be possible to backport.

gotmax23 commented 2 years ago

Aren't there two different argparse implementations in RPM? Are they both broken like that?

I don't think so? This is how rpm macros' getopt style parsing works. Unless you want to parse all the arguments yourself (%{**} contains the unprocessed argv)...

Edited 2 years ago by gotmax23

gotmax23 commented 2 years ago

Alternatively, the macro could take the name of a macro (e.g. -r preamble) and then look that up %{preamble} without the need for %{quote}.