#140 Support %forgesetup and use %writevars
Closed 2 years ago by gotmax23. Opened 2 years ago by gotmax23.
rpms/ gotmax23/python-rpm-macros rawhide  into  rawhide

file modified
+10 -2
@@ -17,7 +17,7 @@ 

  # There are two macros:

  #

  # This always contains the major.minor version (with dots), default for %%python3_version.

- %__default_python3_version 3.11

+ %__default_python3_version @@__DEFAULT_PYTHON3_VERSION@@

  #

  # The pkgname version that determines the alternative provide name (e.g. python3.9-foo),

  # set to the same as above, but historically hasn't included the dot.
@@ -186,7 +186,15 @@ 

  \

      local first = string.sub(src, 1, 1)

  \

-     print(url .. first .. '/' .. src .. '/' .. src .. '-' .. ver .. '.' .. ext)

+     local archivename = src .. '-' .. ver

+     print(url .. first .. '/' .. src .. '/' .. archivename .. '.' .. ext)

+ \

+     -- Plug into the forge macros so users can run %forgesetup or %forgeautosetup

+     -- and not have to specify -n to %setup / %autosetup

+     -- and potentially define extra vars

+     local fedora = require "fedora.common"

+     fedora.safeset("forgesetupargs0", '-n ' .. archivename, true)

+     fedora.safeset("forgesetupargs", '-n ' .. archivename, true)

  }

  

  %py_provides() %{lua:

file modified
+11 -12
@@ -37,19 +37,9 @@ 

  # brp scripts: GPLv2+

  License:        MIT and Python and GPLv2+

  

- # The package version MUST be always the same as %%{__default_python3_version}.

- # To have only one source of truth, we load the macro and use it.

- # The macro is defined in python-srpm-macros.

- %{lua:

- if posix.stat(rpm.expand('%{SOURCE102}')) then

-   rpm.load(rpm.expand('%{SOURCE102}'))

- elseif posix.stat('macros.python-srpm') then

-   -- something is parsing the spec without _sourcedir macro properly set

-   rpm.load('macros.python-srpm')

- end

- }

+ %global __default_python3_version 3.11

  Version:        %{__default_python3_version}

- Release:        1%{?dist}

+ Release:        2%{?dist}

  

  BuildArch:      noarch

  
@@ -102,6 +92,9 @@ 

  %prep

  %autosetup -c -T

  cp -a %{sources} .

+ # Write definition of %%__default_python3_version from specfile into

+ # macros.python-srpm.

+ %writevars -f macros.python-srpm __default_python3_version

  

  

  %install
@@ -151,6 +144,12 @@ 

  

  

  %changelog

+ * Sun Jun 26 2022 Maxwell G <gotmax@e.email> - 3.11-2

+ - Define %%forgesetupargs in %%pypi_source so users can call %%forgeautosetup

+   or %%forgesetup and not have to define extra macros or pass -n to %setup.

+ - Write definition of %__default_python3_version from spec instead of

+   loading macros.python-srpm.

+ 

  * Mon Jun 13 2022 Tomáš Hrnčiar <thrnciar@redhat.com> - 3.11-1

  - Update main Python to Python 3.11

  - https://fedoraproject.org/wiki/Changes/Python3.11

I know the forge macros are a bit controversial, but I find them useful, and it's pretty trivial to implement support here. As for %writevars, we use that macro in go-rpm-macros, and it's simpler to write from the specfile to the macros file than the other way around. Feel free to take one, both, or none of these changes.

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

I run the tests locally like this:

PYTHONPATH=. TESTED_FILES='/usr/lib/rpm/macros:/usr/lib/rpm/platform/x86_64-linux/macros:macros.*' ALTERNATE_PYTHON_VERSION=3.6 pytest tests/test_evals.py

Hence, I do not like the %writevars commit much.

Also, the CI failed because it doesn't really work, the installed file /usr/lib/rpm/macros.d/macros.python-srpm still has %__default_python3_version @@__default_python3_version@@ in it. The build.log has:

+ sed -i $'s\035@@__DEFAULT_PYTHON3_VERSION@@\0353.11\035g' macros.python-srpm

So I suppose this is a problem in the case of the placeholder value used.


About the forge macros thing: Could you please elaborate more on why this is needed and how it works? Ideally, show me an example spec file.

You're right. I fixed the %writevars step. I should have tested it first... It's possible run fedpkg prep and change into that directory, but that is admittedly more steps.

Regarding the other change: take this example spec. This change would simply allow

--- python-pynvim.spec.orig 2022-06-28 16:10:32.193457803 -0500
+++ python-pynvim.spec  2022-06-28 16:06:27.924545798 -0500
@@ -31,7 +31,7 @@


 %prep
-%autosetup -p1 -n pynvim-%{version}
+%forgeautosetup -p1


 %generate_buildrequires

This allows less typing and provide a better solution than the %{pypi_name} and %{srcname} variables which you're trying to get rid of.

You can look at the %forgesetup/%forgeautosetup code here: https://src.fedoraproject.org/rpms/redhat-rpm-config/blob/rawhide/f/macros.forge#_41 . Basically, these macros take the value of %forgesetupargs and pass it to %setup or %autosetup. Normally, this is set by %forgemeta, but we can also plugin to it here.

I suppose the one downside with my implementation is that it doesn't properly handle specs that use %{pypi_source} multiple times. %{pypi_source} will only set %forgesetupargs if it is empty (that's what safeset does), so %forgesetup would only use the first source (it has the ability to support multiple). It should be possible to handle this, but it would require more extensive changes to %pypi_source that I'm not comfortable making. I'm not sure if any specfile actually calls %pypi_source twice, but I figured I'd point out that caveat.

2 new commits added

  • Write %__default_python3_version value from spec
  • Define %forgesetupargs in %pypi_source so users can call %forgeautosetup
2 years ago

So, this assumes archivename is the directory name in the archive as well. Is that always the case with sidsts? Probably yes, but I am not sure.

What I don't understand about your example: Why would you use %forgeautosetup when you don't use any git forge? Just to save typing the -n attribute? Wouldn't that be a bit confusing to the reader?

In other words, wouldn't it be more understandable liek this?

%prep
%pypi_source_autosetup -p1

I am not saying we need that macro, but it sounds more appealing to me than a "magical" %pypi_source -> %forgeautosetup passing of archive name.

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

So, this assumes archivename is the directory name in the archive as well. Is that always the case with sidsts? Probably yes, but I am not sure.

You would know better than me :).

Just to save typing the -n attribute? Wouldn't that be a bit confusing to the reader?

Yeah, that was pretty much the idea. I agree that it could potentially be confusing.

In other words, wouldn't it be more understandable liek this?

%prep %pypi_source_autosetup -p1

I am not saying we need that macro, but it sounds more appealing to me than a "magical" %pypi_source -> %forgeautosetup passing of archive name.

That's probably a better option if we decide this is worthwhile. If you really want to separate from the forge macros, we should not use the %forgesetupargs macro name.

EDIT: I meant we shouldn't use the %forgesetupargs macro name to avoid clashing with the forge macros.

So, this assumes archivename is the directory name in the archive as well. Is that always the case with sidsts? Probably yes, but I am not sure.

You would know better than me :).

I looked this up. At least according to the PyPA, the top level directory name in an sdist is supposed to be named {name}-{version}. I, for one, have never seen anything different in my packaging experience.

Pull-Request has been closed by gotmax23

2 years ago