Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change shebangs with Tools/scripts/pathfix.py? #24

Open
hroncok opened this issue Aug 31, 2017 · 23 comments
Open

Change shebangs with Tools/scripts/pathfix.py? #24

hroncok opened this issue Aug 31, 2017 · 23 comments

Comments

@hroncok
Copy link
Member

hroncok commented Aug 31, 2017

There is a script in /usr/lib64/python3.6/Tools/scripts/pathfix.py that changes shebangs. We may want to:

  • recommend that in the shebang sections of this guide
  • use it automagically in rpm builds to fix shebangs? (with similar logic to pybytecompile script)

I'm opening this here, because of the first above point. If we say we want to use this automatically, we can discuss this in some ML.

Example usage:

%install
...
%{python3_sitearch}/Tools/scripts/pathfix.py -i "%{__python3} -s" -p %{buildroot}%{_bindir}/*

Note: A small problem with this script is that it only converts files where the name matches r'^[a-zA-Z0-9_]+\.py$' when called on directory, but you can call it directly on files.

@hroncok
Copy link
Member Author

hroncok commented Sep 18, 2017

@encukou @torsava bump?

@encukou
Copy link
Member

encukou commented Sep 18, 2017

I think we should package this properly – into /usr/bin as part of python3-devel – and then recommend it. The %{python3_sitearch}/Tools/scripts directory is horrible and I'd like to get rid of it eventually.
If we run this automatically, we need to be really careful about the shebangs it generates: the -s will not combine cleanly with other options.

@torsava
Copy link
Member

torsava commented Sep 18, 2017

Is the file name limit a bug or a feature? I'd fix it before we recommend it.

@encukou
Copy link
Member

encukou commented Sep 18, 2017

@torsava, I think adding a CLI option to act on all files (or all executable files?) would have a good chance of being accepted ustream.
See issue10140 and python/cpython@7cd94b8 for a similar change.

@torsava
Copy link
Member

torsava commented Sep 18, 2017

So, do we want to act on all files with the extension .py, or all executable files that have the word python in the shebang line?

I'm more inclined towards the first.

@encukou
Copy link
Member

encukou commented Sep 18, 2017

On further thought, for Fedora the tool might be OK as it is. Files in /usr/lib should not be executable and should not have shebangs anyway. And files in /usr/bin don't need the directory traversal.

(Also, setuptools gives correct shebangs anyway – this is only for other buildsystems.)

@torsava
Copy link
Member

torsava commented Sep 18, 2017

On further thought, for Fedora the tool might be OK as it is. Files in /usr/lib should not be executable and should not have shebangs anyway. And files in /usr/bin don't need the directory traversal.

So we advise to do ... pathfix.py -i .... %{_bindir}/* ? That does sound alright.

@hroncok
Copy link
Member Author

hroncok commented Sep 18, 2017

I think we should package this properly – into /usr/bin as part of python3-devel – and then recommend it. The %{python3_sitearch}/Tools/scripts directory is horrible and I'd like to get rid of it eventually.

That of course was my intention as well.

@hroncok
Copy link
Member Author

hroncok commented Sep 19, 2017

I'll craft a pull-request for python3.spec. What executable name? /usr/bin/pathfix (that one seems to be free in Fedora)?

@hroncok
Copy link
Member Author

hroncok commented Sep 19, 2017

I went with /usr/bin/pathfix for now: https://src.fedoraproject.org/rpms/python3/pull-request/13

@ignatenkobrain
Copy link

@hroncok I'm kinda reluctant requiring python3-devel in buildroot just to change shebang..

@hroncok
Copy link
Member Author

hroncok commented Nov 3, 2017

Well since this was originally opened for Python only (unlike rpm-software-management/rpm#344), it wasn't a problem for me. Feel free to go with Shell instead.

@hroncok
Copy link
Member Author

hroncok commented Nov 3, 2017

We gonna need this anyway to get rid of /usr/bin/python, unless you want rpm-software-management/rpm#344 to have language specific hacks.

@torsava
Copy link
Member

torsava commented Nov 13, 2017

So to sum up: pathfix.py is already under /usr/bin/ in python3-devel, and all Python packages have to BR python3-devel anyway, therefore there's nothing stopping us from recommending using the script in this guide and closing this issue. Any objections, @ignatenkobrain ?

@ignatenkobrain
Copy link

ignatenkobrain commented Nov 13, 2017 via email

@encukou
Copy link
Member

encukou commented Nov 13, 2017

Can the global solution can change /usr/bin/python to /usr/bin/python2? That sounds to me like a Python-specific problem. Or should the global solution do things like this?

(Apologies if this was answered before. If it was, the answer wasn't explicit enough for me.)

@encukou
Copy link
Member

encukou commented Nov 13, 2017

Additionally, that change should only be done for Python 2, not with Python 3. I don't think a global tool should be customizable (=complicated) enough for that...

@ignatenkobrain
Copy link

Can the global solution can change /usr/bin/python to /usr/bin/python2?

Why not? There is implemented generic mechanism of changing shebangs. And brp stands for BuildRoot Policy, so it is up to distros how to change shebangs for python/python2/python3.

@torsava
Copy link
Member

torsava commented Nov 13, 2017

To separate the two intertwined issues currently under discussion:

1. "recommend that in the shebang sections of this guide"

I don't think brp-mangle-shebangs (rpm-software-management/rpm#344) is a good choice for this, however, @ignatenkobrain is preparing a new RPM macro specifically for changing shebangs, which should be ready within a month. I would personally opt to recommend this over pathfix.py, as it's not Python specific and thus packagers that learn about it here can use it for all their other Fedora packages as well.

2. "use it automagically in rpm builds to fix shebangs? (with similar logic to pybytecompile script)"

Here brp-mangle-shebangs would be a good choice, but I don't think this is a good idea overall. If somebody wants to pursue it, let's first discuss it on some mailing lists with the community.

@hroncok
Copy link
Member Author

hroncok commented Nov 14, 2017

So to sum up: pathfix.py is already under /usr/bin/ in python3-devel

I'm afraid this is rawhide only anyway.

@torsava
Copy link
Member

torsava commented Nov 14, 2017

So to sum up: pathfix.py is already under /usr/bin/ in python3-devel
I'm afraid this is rawhide only anyway.

You're right, that even strengthens the argument for the macro.

stratakis pushed a commit to fedora-python/python36 that referenced this issue Nov 30, 2017
@hroncok
Copy link
Member Author

hroncok commented Sep 20, 2018

This is now in all Fedoras and in EPEL7.

@hroncok
Copy link
Member Author

hroncok commented Sep 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants