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

Replace the runtime-fixer Python script with Bash #1568

Merged
merged 1 commit into from
Apr 18, 2024
Merged

Conversation

edmorley
Copy link
Member

@edmorley edmorley commented Apr 17, 2024

The legacy runtime-fixer script uses Python which is problematic since the script has to run prior to the buildpack actually installing Python.

As such:

  • For clean installs of new apps (where there is no cached Python install), the script ends up relying upon system Python.
  • For cached builds, the script ends up using the Python install restored from the cache. If an app has changed stack, this means a Python install intended for one Ubuntu version ends up being used on another during the next build. In the case of stack downgrades this results in a glibc warning (it's only a warning rather than an error, since the script is allowed to fail, because of this bug).

There was unfortunately no explanation given as to why Python was chosen when the runtime-fixer script was first added:
df52fd4

(It probably didn't help that there were no stack change tests back then.)

Regardless, the only purpose of this script is to trim whitespace from the contents of runtime.txt, which we can easily do using Bash built-ins instead to avoid this issue.

The whitespace stripping already has a test, which still passes.

As an added bonus, removing the runtime-fixer script means we won't have to make it compatible with the python -> python3 switch for Heroku-24.

GUS-W-8059923.

@edmorley edmorley added the bug label Apr 17, 2024
@edmorley edmorley self-assigned this Apr 17, 2024
@edmorley edmorley marked this pull request as ready for review April 17, 2024 16:23
@edmorley edmorley requested a review from a team as a code owner April 17, 2024 16:23
@edmorley edmorley enabled auto-merge (squash) April 17, 2024 16:25
The legacy `runtime-fixer` script uses Python which is problematic since
the script has to run prior to the buildpack actually installing Python.

As such:
- For clean installs of new apps (where there is no cached Python
  install), the script ends up relying upon system Python.
- For cached builds, the script ends up using the Python install
  restored from the cache. If an app has changed stack, this means a
  Python install intended for one Ubuntu version ends up being used on
  another during the next build. In the case of stack downgrades this
  results in a glibc warning (it's only a warning rather than an error,
  since the script is allowed to fail, because of this bug).

There was unfortunately no explanation given as to why Python was chosen
when the `runtime-fixer` script was first added:
df52fd4

(It probably didn't help that there were no stack change tests back then.)

Regardless, the only purpose of this script is to trim whitespace from
the contents of `runtime.txt`, which we can easily do using Bash
built-ins instead to avoid this issue.

The whitespace stripping already has a test, which still passes:
https://github.com/heroku/heroku-buildpack-python/blob/84684a01dc5b2ec73e6debf3cfeda008f15db1a4/spec/hatchet/python_version_spec.rb#L236-L240

As an added bonus, removing the `runtime-fixer` script means we won't
have to make it compatible with the `python` -> `python3` switch for
Heroku-24.

GUS-W-8059923.
@edmorley edmorley merged commit bad3e01 into main Apr 18, 2024
5 checks passed
@edmorley edmorley deleted the rm-runtime-fixer branch April 18, 2024 13:46
@heroku-linguist heroku-linguist bot mentioned this pull request Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants