#72 Adapt %py3_shebang_fix to use versioned pathfixX.Y.py
Merged 3 years ago by churchyard. Opened 3 years ago by lbalhar.
rpms/ lbalhar/python-rpm-macros shebang_fix  into  master

file modified
+8 -1
@@ -10,7 +10,14 @@ 

  %py_shbang_opts -s

  %py_shbang_opts_nodash %(opts=%{py_shbang_opts}; echo ${opts#-})

  %py_shebang_flags %(opts=%{py_shbang_opts}; echo ${opts#-})

- %py_shebang_fix %{expand:/usr/bin/pathfix.py -pni %{__python} -k%{?py_shebang_flags:a %py_shebang_flags}}

+ %py_shebang_fix %{expand:\\\

+   if [ -f /usr/bin/pathfix%{python_version}.py ]; then

+     pathfix=/usr/bin/pathfix%{python_version}.py

+   else

+     # older versions of Python don't have it and must BR /usr/bin/pathfix.py from python3-devel explicitly

+     pathfix=/usr/bin/pathfix.py

+   fi

+   $pathfix -pni %{__python} -k%{?py_shebang_flags:a %py_shebang_flags}}

  

  # Use the slashes after expand so that the command starts on the same line as

  # the macro

file modified
+8 -1
@@ -8,7 +8,14 @@ 

  %py3_shbang_opts -s

  %py3_shbang_opts_nodash %(opts=%{py3_shbang_opts}; echo ${opts#-})

  %py3_shebang_flags %(opts=%{py3_shbang_opts}; echo ${opts#-})

- %py3_shebang_fix %{expand:/usr/bin/pathfix.py -pni %{__python3} -k%{?py3_shebang_flags:a %py3_shebang_flags}}

+ %py3_shebang_fix %{expand:\\\

+   if [ -f /usr/bin/pathfix%{python3_version}.py ]; then

+     pathfix=/usr/bin/pathfix%{python3_version}.py

+   else

+     # older versions of Python don't have it and must BR /usr/bin/pathfix.py from python3-devel explicitly

+     pathfix=/usr/bin/pathfix.py

+   fi

+   $pathfix -pni %{__python3} -k%{?py3_shebang_flags:a %py3_shebang_flags}}

  

  # Use the slashes after expand so that the command starts on the same line as

  # the macro

file modified
+4 -1
@@ -1,6 +1,6 @@ 

  Name:           python-rpm-macros

  Version:        3.9

- Release:        8%{?dist}

+ Release:        9%{?dist}

  Summary:        The common Python RPM macros

  

  # macros and lua: MIT, compileall2.py: PSFv2
@@ -107,6 +107,9 @@ 

  

  

  %changelog

+ * Fri Jul 24 2020 Lumír Balhar <lbalhar@redhat.com> - 3.9-9

+ - Adapt %%py[3]_shebang_fix to use versioned pathfixX.Y.py

+ 

  * Fri Jul 24 2020 Lumír Balhar <lbalhar@redhat.com> - 3.9-8

  - Disable Python hash seed randomization in %%py_byte_compile

  

file modified
+23 -8
@@ -8,9 +8,24 @@ 

  X_Y = f'{sys.version_info[0]}.{sys.version_info[1]}'

  XY = f'{sys.version_info[0]}{sys.version_info[1]}'

  

+ # Handy environment variable you can use to run the tests

+ # with modified macros files. Multiple files should be

+ # separated by colon.

+ # To get 'em all, run:

+ # ls -1 macros.* | tr "\n" ":"

+ # and then:

+ # TESTED_FILES="<output of previous command>" pytest -v

+ # or both combined:

+ # TESTED_FILES=$(ls -1 macros.* | tr "\n" ":") pytest -v

+ # Remember that some tests might need more macros files than just

+ # the local ones.

+ TESTED_FILES = os.getenv("TESTED_FILES", None)

+ 

  

  def rpm_eval(expression, fails=False, **kwargs):

      cmd = ['rpmbuild']

+     if TESTED_FILES:

+         cmd += ['--macros', TESTED_FILES]

      for var, value in kwargs.items():

          if value is None:

              cmd += ['--undefine', var]
@@ -228,23 +243,23 @@ 

  

  

  def test_py3_shebang_fix():

-     cmd = rpm_eval('%py3_shebang_fix arg1 arg2 arg3')[0]

-     assert cmd == '/usr/bin/pathfix.py -pni /usr/bin/python3 -ka s arg1 arg2 arg3'

+     cmd = rpm_eval('%py3_shebang_fix arg1 arg2 arg3')[-1].strip()

+     assert cmd == '$pathfix -pni /usr/bin/python3 -ka s arg1 arg2 arg3'

  

  

  def test_py3_shebang_fix_custom_flags():

-     cmd = rpm_eval('%py3_shebang_fix arg1 arg2 arg3', py3_shebang_flags='Es')[0]

-     assert cmd == '/usr/bin/pathfix.py -pni /usr/bin/python3 -ka Es arg1 arg2 arg3'

+     cmd = rpm_eval('%py3_shebang_fix arg1 arg2 arg3', py3_shebang_flags='Es')[-1].strip()

+     assert cmd == '$pathfix -pni /usr/bin/python3 -ka Es arg1 arg2 arg3'

  

  

  def test_py3_shebang_fix_empty_flags():

-     cmd = rpm_eval('%py3_shebang_fix arg1 arg2 arg3', py3_shebang_flags=None)[0]

-     assert cmd == '/usr/bin/pathfix.py -pni /usr/bin/python3 -k arg1 arg2 arg3'

+     cmd = rpm_eval('%py3_shebang_fix arg1 arg2 arg3', py3_shebang_flags=None)[-1].strip()

+     assert cmd == '$pathfix -pni /usr/bin/python3 -k arg1 arg2 arg3'

  

  

  def test_py_shebang_fix_custom():

-     cmd = rpm_eval('%py_shebang_fix arg1 arg2 arg3', __python='/usr/bin/pypy')[0]

-     assert cmd == '/usr/bin/pathfix.py -pni /usr/bin/pypy -ka s arg1 arg2 arg3'

+     cmd = rpm_eval('%py_shebang_fix arg1 arg2 arg3', __python='/usr/bin/pypy')[-1].strip()

+     assert cmd == '$pathfix -pni /usr/bin/pypy -ka s arg1 arg2 arg3'

  

  

  def test_pycached_in_sitelib():

Depends on: https://src.fedoraproject.org/rpms/python3.9/pull-request/19

The tests should work on all supported releases with the main python3-devel packages because the oldest one is Fedora 31 with Python 3.7.7 where we backport the mentioned change.

The eval tests only eval RPM macros, not the Shell within.

Unversioned %py_shebang_fix still uses the unversioned script name and therefore depends on python3-devel. I can switch it as well so when __python is defined as __python3 it tries to find the versioned script and use the unversioned version otherwise (with Python2, for example).

BTW, only one package uses these macros — python-MultipartPostHandler2

Build failed.

Unversioned %py_shebang_fix still uses the unversioned script name and therefore depends on python3-devel. I can switch it as well so when __python is defined as __python3 it tries to find the versioned script and use the unversioned version otherwise (with Python2, for example).

Just do the same thing as here please.

BTW, only one package uses these macros — python-MultipartPostHandler2

Yes, the macros are new and not yet documented.

1 new commit added

  • Adapt %py_shebang_fix to use versioned pathfixX.Y.py
3 years ago

Unversioned macro implemented in the latest commit.

Build failed.

The code looks reasonable now, although I have not tested it yet.

See my first comment: The eval tests only eval RPM macros, not the Shell within.

See my first comment: The eval tests only eval RPM macros, not the Shell within.

My plan is to fix tests now when the code looks good to you.

It looks good enough but I still wasn't bale to test it and won't be able today, sorry.

1 new commit added

  • Update tests for new %py[3]_shebang_fix
3 years ago

Is there any way how to recognize that the tests are running in Zuul vs. locally? Any environment variable or something? I am asking because if I want to run the tests on local modified macro files, I have to update the rpm_eval function to load the modified files instead of system ones:

cmd = ['rpmbuild', '--macros', './macros.python3:./macros.python-srpm:./macros.python']

And it might make sense to do it automatically based on some settings.

Build failed.

Fedora CI and Zuul runs Standard Test Interface/Roles:

https://docs.fedoraproject.org/en-US/ci/standard-test-interface/
https://docs.fedoraproject.org/en-US/ci/standard-test-roles/

One way to detect that is to check that the user is root.

However rather than detecting the CI, I suggest a more flexible solution: invent an environment variable, e.g. $TESTED_RPM_MACROS and set --macros to its content when set. That way, we can also manually test with different macro files when needed (e.g. from a rawhide mock when the default macros outside of the scope of this package change). WDYT?

1 new commit added

  • Implement an environment variable to run tests with specific macros
3 years ago

Implementation with an environment variable is ready.

Build failed.

I am not 100% happy with repeating this line in here, but replacing it with a placeholder might make the tests unnecessary complicated, so I think it is fine. OTOH if we are not testing it with custom %__python3value, maybe we can only test the last line?

Suggestion: instead of back-ticks, recommend $(ls ...) (IIRC back-ticks are bad, but I don't recall why)

OTOH if we are not testing it with custom %__python3 value, maybe we can only test the last line?

Given the test now mostly test the copy pasted implementation, I would prefer if the tests were simplified to asserts like this:

    cmd = rpm_eval('%py3_shebang_fix arg1 arg2 arg3')[-1].strip()
    assert cmd == '$pathfix -pni /usr/bin/python3 -ka s arg1 arg2 arg3'

Reposting a comment that I've previously accidenytally posted to another PR:

The code works.

$ mock -r fedora-rawhide-x86_64 --enablerepo=local init
$ mock -r fedora-rawhide-x86_64 --enablerepo=local install https://kojipkgs.fedoraproject.org//work/tasks/5244/47675244/{python-{s,}rpm,python{2,3}-rpm}-macros-3.9-8.fc33.noarch.rpm


$ fedpkg clone python-MultipartPostHandler2
$ cd python-MultipartPostHandler2
$ fedpkg --release master mockbuild -N --enablerepo=local
...
Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.EDlABt
...
+ '[' -f /usr/bin/pathfix3.9.py ']'
+ pathfix=/usr/bin/pathfix.py
+ /usr/bin/pathfix.py -pni /usr/bin/python3 -ka s .
recursedown('.')
recursedown('./MultipartPostHandler2.egg-info')
recursedown('./examples')
./MultipartPostHandler.py: no change
./setup.py: no change
./examples/MultipartPostHandler-example.py: updating
...

$ wget https://fedora.softwarefactory-project.io/logs/19/19/a81e14d702b6a1a45ee71015735864ea7488b2f1/check/rpm-scratch-build/f5e9270/buildset/python3-devel-3.9.0~b5-1.fc33.x86_64.rpm
$ mock -r fedora-rawhide-x86_64 --remove python3-devel
$ mock -r fedora-rawhide-x86_64 --enablerepo=local --install python3-devel-3.9.0~b5-1.fc33.x86_64.rpm

$ fedpkg --release master mockbuild -N --enablerepo=local
...
Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.ai1Uwv
...
+ '[' -f /usr/bin/pathfix3.9.py ']'
+ pathfix=/usr/bin/pathfix3.9.py
+ /usr/bin/pathfix3.9.py -pni /usr/bin/python3 -ka s .
recursedown('.')
recursedown('./MultipartPostHandler2.egg-info')
recursedown('./examples')
./MultipartPostHandler.py: no change
./setup.py: no change
./examples/MultipartPostHandler-example.py: updating

rebased onto d135b7e30d2469e0f98d2e65937df21492a133c4

3 years ago

Updated, rebased but still waiting for python3.9.

Build succeeded.

I would prefer to see the 3 related commits squashed and only the env var separated, but I won't block the review on this.

However, please add some rationale to the commit message. It explains what happens, not why.

Also, note that we can ship this before the python3.9, the if will make it work before and after that chnage.

rebased onto 1979a78

3 years ago

Squashed, updated, no more WIP.

Build succeeded.

Pull-Request has been merged by churchyard

3 years ago