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

make_deb.py does not seem to be Python3-compatible #8443

Closed
c4urself opened this issue May 22, 2019 · 7 comments
Closed

make_deb.py does not seem to be Python3-compatible #8443

c4urself opened this issue May 22, 2019 · 7 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-Server Issues for serverside rules included with Bazel type: feature request
Milestone

Comments

@c4urself
Copy link
Contributor

Description of the problem / feature request:

sys.argv is assumed to be bytes but is actually unicode in Python3

Feature requests: what underlying problem are you trying to solve with this feature?

using make_deb.py with Python3 like I did before this commit: (70e3c33#diff-687105bbda36ddcad584169fe214e7e1)

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

virtualenv -p $(which python3) test_py3_venv
bazel build --python_path=test_py3_venv/bin/python :some_deb

What operating system are you running Bazel on?

MacOS 10.13

What's the output of bazel info release?

release 0.25.2

If bazel info release returns "development version" or "(@non-git)", tell us how you built Bazel.

N/A

What's the output of git remote get-url origin ; git rev-parse master ; git rev-parse HEAD ?

N/A

Have you found anything relevant by searching the web?

No

Any other information, logs, or outputs that you want to share?

Traceback (most recent call last):
  File "/private/var/tmp/_bazel_c4urself/ffc93e913d26536f0935179fcb6170b5/sandbox/darwin-sandbox/2/execroot/__main__/bazel-out/host/bin/external/bazel_tools/tools/build_defs/pkg/make_deb.runfiles/bazel_tools/tools/build_defs/pkg/make_deb.py", line 355, in <module>
    main(FLAGS(sys.argv))
  File "/private/var/tmp/_bazel_c4urself/ffc93e913d26536f0935179fcb6170b5/sandbox/darwin-sandbox/2/execroot/__main__/bazel-out/host/bin/external/bazel_tools/tools/build_defs/pkg/make_deb.runfiles/bazel_tools/tools/build_defs/pkg/make_deb.py", line 321, in main
    preinst=GetFlagValue(FLAGS.preinst, False),
  File "/private/var/tmp/_bazel_c4urself/ffc93e913d26536f0935179fcb6170b5/sandbox/darwin-sandbox/2/execroot/__main__/bazel-out/host/bin/external/bazel_tools/tools/build_defs/pkg/make_deb.runfiles/bazel_tools/tools/build_defs/pkg/make_deb.py", line 301, in GetFlagValue
    flagvalue = flagvalue.decode('utf-8')
AttributeError: 'str' object has no attribute 'decode'
@ishikhman ishikhman added team-Rules-Server Issues for serverside rules included with Bazel type: bug untriaged labels May 23, 2019
@jwnimmer-tri
Copy link
Contributor

jwnimmer-tri commented May 24, 2019

This is hitting us as well. I think this was fixed in #6818 and then regressed by #7370. Note that there are many python3-broken lines the new revision, not just the one posted above.

I worked around it by editing my pkg_deb rules to say make_deb = "//third_party/bazel:make_deb", where that label is my vendored copy of the file from Bazel 0.24.1.

@praseodym
Copy link

praseodym commented May 26, 2019

Same problem with release 0.25.3, even with the BAZEL_PYTHON environment variable set to /usr/bin/python2.

Edit: apparently BAZEL_PYTHON does nothing, adding build --python_path=/usr/bin/python2 to a .bazelrc file does work around this issue.

@c4urself
Copy link
Contributor Author

Our entire project is Python3 so using --python_path=/usr/bin/python2 is never going to work in our case. Why do we even try to achieve Python2 compatibility again? (https://pythonclock.org/)

philwo added a commit to bazelbuild/continuous-integration that referenced this issue Jun 3, 2019
philwo added a commit to bazelbuild/continuous-integration that referenced this issue Jun 4, 2019
* Use --host_force_python=PY2 when building the Linux release

This is a workaround for bazelbuild/bazel#8443

* Add comment explaining the flag.
philwo added a commit to bazelbuild/continuous-integration that referenced this issue Jun 5, 2019
* Use --host_force_python=PY2 when building the Linux release

This is a workaround for bazelbuild/bazel#8443

* Add comment explaining the flag.
@hlopko hlopko added type: feature request P2 We'll consider working on this in future. (Assignee optional) and removed untriaged type: bug labels Jun 7, 2019
@hlopko
Copy link
Member

hlopko commented Jun 7, 2019

@brandjon what do you recommend? :)

Also ccing @aiuto as he's been working on packaging rules recently.

@brandjon
Copy link
Member

brandjon commented Jun 10, 2019

If this tool is meant to run under either Python 2 or Python 3, there ought to be py_test targets for both values for python_version.

If this tool is used in a non-host config, it'll only ever be analyzed for whichever version is declared in its own python_version attribute, or PY3 if none is specified. If the tool is used in the host config, it'll be analyzed as whatever version --host_force_python says (default PY3).

With Python toolchains not enabled (Bazel < 0.27), it may still run as whatever version the system has installed for the python command. With Python toolchains enabled (Bazel >= 0.27), it'll enforce that it runs under the same version as what was analyzed as per above.

If the source code truly is compatible with both PY2 and PY3, and we add tests to enforce this, and we don't include any analysis-time logic that is sensitive to the Python version (select()ing on the version, or srcs_version constraints), then it should be fine to let Bazel analyze whatever version it wants, and not care what happens at runtime or what --host_force_python is (at least as far as this particular target is concerned). Mac users may have to opt into the non-strict toolchain if they don't have Python 3 installed though (see 65b20f7 and #8547).

@aiuto
Copy link
Contributor

aiuto commented Jun 10, 2019

Let's take this over to the new place for these rules.
https://github.com/bazelbuild/rules_pkg

@c4urself Would you like to place the issue (or more) there, or are you happy with me copying them over.

But, to answer one question here.

Why do we even try to achieve Python2 compatibility again? (https://pythonclock.org/)

For the Apple users who have to struggle to use Python3.
That said, there is hope. With the rules in Bazel, we are going to opt for the backwards compatibility. With the rules in rules_pkg, there is a lot more option for the maintainers of the rules to decide that if you want to use them they are Python 3 only. Or, to put it more explicitly, I would approve a pure python3 PR in that repo, but I might hesitate about it here.

gvisor-bot pushed a commit to google/gvisor that referenced this issue Jun 17, 2019
$ bazel build runsc:runsc-debian
  File ".../bazel_tools/tools/build_defs/pkg/make_deb.py", line 311,
  in GetFlagValue:
    flagvalue = flagvalue.decode('utf-8')
AttributeError: 'str' object has no attribute 'decode'

make_deb.py is incompatible with Python3.
bazelbuild/bazel#8443

PiperOrigin-RevId: 253640180
gvisor-bot pushed a commit to google/gvisor that referenced this issue Jun 18, 2019
$ bazel build runsc:runsc-debian
  File ".../bazel_tools/tools/build_defs/pkg/make_deb.py", line 311,
  in GetFlagValue:
    flagvalue = flagvalue.decode('utf-8')
AttributeError: 'str' object has no attribute 'decode'

make_deb.py is incompatible with Python3.
bazelbuild/bazel#8443

PiperOrigin-RevId: 253691923
joeleba pushed a commit to joeleba/continuous-integration that referenced this issue Jun 19, 2019
…ld#696)

* Use --host_force_python=PY2 when building the Linux release

This is a workaround for bazelbuild/bazel#8443

* Add comment explaining the flag.
@aiuto
Copy link
Contributor

aiuto commented Jul 9, 2019

@aiuto aiuto closed this as completed Jul 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-Server Issues for serverside rules included with Bazel type: feature request
Projects
None yet
Development

No branches or pull requests

7 participants