PR#35: Allow piping output of %pyproject_buildrequires - rpms/pyproject-rpm-macros

rpms / pyproject-rpm-macros

#35 Allow piping output of %pyproject_buildrequires

Closed 4 years ago by churchyard. Opened 4 years ago by churchyard.

rpms/ churchyard/pyproject-rpm-macros allow_grep into master

Allow piping output of %pyproject_buildrequires

Miro Hrončok • 4 years ago

82ead99

macros.pyproject

file modified

+1 -4

		`@@ -30,10 +30,7 @@`
		`echo 'python3dist(pytoml)'`
		`# setuptools assumes no pre-existing dist-info`
		`rm -rfv *.dist-info/ >&2`
		`- if [ -f %{__python3} ]; then`
		`- RPM_TOXENV="%{toxenv}" %{__python3} -I %{_rpmconfigdir}/redhat/pyproject_buildrequires.py %{?**}`
		`- fi`
		`- }`
		`+ test -f %{__python3} && RPM_TOXENV="%{toxenv}" %{__python3} -I %{_rpmconfigdir}/redhat/pyproject_buildrequires.py %{?**}}`

		`%tox(e:) %{expand:\\\`
		`TOX_TESTENV_PASSENV="${TOX_TESTENV_PASSENV:-*}" \\`

pyproject-rpm-macros.spec

file modified

		`@@ -89,6 +89,7 @@`
		`%changelog`
		`* Mon Mar 02 2020 Miro Hrončok <mhroncok@redhat.com> - 0-13`
		`- Tox dependency generator: Handle deps read in from a text file (#1808601)`
		`+ - Allow piping output of %%pyproject_buildrequires`

		`* Wed Feb 05 2020 Miro Hrončok <mhroncok@redhat.com> - 0-12`
		`- Fallback to setuptools.build_meta:__legacy__ backend instead of setuptools.build_meta`

tests/python-openqa_client.spec

file modified

+4 -4

		`@@ -24,12 +24,12 @@`

		`%prep`
		`%autosetup -p1 -n %{pypi_name}-%{version}`
		`- # setuptools-git is needed to build the source distribution, but not`
		`- # for packaging, which starts from the source distribution`
		`- sed -i -e 's., "setuptools-git"..g' pyproject.toml`

		`%generate_buildrequires`
		`- %pyproject_buildrequires -t`
		`+ # setuptools-git is needed to build the source distribution, but not`
		`+ # for packaging, which starts from the source distribution`
		`+ # technically, this is not an API, but we test that piping the output to a subcommand works`
		`+ %{pyproject_buildrequires -t} \| grep -v setuptools-git`

		`%build`
		`%pyproject_wheel`

churchyard commented 4 years ago

@adamwill - this should allow you to do:

%{pyproject_buildrequires -t} | grep -v setuptools-git

ignatenkobrain commented 4 years ago

Why do we even do this? I think instead we should patch thing which is giving metadata instead. This would make it very hard to do any queries on installed files

adamwill commented 4 years ago

"Why do we even do this? I think instead we should patch thing which is giving metadata instead."

That's what I'm doing now, @churchyard suggested this instead. This is certainly a little bit less ugly than:

sed -i -e 's., "setuptools-git"..g' pyproject.toml

churchyard commented 4 years ago

Why do we even do this?

It looks more straightforward for the reader of the spec IMHO.

This would make it very hard to do any queries on installed files

Not in this case, this is not a runtime dependency.

pviktori commented 4 years ago

This looks useful, but…
I'm worried about recommending grep specifically, since it works on text rather than actual data: grep -v setuptools will nuke setuptools-git as well. While | grep -v will probably work most of the time, I wouldn't want to document this as the way to grep out some packages.

We could document this as a way to apply a filter to the dependencies. But to allow it being used that way, we need to specify (and lock) the output of %pyproject_buildrequires. AFAIK, we can't realistically do that; it can only be robustly parsed by RPM.

+1 to adding this, but when it's used there should be a comment that it's a hack.
I think there should be a test (to notify us when it breaks); I don't think it needs a mention in the docs.

How soon/how hard do we need to look for other use cases, and better solutions?

churchyard commented 4 years ago

As for the robustness of this: I think it's acceptable that grepping the output is not API, but at least we allow to pipe the output to any arbitrary command. I think that is an improvement.

Once we fix https://bugzilla.redhat.com/show_bug.cgi?id=1808601 we can add a modified version of https://src.fedoraproject.org/rpms/python-openqa_client/blob/master/f/python-openqa_client.spec as a test.

churchyard commented 4 years ago

[citest]

pviktori commented 4 years ago

+1 for the plan. It does need a test.

churchyard commented 4 years ago

The test spec is ready at https://src.fedoraproject.org/rpms/pyproject-rpm-macros/pull-request/36 -- when that one is merged, will rebase this on top to use the grep in that spec.

rebased onto 82ead99

4 years ago

churchyard commented 4 years ago

OK, this doesn't work at all.

The internal code says: "Requirement not satisfied: setuptools-git" and aborts, so mock can install it. However, mock doesn't see it (it is grepped out) and hence the build proceeds to %build.

Sorry for the noise.

Pull-Request has been closed by churchyard

4 years ago