Skip to content
This repository has been archived by the owner on May 23, 2024. It is now read-only.

Cleanup for Python 3 compat #48

Merged
merged 5 commits into from
Jan 20, 2020
Merged

Cleanup for Python 3 compat #48

merged 5 commits into from
Jan 20, 2020

Conversation

dzuelke
Copy link
Contributor

@dzuelke dzuelke commented Jan 15, 2020

This PR contains three changes:

  1. remove the whole subprocess output piping business, since that had issues with text vs binary on Python 3, and is unnecessary (output is now auto-sent to the parent stdout) - as a bonus, there no longer is any buffering, which means e.g. curl status output is now "live", and not just printed when there's finally a newline (like with the line buffering before)
  2. move all informational output from Bob itself to stderr, so that one can e.g. bob build foobar | tee foobar.build.log, with stdout containing only the output from the formula
  3. fix Ctrl+C handling (programs reacting to a signal must kill themselves using that same signal, not exit with a status code) so that e.g. a bash loop of bob invocations can be aborted

More details in each commit message.

Fixes #34.

This brings Python 3 compatibility

With this change, all output from the formula ends up on stdout, without any buffering, meaning e.g. progress bars (from curl for instance) also finally show up in real time, without line buffering like before
all output from the formula build is on stdout already

this allows easy separating of the two into e.g. raw compile logs without bob's messages
When receiving a signal, a process must kill itself using the same signal
sys.exit()ing 0, 1, 130, whatever will not signal to the calling program that we terminated in response to the signal

Best example: `for f in a b c; do bob deploy $f; done`, hitting Ctrl+C should interrupt Bob and stop the bash loop, but does not with `sys.exit()`:

    # for x in php-7.3.13 php-7.3.13 php-7.3.13; do bob build $x; done
    Fetching dependencies... found 1:
      - libraries/libc-client-2007f

    Building formula php-7.3.13 in /tmp/bob-35s7cr5z:

    -----> Building php-7.3.13...
      % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                     Dload  Upload   Total   Spent    Left  Speed
    100 18.7M  100 18.7M    0     0  7435k      0  0:00:02  0:00:02 --:--:-- 7434k
    ^Cool.
    Fetching dependencies... found 1:
      - libraries/libc-client-2007f

    Building formula php-7.3.13 in /tmp/bob-vmuko2ra:

    -----> Building php-7.3.13...
      % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                     Dload  Upload   Total   Spent    Left  Speed
     20 18.7M   20 3887k    0     0  7479k      0  0:00:02 --:--:--  0:00:02 7476k^Cool.
    Fetching dependencies... found 1:
      - libraries/libc-client-2007f

    Building formula php-7.3.13 in /tmp/bob-p2t8as81:

    -----> Building php-7.3.13...
      % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                     Dload  Upload   Total   Spent    Left  Speed
      4 18.7M    4  943k    0     0  3599k      0  0:00:05 --:--:--  0:00:05 3588k^Cool.

We want instead to have the loop end upon the first Ctrl+C.

That's only possible if Bash knows that we exited in response to Ctrl+C (=SIGINT), then it'll also terminate the loop
Bash will report the exit status as 128+$signal, so 130 for SIGINT, but sys.exit(130) does not to the same thing - the value of 130 is simply bash's representation
Killing ourselves with the signal number that we are aborting in response to does all this correctly, and bash will see the right WIFSIGNALED() status of our program, not WIFEXITED()
@dzuelke
Copy link
Contributor Author

dzuelke commented Jan 15, 2020

Fixes #34

when hitting Ctrl+C in a 'docker run bob …', the main script itself actually can't be terminated by Ctrl+C, because it's PID 1

in that case, rather than just shutting down silently (by terminating itself using a SIGINT), we will see the subprocess exit (because Ctrl+C is sent to the whole process group)

When this happens, the return code of the process will be negative, to indicate that it didn't exit with that code, but instead got terminated by a signal of that (absolute) number
Copy link
Contributor

@edmorley edmorley left a comment

Choose a reason for hiding this comment

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

Looks great, thank you :-)

@dzuelke dzuelke merged commit 7c172f3 into master Jan 20, 2020
@edmorley edmorley deleted the python3 branch April 23, 2020 12:06
edmorley added a commit to heroku/heroku-buildpack-python that referenced this pull request Apr 24, 2020
This adds support for generating binaries for the upcoming Heroku-20
stack, and testing them in CI.

The Heroku-20 Dockerfile is based on that for Heroku-18, but with:
* the unused `python-{pip-whl,setuptools,wheel}` dependencies removed
* `python-pip` replaced with `python3-pip` since the former does not
  exist on Ubuntu 20.04.

The switch to Python 3 pip in the builder image only affects bob-builder,
since everything else (such as pip-diff) runs against the Python binaries
downloaded as part of the compile.

bob-builder has been updated to v0.0.18 to pick up the Python 3 fixes in:
heroku-python/bob-builder#48

Before this is merged, the binaries will need to be built/uploaded using
the steps here:
https://github.com/heroku/heroku-buildpack-python/blob/master/builds/README.md

A decision will need to be made about whether to backfill older Python
versions, however for now we should at least provide the latest point
release of each supported Python branch:
https://devguide.python.org/#status-of-python-branches

Which are:
* `3.5.9`
* `3.6.10`
* `3.7.7`
* `3.8.2`

We'll also want to upload at least one version of PyPy, however the
current script is for the out of date `pypy3.6-7.2.0`, so we'll want to
generate newer binaries for all stacks, which is out of scope for now.

Refs W-7485877.
edmorley added a commit to heroku/heroku-buildpack-python that referenced this pull request Apr 27, 2020
This adds support for generating binaries for the upcoming Heroku-20
stack, and testing them in CI.

The Heroku-20 Dockerfile is based on that for Heroku-18, but with:
* the unused `python-{pip-whl,setuptools,wheel}` dependencies removed
* `python-pip` replaced with `python3-pip` since the former does not
  exist on Ubuntu 20.04.

The switch to Python 3 pip in the builder image only affects bob-builder,
since everything else (such as pip-diff) runs against the Python binaries
downloaded as part of the compile.

bob-builder has been updated to v0.0.18 to pick up the Python 3 fixes in:
heroku-python/bob-builder#48

Before this is merged, the binaries will need to be built/uploaded using
the steps here:
https://github.com/heroku/heroku-buildpack-python/blob/master/builds/README.md

A decision will need to be made about whether to backfill older Python
versions, however for now we should at least provide the latest point
release of each supported Python branch:
https://devguide.python.org/#status-of-python-branches

Which are:
* `3.5.9`
* `3.6.10`
* `3.7.7`
* `3.8.2`

We'll also want to upload at least one version of PyPy, however the
current script is for the out of date `pypy3.6-7.2.0`, so we'll want to
generate newer binaries for all stacks, which is out of scope for now.

Refs W-7485877.
edmorley added a commit to heroku/heroku-buildpack-python that referenced this pull request May 21, 2020
This adds support for generating binaries for the upcoming Heroku-20
stack, and testing them in CI.

The Heroku-20 Dockerfile is based on that for Heroku-18, but with:
* the unused `python-{pip-whl,setuptools,wheel}` dependencies removed
* `python-pip` replaced with `python3-pip` since the former does not
  exist on Ubuntu 20.04.

The switch to Python 3 pip in the builder image only affects bob-builder,
since everything else (such as pip-diff) runs against the Python binaries
downloaded as part of the compile.

bob-builder has been updated to v0.0.18 to pick up the Python 3 fixes in:
heroku-python/bob-builder#48

Before this is merged, the binaries will need to be built/uploaded using
the steps here:
https://github.com/heroku/heroku-buildpack-python/blob/master/builds/README.md

The initial versions made available will be those in:
https://devcenter.heroku.com/articles/python-support#supported-runtimes

...minus CPython 2.7, since it's EOL.

Which are:
* `3.6.10`
* `3.7.7`
* `3.8.3`

And also the latest versions of PyPy:
* `pypy2.7-7.3.1` - Since unlike CPython the 2.7 branch is still supported:
  https://doc.pypy.org/en/latest/faq.html#how-long-will-pypy-support-python2
* `pypy3.6-7.3.1`

Refs W-7485877.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Add support for Python 3
2 participants