#9 Add brp-mangle-shebangs
Merged 2 years ago by tibbs. Opened 2 years ago by churchyard.
rpms/ churchyard/redhat-rpm-config brp-mangle-shebangs  into  master

file added
+54

@@ -0,0 +1,54 @@ 

+ #!/bin/sh -eu

+ 

+ # If using normal root, avoid changing anything.

+ if [ -z "$RPM_BUILD_ROOT" -o "$RPM_BUILD_ROOT" = "/" ]; then

+   exit 0

+ fi

+ 

+ cd "$RPM_BUILD_ROOT"

+ 

+ trim() {

+   printf '%s' "$*"

+ }

+ 

+ fail=0

+ for f in $(find -executable -type f | xargs --no-run-if-empty file -N --mime-type | grep -Po "^\K.+(?=: text/)"); do

Should we depend on findutils and file now?

file is utterly unreliable. You need to use something else.

@fweimer please elaborate this. unreliable in what way?

@praiskup all those sem to be in the buildroot, but I can add those explicitly

Added in extra commit, @tibbs feel free to merge with or without

tibbs commented 2 years ago

If file triggers improperly, as far as I can tell the only thing which would happen is that the script fails to fix up a shebang line. For any damage to occur, you would have to have a file which is executable, detected as some kind of text, which starts with a shebang calling env or python but which is somehow not a script that needs fixing. I do not believe this is possible.

It may turn out that this does miss more instances of #!env or unversioned python than we would like, and those would indeed need fixing. But I don't think it needs to catch 100% of them as long as it doesn't change anything which it's not supposed to change.

+ 

+   ts=$(stat -c %y "$f")

+ 

+   read orig_shebang < "$f" || :

+   shebang=$(trim $(echo "$orig_shebang" | grep -Po "#!\K.*" || echo))

+   if [ -z "$shebang" ]; then

+     echo >&2 "*** WARNING: $f is executable but has empty or no shebang, removing executable bit"

+     chmod -x "$f"

+     touch -d "$ts" "$f"

+     continue

+   elif [ "${shebang%${shebang#?}}" != "/" ]; then

+     echo >&2 "*** ERROR: $f has shebang which doesn't start with '/' ($shebang)"

+     fail=1

+     continue

+   fi

+ 

+   if ! { echo "$shebang" | grep -q -P "^/(?:usr/)?(?:bin|sbin)/"; }; then

+     continue

+   fi

+ 

+   # Replace "special" env shebang:

+   # /whatsoever/env foo → /whatsoever/foo

+   shebang=$(echo "$shebang" | sed -r -e 's@^(.+/)env (.+)$@\1\2@')

+ 

+   # Replace ambiguous python with python2

+   py_shebang=$(echo "$shebang" | sed -r -e 's@/usr/bin/python(\s|$)@/usr/bin/python2\1@')

+ 

+   if [ "$shebang" != "$py_shebang" ]; then

+     sed -i -e "1c #!$py_shebang" "$f"

+     echo >&2 "*** WARNING: mangling shebang in $f from $orig_shebang to #!$py_shebang. This will become an ERROR, fix it manually!"

+   elif [ "#!$shebang" != "$orig_shebang" ]; then

+     sed -i -e "1c #!$shebang" "$f"

+     echo "mangling shebang in $f from $orig_shebang to #!$shebang"

+   fi

+ 

+   touch -d "$ts" "$f"

+ done

+ 

+ exit $fail

file modified
+2

@@ -107,6 +107,7 @@ 

  %__brp_strip_static_archive /usr/lib/rpm/brp-strip-static-archive %{__strip}

  %__brp_python_bytecompile /usr/lib/rpm/brp-python-bytecompile %{__python} %{?_python_bytecompile_errors_terminate_build}

  %__brp_python_hardlink /usr/lib/rpm/brp-python-hardlink

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

  

  %__os_install_post    \

      %{?__brp_ldconfig} \

@@ -118,6 +119,7 @@ 

      %{?__brp_strip_static_archive} \

      %{?py_auto_byte_compile:%{?__brp_python_bytecompile}} \

      %{?__brp_python_hardlink} \

+     %{?__brp_mangle_shebangs} \

  %{nil}

  

  %__spec_install_post\

file modified
+19 -1

@@ -6,7 +6,7 @@ 

  

  Summary: Red Hat specific rpm configuration files

  Name: redhat-rpm-config

- Version: 89

+ Version: 90

  Release: 1%{?dist}

  # No version specified.

  License: GPL+

@@ -42,6 +42,12 @@ 

  Source153: macros.forge

  Source154: macros.ldconfig

  

+ # Build policy scripts

+ # this comes from https://github.com/rpm-software-management/rpm/pull/344

+ # added a python -> python2 conversion for fedora with warning

+ # and an echo when the mangling happens

+ Source201: brp-mangle-shebangs

+ 

  # Dependency generator scripts (deprecated)

  Source300: find-provides

  Source301: find-provides.ksyms

@@ -92,6 +98,13 @@ 

  Requires: zip

  Requires: (annobin if gcc)

  

+ # for brp-mangle-shebangs

