[RFC] option to skip 03-rewrite-python-shebang

[tornaria]

For #34030 we need to disable pre-pkg/03-rewrite-python-shebang, since sagemath ships its own python and rewriting shebangs breaks it (we will work on un-vendoring python, but that's a different story).

This PR adds an option no_python_shebang=yes to skip running this hook, similar to nocross=yes.

If this goes through, it will have to be added to Manual.md and to xlint, but I want to get feedback before.

Maybe, as an alternative, a way to skip hooks by name would be more generally useful? Something like skip_hook="pre-pkg/03-rewrite-python-shebang" with a space separated list of hooks to skip. That shouldn't be too hard to implement in run_pkg_hooks().

[ericonr]

My position here is that sage shouldn't be merged into void as long as it's using built-in python.

Most of the hooks exist for good reason, and I don't think this change has enough value.

A variable to skip specific hooks is also fragile: sometimes we need to change the order they run at, and that means changing their name.

[dkwo]

Will this be necessary after https://trac.sagemath.org/ticket/30766 is completed?
Namely, also for python modules?

[leahneukirchen]

I agree we should use system Python, but it's targetted for Sage 9.5.

[tornaria]

My position here is that sage shouldn't be merged into void as long as it's using built-in python.

Fair enough, I tend to think the same. Hopefully it will work with python 3.10 by the time they release sagemath-9.5. If it doesn't, though, it may mean another 6 months wait for sagemath-9.6.

Most of the hooks exist for good reason, and I don't think this change has enough value.

Doesn't scanning and replacing all python shebangs cause issues in any other situations? In this case we are mostly replacing "#! /usr/bin/env python3" or worse "#! /usr/bin/env sage-python" (which complains about lack of python version and aborts).

In particular this is NOT about hardcoding paths to the python interpreter but allowing it to run through env.

It might be argued that rewriting "#! /usr/bin/env sage-python" is incorrect.

Also: for packages with lots of files (and/or huge text files?) this hook takes its time... AFAICT it's not only checking in /usr/bin or +x files but every single non-binary file is scanned.

A similar situation is 11-pkglint-elf-in-usrshare, but for that case we have "ignore_elf_dirs=". Example package pari-galpol contains 14681 files and using nostrip=true plus ignore_elf_dirs=/usr/share/pari saves ~ 3m in install step. It may not sound much for CI, but when testing changes locally it's a workflow killer.

A variable to skip specific hooks is also fragile: sometimes we need to change the order they run at, and that means changing their name.

That's a good point. It could be made different, e.g. skip_hooks="pre-pkg:rewrite-python-shebang" where the first part matches a stage exactly and the second part matches partially, so this is robust to change in the numbers. But I understand this gets a bit hairy...

Would one of the following be acceptable:
a. fix the pattern in 03-rewrite-python-shebang so it doesn't match "#!/usr/bin/env sage-python".
b. add an option "ignore_python_shebang_dirs" similar to "ignore_elf_dirs"

[ericonr]

Doesn't scanning and replacing all python shebangs cause issues in any other situations?

There used to be a bug with replacing it without taking into account the flags passed to the interpreter in the shebang, but that's been fixed.

In particular this is NOT about hardcoding paths to the python interpreter but allowing it to run through env.

That is the exact purpose of the hook. It means system packages aren't affected by alternative python executables in PATH, and that the xbps-alternatives selected python doesn't change what's used for a given script. Since an alternative python will usually look for modules in a different place from /usr/lib/python${py3_ver} as well, this means that programs which depend on packaged modules continue working. (way back when, installing conda provided Python on ubuntu completely hosed a bunch of GUI programs for me, since they could no longer import gi, for example)

It might be argued that rewriting "#! /usr/bin/env sage-python" is incorrect.

How does this break? Where is sage-python located?

Also: for packages with lots of files (and/or huge text files?) this hook takes its time... AFAICT it's not only checking in /usr/bin or +x files but every single non-binary file is scanned.

Yup, there are packages with scripts under /usr/share, for example.

A similar situation is 11-pkglint-elf-in-usrshare, but for that case we have "ignore_elf_dirs=".

ignore_elf_dirs is about policy: that directory can have ELF files because they aren't arch specific or because they have to go there. It's not about speedup, and it bothers me slightly that pari-galpol is using it for that purpose. The purpose of hooks is to always be there to catch things that we may miss; manually adding a bunch of skips for speed rather than policy feels wrong to me.

a. fix the pattern in 03-rewrite-python-shebang so it doesn't match "#!/usr/bin/env sage-python".

I can kinda see the case for that, since it isn't one of the two "official" python interpreters.

b. add an option "ignore_python_shebang_dirs" similar to "ignore_elf_dirs"

I don't think this is a valid option.

[tornaria]

@ericonr thanks for your detailed answer. Let me add a few things:

• I misread the code and in fact #!/usr/bin/env sage-python is not rewritten. The first grep will match that, but there is a more precise regexp in the loop that will only match python when preceded by space or / and followed by numbers.
• the hook was failing for sagemath because some shebangs refer to python without a version. That's not a bug: these are standalone binaries intended to run with whatever python is already installed in the system, regardless of version (sage will force install python3 if only python2 is available; these binaries may run before that).
• adding python_version=3 fixes that breakage in a proper way.
• allowing xbps-src to rewrite the shebangs doesn't cause terrible breakage (maybe some things are broken, I don't know yet, but if so they are relatively minor). Maybe sage is not relying on shebangs to run python files that must be run on its own venv with its own python.

I think we can close this PR for now and I'll probably drop the commit from the sagemath branch as well.

If / when we discover issues with some of the rewritten shebangs, we can try to find workarounds.