Bug 1335203 - %pyX_build breaks shebang when upstream already uses some flags
Summary: %pyX_build breaks shebang when upstream already uses some flags
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: python-rpm-macros
Version: 33
Hardware: Unspecified
OS: Unspecified
low
medium
Target Milestone: ---
Assignee: Miro Hrončok
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 1685582
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-05-11 14:56 UTC by John Dennis
Modified: 2020-10-13 09:05 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-10-13 09:05:19 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Python 27004 0 None None None 2016-05-11 19:39:54 UTC

Description John Dennis 2016-05-11 14:56:02 UTC
The problem occurs with Python packages that install a script into /usr/bin. If the script includes an interpreter argument in it's shebang line the installed script cannot execute and aborts. For example if the source script has a shebang line like this:

#!/usr/bin/python -E

The installed script will have a shebang line like this (using Py2 as an example)

#!/usr/bin/python2 -s -E

The problem is how the shebang line is parsed to build parameters for exec. Unfortunately there is no standard. Some operating systems split at whitespace and treat the first part as the path to the interpreter and the rest as individual arguments. Some operating systems split at the first whitespace and treat the front part as the path to the interpeter and the rest as a single argument (this is what happens in Linux).

Thus when the Python interpreter is run it sees argv[1] as "-s -E" which it doesn't understand as a single argument and the result is:

Unknown option: -
usage: /usr/bin/python [option] ... [-c cmd | -m mod | file | -] [arg] ...
Try `python -h' for more information.

The cause of the problem is in /usr/lib/rpm/macros.d/macros.python2 and /usr/lib/rpm/macros.d/macros.python3. I'll show the Py2 version but the Py3 version is equivalent.

%py2_shbang_opts -s

%py2_build() %{expand:\
CFLAGS="%{optflags}" %{__python2} %{py_setup} %{?py_setup_args} build --executable="%{__python2} %{py2_shbang_opts}" %{?1}\
}

Then in distutils/command/build_scripts.py it uses the first_line_re regular expression to split the shebang line into 2 parts, the interpreter and the interpreter arguments. It then replaces the shebang line with the value of --executable followed by a space and interpreter arguments the regexp found. But in our case the interpreter is *not* the path to an interpreter, rather it's the path to the interpreter *plus* the %py2_shbang_opts. Thus you end up with multiple arguments to the interpreter, but the interpreter doesn't see it as multiple arguments, instead it sees 1 argument, a single string containing "-s -E".

I think the fundamental problem is that macros.python{2,3} is abusing the --executable argument, it's meant to be the path to an executable *only*. It clearly does not expect extra arguments.

If build_scripts.py had logic to merge arguments together, e.g. -s -E would become -sE then all would be good, but it doesn't.

Comment 1 Orion Poplawski 2016-05-11 16:01:07 UTC
I'll just note that a workaround is to redefine the %py/py2/py3_shbang_opts to -sE. But yeah, we should try to handle this better if possible.

Comment 2 Orion Poplawski 2016-05-11 16:01:46 UTC
Actually, you would need to define it to %{nil} and add "s" to the script.

Comment 3 Orion Poplawski 2016-05-11 20:00:48 UTC
Really need more python people to look at this.

Comment 4 John Dennis 2016-05-11 20:18:26 UTC
The immediate fix is to not have RPM adding interpreter arguments. It seems wrong to me to have RPM making decisions like this. Isn't it the province of the script author to decide the proper interpreter invocation?

Decisions about whether a script should execute with -s or -E or for that mater any other option is something that should be caught in the package review process.

Comment 5 Orion Poplawski 2016-05-11 20:46:12 UTC
For some background:
https://fedorahosted.org/fpc/ticket/513
https://fedorahosted.org/fpc/ticket/281

The goal was to automatically make system python scripts' behavior more predictable (and possibly more secure).  I think the goal to have every system python script use -s is still laudable.  But clearly our implementation is broken.

Serhiy Storchaka's suggestion in the upstream report, while somewhat ugly, is perhaps worth considering:

- Add a wrapper /usr/bin/python2-s and pass it to to --executable.

$ cat /usr/bin/python2-s
#!/bin/sh
exec /usr/bin/python2 -s "$@"

Comment 6 Petr Viktorin (pviktori) 2016-05-12 08:53:33 UTC
Hmm. I wonder if that should be /usr/libexec/system-python2 instead.

Comment 7 Miro Hrončok 2016-05-12 11:11:47 UTC
(In reply to Petr Viktorin from comment #6)
> Hmm. I wonder if that should be /usr/libexec/system-python2 instead.

That would make all our Python 2 scripts to use /usr/libexec/system-python2,
but only some fo the Python 3 scripts to use /usr/libexec/system-python, it would be a mess.

Comment 8 Fedora End Of Life 2016-11-25 09:00:22 UTC
This message is a reminder that Fedora 23 is nearing its end of life.
Approximately 4 (four) weeks from now Fedora will stop maintaining
and issuing updates for Fedora 23. It is Fedora's policy to close all
bug reports from releases that are no longer maintained. At that time
this bug will be closed as EOL if it remains open with a Fedora  'version'
of '23'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not 
able to fix it before Fedora 23 is end of life. If you would still like 
to see this bug fixed and are able to reproduce it against a later version 
of Fedora, you are encouraged  change the 'version' to a later Fedora 
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events. Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

Comment 9 Fedora End Of Life 2017-11-16 19:16:43 UTC
This message is a reminder that Fedora 25 is nearing its end of life.
Approximately 4 (four) weeks from now Fedora will stop maintaining
and issuing updates for Fedora 25. It is Fedora's policy to close all
bug reports from releases that are no longer maintained. At that time
this bug will be closed as EOL if it remains open with a Fedora  'version'
of '25'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version'
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not
able to fix it before Fedora 25 is end of life. If you would still like
to see this bug fixed and are able to reproduce it against a later version
of Fedora, you are encouraged  change the 'version' to a later Fedora
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's
lifetime, sometimes those efforts are overtaken by events. Often a
more recent Fedora release includes newer upstream software that fixes
bugs or makes them obsolete.

Comment 10 Ben Cotton 2018-11-27 18:13:01 UTC
This message is a reminder that Fedora 27 is nearing its end of life.
On 2018-Nov-30  Fedora will stop maintaining and issuing updates for
Fedora 27. It is Fedora's policy to close all bug reports from releases
that are no longer maintained. At that time this bug will be closed as
EOL if it remains open with a Fedora  'version' of '27'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not 
able to fix it before Fedora 27 is end of life. If you would still like 
to see this bug fixed and are able to reproduce it against a later version 
of Fedora, you are encouraged  change the 'version' to a later Fedora 
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events. Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

Comment 11 Miro Hrončok 2018-11-27 20:19:12 UTC
We need to look at this properly again. I'll take it, but I won't have time until the end of the year.

Comment 12 Miro Hrončok 2018-12-09 19:00:58 UTC
Strikes back https://src.fedoraproject.org/rpms/linkchecker/pull-request/2

Comment 13 Miro Hrončok 2018-12-09 19:47:18 UTC
I consider /usr/bin/pythonX-s or /usr/libexec/pythonX-s particularly ugly. Also defining a new Python entrypoint after all the shenanigans with executables and shebangs we already had will probably actually make some people want us to burn in hell. Hence I consider it a no go.

We should fix disturils/setuptools ar whatever is taking the --executable argument to accept interpreter with flags properly.

I'll read all the upstream discussions about it we had years ago and hopefully propose a patch upstream in the linked issue.

Comment 14 Miro Hrončok 2019-01-10 09:26:21 UTC
Sorry, wrong bug.

Comment 15 Ben Cotton 2019-02-19 17:12:09 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 30 development cycle.
Changing version to '30.

Comment 16 Petr Viktorin (pviktori) 2019-03-19 14:33:20 UTC
If anyone wants to take this, help wanted.
Sorry for hogging the bug.

Comment 17 Jason Tibbitts 2019-03-19 16:47:11 UTC
I don't see a good way out of this.  It seems that defaulting %py3_shbang_opts to '-s' is simply not something we should be doing at all since it can't be done safely.

I suppose it would be technically possible to detect and fix this case up in brp-mangle-shebangs, which still isn't part of python-rpm-macros but at least is Fedora-specific.  But it makes me uncomfortable to have one piece of the system deliberately introduce a bug with the assumption that another piece will magically fix it.

One other possibility is to get rid of %py3_shbang_opts entirely and then leave adding '-s' to brp-mangle-shebangs.  And it sure would be nice if we could also decide if it's 'shbang' or 'shebang' at some point.

But neither of those seem all that satisfactory to me, and if we really want -s on by default then it should probably be done in python itself or by using an alternate executable which adds it as suggested previously.

Comment 18 Miro Hrončok 2019-03-19 17:10:19 UTC
I believe distutils can safely merge two sets of python flags (mot of them, really).

However given bz1685582 would not use that code, this might not be worth fixing.

Instead, adding -s in brp-mangle-shebangs (or any other brp script) might be the way.

Another Python executable is something I want to avoid.

Comment 19 Petr Viktorin (pviktori) 2019-04-02 13:15:00 UTC
We can add it to setuptools or a brp script, but either way, someone will need to write the code to merge two sets of flags (and error out if it's impossible).

Comment 20 Miro Hrončok 2019-04-18 09:47:36 UTC
See https://bugzilla.redhat.com/show_bug.cgi?id=1685582#c9

Comment 21 Patrik Kopkan 2019-07-23 13:23:58 UTC
This PR will add support to pathfix (python script for changing shebangs) option for adding single letter flag. It will get to the %pyproject macros will use it.

https://github.com/python/cpython/pull/13591

Comment 22 Patrik Kopkan 2019-09-17 13:16:04 UTC
PR moved here: https://github.com/python/cpython/pull/15717

Comment 23 Petr Viktorin (pviktori) 2019-10-15 13:10:57 UTC
The PR was merged, we should backport it to Pythons 3.7 and 3.8.

Comment 24 Petr Viktorin (pviktori) 2019-10-29 14:07:45 UTC
Correction: it should already be in 3.8. We need to backport to 3.7.

Comment 25 Patrik Kopkan 2020-01-07 14:37:40 UTC
Pr is backported to 3.7. I will adapt this next week to pyproject-rpm-macros.

Comment 26 Miro Hrončok 2020-02-05 13:47:12 UTC
Here is an open PR that solves the problem in pyproject-rpm-macros: https://src.fedoraproject.org/rpms/pyproject-rpm-macros/pull-request/30

Comment 27 Petr Viktorin (pviktori) 2020-02-18 14:37:34 UTC
The PR is merged now.

Comment 28 Petr Viktorin (pviktori) 2020-03-03 14:04:33 UTC
The new macros solve this, and it's not likely that we'll fix %pyX_build.
This should be documented in the new packaging guidelines.

Comment 29 Ben Cotton 2020-04-30 20:23:35 UTC
This message is a reminder that Fedora 30 is nearing its end of life.
Fedora will stop maintaining and issuing updates for Fedora 30 on 2020-05-26.
It is Fedora's policy to close all bug reports from releases that are no longer
maintained. At that time this bug will be closed as EOL if it remains open with a
Fedora 'version' of '30'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not 
able to fix it before Fedora 30 is end of life. If you would still like 
to see this bug fixed and are able to reproduce it against a later version 
of Fedora, you are encouraged  change the 'version' to a later Fedora 
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events. Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

Comment 30 Petr Viktorin (pviktori) 2020-08-11 09:16:20 UTC
This should be documented in the *old* packaging guidelines.

The new ones won't cover %pyX_build.

Comment 31 Ben Cotton 2020-08-11 15:35:23 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 33 development cycle.
Changing version to 33.

Comment 32 Petr Viktorin (pviktori) 2020-10-13 09:05:19 UTC
Realistically, we'll never get to fixing this.


Note You need to log in before you can comment on or make changes to this bug.