+ Requires: %{_bindir}/find

+ Requires: %{_bindir}/grep

+ Requires: %{_bindir}/xargs

+ Requires: %{_bindir}/file

+ Requires: %{_bindir}/sed

+ 

  # -fstack-clash-protection and CET requires GCC 8.

  Conflicts: gcc < 8.0

  

@@ -122,6 +135,7 @@ 

  install -p -m 444 -t %{buildroot}%{rrcdir} redhat-annobin-*

  install -p -m 755 -t %{buildroot}%{rrcdir} config.*

  install -p -m 755 -t %{buildroot}%{rrcdir} dist.sh rpmsort symset-table kmodtool

+ install -p -m 755 -t %{buildroot}%{rrcdir} brp-*

  

  install -p -m 755 -t %{buildroot}%{rrcdir} find-*

  mkdir -p %{buildroot}%{rrcdir}/find-provides.d

@@ -140,6 +154,7 @@ 

  %dir %{rrcdir}

  %{rrcdir}/macros

  %{rrcdir}/rpmrc

+ %{rrcdir}/brp-*

  %{rrcdir}/dist.sh

  %{rrcdir}/redhat-hardened-*

  %{rrcdir}/redhat-annobin-*

@@ -168,6 +183,9 @@ 

  %{_rpmconfigdir}/macros.d/macros.kmp

  

  %changelog

+ * Mon Jan 29 2018 Miro Hrončok <mhroncok@redhat.com> - 90-1

+ - Add brp-mangle-shebangs

+ 

  * Mon Jan 29 2018 Igor Gnatenko <ignatenkobrain@fedoraproject.org> - 89-1

  - Add macros.ldconfig

  

no initial comment

I've created a copr repo where I'd like to test to build various packages and see the results.

https://copr.fedorainfracloud.org/coprs/churchyard/brp-mangle-shebangs/

libtaskotorn-core successfully mangled from /usr/bin/python to /usr/bin/python2

rubygem-builder successfully mangled from /usr/bin/env ruby to /usr/bin/ruby

python-kid successfully mangled from /usr/bin/env python to /usr/bin/python2

I'm not aware of Fedora accepting universally changing those #!/usr/bin/env shebangs. Feel free to prove me wrong on that account, but unless it has been accepted you're better off not combining that mess with your own agenda.

@pmatilai at this point of time, guidelines prohibit usage of /usr/bin/env. when/if this will change, script can be adjusted.

I would be happy if we only convert the python ones. However, env shebangs are against the guidelines anyway. Should I open and FPC ticket for this?

As noted in the PC issue too, I went ahead and converted the existing brp-stuff to follow a common theme so you don't need to worry about that, just update this PR to follow that scheme:
https://src.fedoraproject.org/rpms/redhat-rpm-config/c/c4646d791dea6b976fd5a04e2562c7b21b9389e0?branch=master

rebased onto a1a6370715132da017dc4dc4de307dd7c3d014b9

2 years ago

Rebased to support that.

rebased onto 602c6e9d4f8fa9ecf656d2747da47ee801114ac1

2 years ago

I was thinking, maybe we should add a WARNING/INFO when we mangle the shebang. So it appears in the log.

brp-mangle-shebangs should be more careful about what changes it makes.

If a file has a shebang like #!/home/bob/stuff/usr/bin/python, then is it really correct to change that to #!/home/bob/stuff/usr/bin/python2?

Suppose there's a program called "Excellent Neutrino Validator", or "env" for short, and a file named "/usr/libexec/neutrinoval/antired_tau" contains the shebang #!/usr/libexec/neutrinoval/env --oscillation. That would be mangled into #!/usr/libexec/neutrinoval/--oscillation, which is definitely wrong.

So.... point taken as in this shouldn't be merged until you make those changes?

BTW I've changed the title of this PR to have WIP when adding the comment, however name changes are not visible in the comments stream.

rebased onto aaaf141dffb8dfbce84d9c1dfa3aa85ff4036b4c

2 years ago

Then I'm confused; is this going to come into Fedora via some upstream RPM change? Is this PR still relevant?

Yes this PR is relevant. This is going into Fedora via this PR. It might or might not end up in upstream rpm as well. Note that this Pr is a bit different tough. I'm testing the recent changes now.

rebased onto ec14d415ad9ef970065a3b642935747fee99d767

2 years ago

libtaskotorn-core successfully mangled from /usr/bin/python to /usr/bin/python2

rubygem-builder successfully mangled from /usr/bin/env ruby to /usr/bin/ruby

python-kid successfully mangled from /usr/bin/env python to /usr/bin/python2

excellent-neutrino-validator preserved both /home/bob/stuff/usr/bin/python and /usr/libexec/neutrinoval/env

python-kid with %undefine __brp_mangle_shebangs successfully kept /usr/bin/env python

Warnings (added so the changes are easier to spot and harder to overlook) show up:

