More about our motivation and agenda: https://fedoraproject.org/wiki/FinalizingFedoraSwitchtoPython3

We now have two tickets about this, which is too many. I know I've seen this script before.

In any case, I don't have any issue with automatic mangling of shebang lines as long as it's well documented (unlike a lot of the current magic that happens in brp-* scripts) and there is an easy and documented method for disabling it. I looked at the current PR I believe it fails on both accounts.

Chatted with Igor a bit on IRC about this. Since there's both a PR and this ticket it gets tough to decide what discussion should go, and different people will see each comment. So I'll just have to cut-n-paste.

In any case, on the issue of disabling the script, there's a problem because there is no standard way to disable brp-* scripts and apparently Panu has requested that Fedora not ad-hoc start adding them until RPM upstream has chosen a method. But my opinion is that they've had 20 years to do that and haven't, and I simply don't think this should be added without any means to disable it because:

• This affects 1500+ packages. Can we be absolutely sure it doesn't break any of them?
• I personally can't be sure that there isn't some valid use of env which would be wiped out here.
• At least one prominent packager objects very strongly to any prohibition on /usr/bin/env, and simply making it automatic without the potential to be disabled is a rather underhanded way to bypass any complaints about the guidelines. Yes, /usr/bin/env is forbidden by the guidelines, but you can bypass a guideline if you need to. That's significantly different from just wiping env from existence.

So, I'll propose making it conditional on %brp_mangle_shebangs, and have that set to true by default. Then if you don't want it, you just do %undefine %brp_mangle_shebangs. And if RPM upstream does choose a different way to deal with it, then we'll just have to fix the few packages which might be using it.

As to the documentation, I realize that no matter how much you put in the script nobody's ever going to know to look there, but I do think there should at least be something. I can ever find any more time I can add a page or something which talks about what RPM does behind your back after %install and what you can do about it. That would be filled in using information from the documentation in the script.

How to prevent the BRP script changing shebangs for files I don't want?

@vondruch

Easy way: Don't set them executable (I forgot to mention that).

Complicated way if the one is not enough: Redefine os install post

So, I'll propose making it conditional on %brp_mangle_shebangs, and have that set to true by default. Then if you don't want it, you just do %undefine %brp_mangle_shebangs. And if RPM upstream does choose a different way to deal with it, then we'll just have to fix the few packages which might be using it.

I can do that.

At least one prominent packager objects very strongly to any prohibition on /usr/bin/env...

Feel free to go with option B. That wouldn't affect his use case, because it would only change Python.

As to the documentation, I realize that no matter how much you put in the script nobody's ever going to know to look there, but I do think there should at least be something.

I'll gladly accompany this new brp script with some notes to the guidelines. Based on the option we use, the place and content would differ. (E.g. B and C would go to Python guidelines, A would go next to the env guideline + some bits to Python guidelines.)

I've added this to the original post:

Note: All options only mangle shebangs in executable files.

@tibbs This was historically in the script:

 %{!?__jar_repack:/usr/lib/rpm/redhat/brp-java-repack-jars}

Should I do it in similar fashion?

 %{!?__disable_brp_mangle_shebangs:/usr/lib/rpm/redhat/brp-mangle-shebangs}

It seems easier to check for non-availability of the macro and let people set one.

There are several pre-existing examples, with the only common thing between them being that they are all different... (see
py_auto_byte_compile, __debug_package and of course the late __jar_repack one)

One possibility would be introducing a common mangling theme like this:

%__brp_mangle_shebangs /usr/lib/rpm/redhat/brp-mangle-shebangs

...and then:

%__os_install_post    \
   %{?__brp_mangle_shebangs} \
   %{?__brp_this} \
   %{?__brp_that} \
   ...

...and then to disable you just undefine that macro (or you can override it to use a custom one)

I don't have a strong opinion. I'm just unsure where is the right place for the definition if we go that way.

Yeah there'd be some inconsistencies and migration pains no matter what kind of scheme gets selected...

...but lets just say that you could do a whole lot worse than going by that example, even if you only do it for the shebang-mangler.

