Skip to content
This repository was archived by the owner on Mar 3, 2023. It is now read-only.

Disable Fedora's shebang mangling script when building the .rpm - #21964

Merged
sadick254 merged 1 commit into
atom:masterfrom
DeeDeeG:disable-Fedora-shebang-mangling-script
Feb 15, 2021
Merged

sadick254 merged 1 commit into
atom:masterfrom
DeeDeeG:disable-Fedora-shebang-mangling-script

Conversation

@DeeDeeG

@DeeDeeG DeeDeeG commented Feb 14, 2021

Copy link
Copy Markdown
Contributor
Requirements for Contributing a Bug Fix (from template, click to expand):

Identify the Bug

Fixes: #21937

Description of the Change

Disable Fedora's shebang mangling script. (Which was being run when building the .rpm package on Fedora.)

A description of the script (click to expand)

Fedora's script tries to coerce all shebangs to point to exact, system-provided binaries.

For example: #!/usr/bin/env sh becomes #!/usr/bin/sh.

Starting with Fedora 30, the script errors out when it encounters ambiguous, versionless python in shebangs. (python2 and python3 are allowed.) For more about this error, see: #21937

For example, this shebang line causes an error: #!/usr/bin/env python.
(There are a bunch of those in node-gyp.)

I'm proposing to disable this script, for two reasons:

  1. Fedora users should be able to build the Atom .rpm package without errors.
  2. Consistent shebangs in the Atom .rpm package, regardless of building on Debian/Ubuntu or Fedora.

See:

Possible Drawbacks

(This is mostly theoretical, as the .rpm package is built on Ubuntu in CI, where the shebang mangling script has never been available in the first place and has never been run. This PR has no effect on the official .rpm builds, but rather would make manual builds on Fedora consistent with the ones from our CI.)

Potential drawback for users who have been building on Fedora for some time already: Lets more #!/usr/bin/env be used in shebang lines, so in theory if someone has a wonky sh or python on their PATH it could cause bugs. However, it is ultimately the user's responsibility to set their environment in a usable way. (Most people don't put broken sh or python binaries on their PATH.) We don't change those shebangs in the .deb package, or the .rpm package built in CI, so I'd like to be consistent.

Alternate Designs

  • Run another script first, which changes "ambiguous" python in shebangs to python3. Lets the shebang mangling script run without errors. (DeeDeeG@cf3de25 / Still a draft: Try to fix #21937 #21960)
  • Manually mark certain shebangs as allowed, such as #!/usr/bin/env python from node-gyp.
  • Update to the node-gyp v8 release some time in the future, where shebangs are planned to have python3 instead of python. (Not possible until that is released!)

I chose to disable the shebang mangling script entirely, for consistent shebangs regardless of building Atom on Fedora or Debian/Ubuntu.

Verification Process

Manually tested that the Atom .rpm package builds and works correctly on Fedora with this change. apm can still install packages with this change. The Atom .rpm package still builds on Ubuntu with this change.

Release Notes

N/A

Fedora's script tries to coerce all shebangs
to point to exact, system-provided binaries.

For example: `#!/usr/bin/env sh` becomes `#!/usr/bin/sh`.

Starting with Fedora 30, the script errors out when it encounters
ambiguous, versionless `python` in shebangs.

(`python2` and `python3` are allowed.)

For example, this shebang causes an error: `#!/usr/bin/env python`.

---

Disable this script for two reasons:

1) Fedora users should be able to build Atom without errors.
2) Consistent shebangs across builds of Atom on Ubuntu and Fedora.

See: atom#21937 for more details.

Fixes: atom#21937

@sadick254 sadick254 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @DeeDeeG for this contribution. 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to create Atom RPM package on Fedora 33

2 participants