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

Inconsistent import behaviour between py_binary and par_binary #97

Open
thundergolfer opened this issue Mar 13, 2019 · 2 comments
Open

Comments

@thundergolfer
Copy link

thundergolfer commented Mar 13, 2019

Description

I encountered some unexpected incompatibility between py_binary and par_binary, that belies this line "par_binary() is a drop-in replacement for py_binary() in your BUILD file".

How to Reproduce

Subpar version:

# Import the rules to build Python subpar packages.
git_repository(
    name = "subpar",
    remote = "https://github.com/google/subpar.git",
    tag = "1.3.0",
)

Python Rules Version:

# TODO: switch back to upstream once https://github.com/bazelbuild/rules_python/pull/82
# and https://github.com/bazelbuild/rules_python/pull/136 are merged.
http_archive(
    name = "io_bazel_rules_python",
    sha256 = "9bd76f014ae4239f0d3ed0c29900a7e3d7307e5fa00506c287dc658cdfe3840a",
    strip_prefix = "rules_python-be39b7906da9c924b09ca78ce3fa152a10f782ae",
    url = "https://github.com/uri-canva/rules_python/archive/be39b7906da9c924b09ca78ce3fa152a10f782ae.tar.gz",
)

Given a WORKSPACE root of /example, create a buggy directory inside that, with the following:

A.py

import B


if __name__ == '__main__':
    b = B.Thing()
    print(b.speak())

B.py

class Thing:
    def speak(self):
        return "Hi I'm B.Thing!"

BUILD.bazel

load("@io_bazel_rules_python//python:python.bzl", "py_binary", "py_test")
load("@subpar//:subpar.bzl", "par_binary")

SRCS = [
    "A.py",
    "B.py",
]

# Simple py_binary
py_binary(
    name = "main",
    srcs = SRCS,
    default_python_version = "PY3",
    main = "A.py",
    srcs_version = "PY3",
    deps = [],
)

# Google subpar self-contained monoarchive executable
par_binary(
    name = "buggy",
    srcs = SRCS,
    default_python_version = "PY3",
#    imports = [""], <-- Uncommenting this changes our fortunes
    main = "A.py",
    srcs_version = "PY3",
    deps = [],
)

Executing bazel run --force_python=PY3 --python_path=$(which python3) //buggy:main gives the following:

INFO: Invocation ID: fa858959-917a-4240-b2ba-bd6e099c928f
INFO: Build options --force_python and --python_path have changed, discarding analysis cache.
INFO: Analysed target //buggy:main (1 packages loaded, 116 targets configured).
INFO: Found 1 target...
Target //buggy:main up-to-date:
  bazel-bin/buggy/main
INFO: Elapsed time: 0.224s, Critical Path: 0.04s
INFO: 0 processes.
INFO: Build completed successfully, 4 total actions
INFO: Build completed successfully, 4 total actions
Hi I'm B.Thing!

But executing bazel run --force_python=PY3 --python_path=$(which python3) //buggy:buggy.par gives this:

INFO: Invocation ID: 2f0582ac-c075-49b7-be27-76ad4e20a455
INFO: Analysed target //buggy:buggy.par (0 packages loaded, 16 targets configured).
INFO: Found 1 target...
Target //buggy:buggy.par up-to-date:
  bazel-bin/buggy/buggy.par
INFO: Elapsed time: 0.387s, Critical Path: 0.28s
INFO: 2 processes: 2 processwrapper-sandbox.
INFO: Build completed successfully, 10 total actions
INFO: Build completed successfully, 10 total actions
Traceback (most recent call last):
  File "/nix/store/d74rl714a32wbv5i5j5qblcl169k1vs4-python3-3.7.2/lib/python3.7/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/nix/store/d74rl714a32wbv5i5j5qblcl169k1vs4-python3-3.7.2/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/private/var/tmp/_bazel_jbelotti/6d46c1a8d79be3c21fd51c25cfdc932c/execroot/data_eng/bazel-out/darwin-py3-fastbuild/bin/buggy/buggy.par/__main__.py", line 6, in <module>
ModuleNotFoundError: No module named 'B'

The Solution

The solution is to add imports = [""], to the par_binary rule, which is fine, but in my opinion this should be unnecessary as subpar should by default behave the same as py_binary.

Interested to hear your thoughts, and open to submitting a patch for this if you think it appropriate.

@thundergolfer
Copy link
Author

I just ran into py_test having this same issue. Adding imports = [""] allowed for correct importing.

@aaliddell
Copy link
Contributor

This may be related to bazelbuild/bazel#7067

Subpar appears to be enforcing the requirement for using fully qualified imports, whereas in py_* rules you can currently get away without the prefix. In the future there appears to be an intention to enforce similar stricter rules in the py_* rules, but it is not specified quite how that'd look or when.

Setting imports = [""] causes subpar to behave the same as the py_* rules do by adding an import path that py_* is adding by default.

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

No branches or pull requests

2 participants