FWIW to get things moving, I converted the existing brp-scripts to use a common theme to the extent possible without breaking stuff (hopefully):
https://src.fedoraproject.org/rpms/redhat-rpm-config/c/c4646d791dea6b976fd5a04e2562c7b21b9389e0?branch=master

It is now possible to disable this BRP by unsetting %__brp_mangle_shebangs or setting it to /usr/bin/true.

At this point I have no real objection to this and would like to move it forward. I think we should indeed document all of the brp_* stuff but whether that ends up being part of the packaging guidelines or elsewhere (even a file installed as part of redhat-rpm-config) isn't something we should worry about here.

Before just pushing this into redhat-rpm-config, though, I was hoping to get some input from other FPC folks.

In addition, to make sure there aren't any obvious problems I'm gearing things up to do a rebuild of at least a fair portion of rawhide to look for warnings or failures generated by the proposed script. It's been a while since I last did this so my tools for it need to be brought up to snuff. (I have something which spreads mockbuilds out over a couple hundred hosts, but it bitrotted a bit.) I do hope to have some useful results soon. I don't expect any real issues but I think it's fair to at least try.

@tibbs at the moment of writing initial brp I rebuilt standard buildroot and didn't spot any problems.

Guidelines/documentation proposal:

To the end of https://fedoraproject.org/wiki/Packaging:Guidelines#Shebang_lines please add:

Since Fedora 28, shebangs are automatically mangled. All env shebangs are converted to their correct /usr/bin/... counterpart. You might see the following warnings in the build log:

*** WARNING: mangling shebang in ./usr/share/gems/gems/builder-3.2.3/test/test_xchar.rb from #!/usr/bin/env ruby to #!/usr/bin/ruby

You SHOULD fix those shebangs manually to remove the warning.

If this mangling breaks your package, you can turn it off by putting %undefine __brp_mangle_shebangs into your spec. You MUST accompany the line with an explaining comment about why is this needed.

Into https://fedoraproject.org/wiki/Packaging:Python#Multiple_Python_Runtimes after "...used in shebang lines or as a dependency of a package." please add:

All forbidden ambiguous python shebangs are converted to /usr/bin/python2 since Fedora 28. You might observe the following warnings in the build log:

*** WARNING: mangling shebang in ./usr/bin/taskotron_result from #!/usr/bin/python to #!/usr/bin/python2

You MUST fix those shebangs manually to remove the warning to comply with the above guidelines.

Feel free to reword this as you see fit.

There's an interesting question here: If this is being done automatically, why make packagers duplicate the effort by telling them to fix the shebangs themselves?

Certainly don't do things silently. But does this even need to be a WARNING as opposed to just an informative line? And if it is something that people need to be alerted about, why bother fixing anything automatically? Just fail the build instead. That way things would actually get fixed.

As it is now, I can't imagine many people actually fixing this just to quiet some warning deep down in the build logs since in the end they get an identical package and a nastier spec file. And I honestly don't think they should.

Edit: OK, that does sort of ignore <= F27 and EPEL. But as long as nobody takes out any existing shebang mangling, the situation doesn't get any worse for those releases, and simply evaporates for F28+.

Edit2: Left out a word somehow.

As it is now, I can't imagine many people actually fixing this just to quiet some warning deep down in the build logs since in the end they get an identical package and a nastier spec file. And I honestly don't think they should.

As for env, this make sense, so we can change it to INFO and don't bother anyone with fixing. For python, the situation is different because if they don't fix it, we cannot ever change it to python3. We can consider making it hard fail later.

So I guess this builds down to printing "INFO" or something when something is changed, except for the two cases of unversioned python usage:

• #!/usr/bin/env python
• #!/usr/bin/python

Those would be a warning and potentially a failure. Eventually we could even have %_unversioned_python_terminate_build (following the existing examples of %_missing_build_ids_terminate_build and %_python_bytecompile_errors_terminate_build).

I agree. I won't get to the implementation changes until tomorrow though.

Updated guidelines/documentation proposal (implementation changed accordingly):

