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

Reintroduce support for Python 2.6 to python_stub_template #11269

Closed
wants to merge 1 commit into from
Closed

Reintroduce support for Python 2.6 to python_stub_template #11269

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 30, 2020

Commit d5012a7 introduced a Python
2.7 dependency into python_stub_template.txt. Unfortunately this stub
is non-hermetic, so even if the user has configured Python 2.7 and 3
py_runtimes, their builds may fail if the system-provided Python is ancient
(ex: CentOS 6.6 provides Python 2.6.6).

Accommodate ancient Python by reworking the path deduplication in terms
of a set and a generator instead of collections.OrderedDict.

Workaround for #11265.

Testing Done:

  • bazelisk test //src/test/shell/integration:python_stub_test
  • In a CentOS 6.6 env:
$ /usr/bin/env python -V
Python 2.6.6
$ cat >test.py <<EOF
import sys
print(sys.executable)
EOF
$ cat >BUILD <<EOF
py_binary(name = "test", srcs = ["test.py"])
EOF
$ bazel run :test
/path/to/my/hermetic/python3

Commit d5012a7 introduced a Python
2.7 dependency into python_stub_template.txt.  Unfortunately this stub
is non-hermetic, so even if the user has configured Python 2.7 and 3
`py_runtime`s, their builds may fail if the system-provided Python is ancient
(ex: CentOS 6.6 provides Python 2.6.6).

Accommodate ancient Python by reworking the path deduplication in terms
of a `set` and a generator instead of `collections.OrderedDict`.

Workaround for #11265.

Testing Done:
- `bazelisk test //src/test/shell/integration:python_stub_test`
- In a CentOS 6.6 env:
```console
$ /usr/bin/env python -V
Python 2.6.6
$ cat >test.py <<EOF
import sys
print(sys.executable)
EOF
$ cat >BUILD <<EOF
py_binary(name = "test", srcs = ["test.py"])
EOF
$ bazel run :test
/path/to/my/hermetic/python3
```
@aiuto aiuto requested a review from brandjon May 1, 2020 13:33
Copy link
Contributor

@aiuto aiuto left a comment

Choose a reason for hiding this comment

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

LGTM

@aiuto
Copy link
Contributor

aiuto commented May 1, 2020

@brandjon Since it is already the weekend for Jin, I'll cover here. If you do the approve, I've got the patch CL lined up.

@brandjon
Copy link
Member

brandjon commented May 1, 2020

I'm a little reluctant to do this, because it sets a precedent that we want to support Python 2.6 (released about 12 years ago) in the stub script. If that's going to be a requirement then we should arguably include a Python 2.6 interpreter in our test environment and run at least one py_binary in it.

It also may be unnecessary if we instead add the ability to select an alternate stub script shebang in the Python toolchain. See #8685. But I appreciate that in the absence of a shebang-selecting feature in the toolchain, this PR is useful and is anyway harmless.

How about we accept this patch and just add a comment reminding to keep compatibility with Python 2.6 until #8685 is implemented.

@bazel-io bazel-io closed this in d95bfbd May 1, 2020
@brandjon
Copy link
Member

brandjon commented May 1, 2020

(Added comment clarifying this stance in the internal merge.)

@ghost
Copy link
Author

ghost commented May 1, 2020

Great, thanks!

@ghost ghost deleted the topic/beasleyr/11265-python-stub-template branch May 1, 2020 20:23
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.

4 participants