+ /usr/lib/rpm/redhat/brp-mangle-shebangs
*** 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
*** WARNING: mangling shebang in ./usr/share/gems/gems/builder-3.2.3/test/test_namecollision.rb from #!/usr/bin/env ruby to #!/usr/bin/ruby
*** WARNING: mangling shebang in ./usr/share/gems/gems/builder-3.2.3/test/test_method_caching.rb from #!/usr/bin/env ruby to #!/usr/bin/ruby
*** WARNING: mangling shebang in ./usr/share/gems/gems/builder-3.2.3/test/test_markupbuilder.rb from #!/usr/bin/env ruby to #!/usr/bin/ruby
*** WARNING: mangling shebang in ./usr/share/gems/gems/builder-3.2.3/test/test_eventbuilder.rb from #!/usr/bin/env ruby to #!/usr/bin/ruby
*** WARNING: mangling shebang in ./usr/share/gems/gems/builder-3.2.3/test/test_blankslate.rb from #!/usr/bin/env ruby to #!/usr/bin/ruby
*** WARNING: mangling shebang in ./usr/share/gems/gems/builder-3.2.3/test/preload.rb from #!/usr/bin/env ruby to #!/usr/bin/ruby
*** WARNING: mangling shebang in ./usr/share/gems/gems/builder-3.2.3/test/performance.rb from #!/usr/bin/env ruby to #!/usr/bin/ruby
*** WARNING: mangling shebang in ./usr/share/gems/gems/builder-3.2.3/lib/builder/xmlmarkup.rb from #!/usr/bin/env ruby to #!/usr/bin/ruby
*** WARNING: mangling shebang in ./usr/share/gems/gems/builder-3.2.3/lib/builder/xmlevents.rb from #!/usr/bin/env ruby to #!/usr/bin/ruby
*** WARNING: mangling shebang in ./usr/share/gems/gems/builder-3.2.3/lib/builder/xmlbase.rb from #!/usr/bin/env ruby to #!/usr/bin/ruby
*** WARNING: mangling shebang in ./usr/share/gems/gems/builder-3.2.3/lib/builder/xchar.rb from #!/usr/bin/env ruby to #!/usr/bin/ruby
*** WARNING: mangling shebang in ./usr/share/gems/gems/builder-3.2.3/lib/builder/blankslate.rb from #!/usr/bin/env ruby to #!/usr/bin/ruby
*** WARNING: mangling shebang in ./usr/share/gems/gems/builder-3.2.3/lib/builder.rb from #!/usr/bin/env ruby to #!/usr/bin/ruby
*** WARNING: mangling shebang in ./usr/share/gems/gems/builder-3.2.3/lib/blankslate.rb from #!/usr/bin/env ruby to #!/usr/bin/ruby

https://copr.fedorainfracloud.org/coprs/churchyard/brp-mangle-shebangs/builds/

rebased onto 7c8a1a06196c6cead5d62b5e41aa4eabbdd8798a

2 years ago

Changed as per https://pagure.io/packaging-committee/issue/738#comment-490314

Warning changed to info in general case:

+ /usr/lib/rpm/redhat/brp-mangle-shebangs
mangling shebang in ./usr/share/gems/gems/builder-3.2.3/test/test_xchar.rb from #!/usr/bin/env ruby to #!/usr/bin/ruby
mangling shebang in ./usr/share/gems/gems/builder-3.2.3/test/test_namecollision.rb from #!/usr/bin/env ruby to #!/usr/bin/ruby
...

But warning with future error for python:

+ /usr/lib/rpm/redhat/brp-mangle-shebangs
*** WARNING: mangling shebang in ./usr/lib/python2.7/site-packages/kid/run.py from #!/usr/bin/env python to #!/usr/bin/python2. This will become an ERROR, fix it manually!
*** WARNING: mangling shebang in ./usr/lib/python2.7/site-packages/kid/compile.py from #!/usr/bin/env python to #!/usr/bin/python2. This will become an ERROR, fix it manually!

Mangles in the same way as before.

rebased onto ecda980c75e45fab3ffc0ee4270e336d4660446d

2 years ago

rebased onto 1c6488b56a60a978b7c43cb15a4931684b300d55

2 years ago

rebased onto 257a3a9

2 years ago

I'm going to merge this in a few hours unless there are objections.

Should we depend on findutils and file now?

file is utterly unreliable. You need to use something else.

@fweimer please elaborate this. unreliable in what way?

@praiskup all those sem to be in the buildroot, but I can add those explicitly

Added in extra commit, @tibbs feel free to merge with or without

1 new commit added

  • Explicitly require stuff for brp-mangle-shebangs
2 years ago

@churchyard The file command gives random results depending on what's in the file. I'm sure there are plenty of shell scripts which are recognized as something else entirely because the heuristics go wrong.

If file triggers improperly, as far as I can tell the only thing which would happen is that the script fails to fix up a shebang line. For any damage to occur, you would have to have a file which is executable, detected as some kind of text, which starts with a shebang calling env or python but which is somehow not a script that needs fixing. I do not believe this is possible.

It may turn out that this does miss more instances of #!env or unversioned python than we would like, and those would indeed need fixing. But I don't think it needs to catch 100% of them as long as it doesn't change anything which it's not supposed to change.

Pull-Request has been merged by tibbs

2 years ago