To the end of https://fedoraproject.org/wiki/Packaging:Guidelines#Shebang_lines please add:

Since Fedora 28, shebangs are automatically mangled. All env shebangs are converted to their appropriate /usr/bin/... counterpart. You don't have to change them manually. If this mechanism breaks your package, you can turn it off by putting %undefine __brp_mangle_shebangs into your spec. If used, you MUST accompany the line with an explaining comment about why is this needed.
You can also opt out for specific paths or shebangs only be defining __brp_mangle_shebangs_exclude_from or __brp_mangle_shebangs_exclude following similar rules as in Packaging:AutoProvidesAndRequiresFiltering. Only executable files are mangled, removing the executable permission MUST be preferred over utilizing the exclude macros where possible.

Into https://fedoraproject.org/wiki/Packaging:Python#Multiple_Python_Runtimes after "...used in shebang lines or as a dependency of a package." please add:

All forbidden ambiguous python shebangs are automatically converted to /usr/bin/python2 since Fedora 28. You might observe the following warnings in the build log:

*** WARNING: mangling shebang in ./usr/bin/taskotron_result from #!/usr/bin/python to #!/usr/bin/python2. This will become an ERROR, fix it manually!

You MUST fix those shebangs manually to remove the warning to comply with the above guidelines. The warnings will eventually become errors and will fail the build.

For opting out (e.g. for templates) see XXX LINK TO THE ABOVE XXX.

Feel free to reword this as you see fit.

Changelog:

1. changed "correct" to "appropriate"
2. added notes about __brp_mangle_shebangs_exclude_from and __brp_mangle_shebangs_exclude

All env shebangs are converted to their correct /usr/bin/... counterpart.

I might be wrong, but the script does not check if the /usr/bin counterpart exists, so the "correct" might not be precise. Sorry for nitpicking.

It doesn't have to exist during the buildtime, so there is no way to cheek. Changed to "appropriate".

@tibbs can we please make this happen before the mass rebuild?

Does this need tagging with "meeting"?

@mbooth I think this needs draft (if there is none already).

https://pagure.io/packaging-committee/issue/738#comment-490366

Added notes about __brp_mangle_shebangs_exclude_from and __brp_mangle_shebangs_exclude to the draft.

We looked at this ticket this week (https://meetbot-raw.fedoraproject.org/fedora-meeting-1/2018-03-15/fpc.2018-03-15-16.00.txt):

• x738 Mangling shebangs (geppetto, 16:09:02)
• LINK: https://pagure.io/packaging-committee/issue/738#comment-490366
  actually. (limburgher, 16:12:21)
• ACTION: Mangling shebangs (+1:5, 0:0, -1:0) (geppetto, 16:16:41)

Sorry, I'm a little late to this discussion. According to the Python Developer's Guide, the /usr/bin/python shebang is valid for scripts that are compatible with both versions of python:

"
- for the time being, all distributions should ensure that python refers to the same target as python2.
- however, end users should be aware that python refers to python3 on at least Arch Linux (that change is what prompted the creation of this PEP), so python should be used in the shebang line only for scripts that are source compatible with both Python 2 and 3.
"

https://www.python.org/dev/peps/pep-0394/#recommendation

I bring this up because we have python scripts in ZFS that are compatible with both versions of python, and we don't necessary want to pick one.

We are monitoring that PEP, but we are trying to prepare Fedora for the time when this PEP changes. While it is perfectly valid for upstream to use python shebang that indicates "supports both", in Fedora packages we need to make sure it always runs on a specific one, because RPM packages have dependencies and those are not python version agnostic.

Also we want to run as much as possible on 3. So if you support 3, please use that in Fedora packages. You can use pathfix.py (from python3-devel) to fix the shebangs.

Announcement text:

Information about the automatic shebang line checking and modification has been added to both the main guidelines and the Python guidelines.

• https://fedoraproject.org/wiki/Packaging:Guidelines#Shebang_lines
• https://fedoraproject.org/wiki/Packaging:Python#Multiple_Python_Runtimes
• https://pagure.io/packaging-committee/issue/738