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

Python .zip in OutputGroupInfo is missing shebang line #17629

Open
fughilli opened this issue Feb 28, 2023 · 6 comments
Open

Python .zip in OutputGroupInfo is missing shebang line #17629

fughilli opened this issue Feb 28, 2023 · 6 comments
Labels

Comments

@fughilli
Copy link

Description of the bug:

The native py_binary implementation was amended in #9453 to export the zipped python executable in an OutputGroupInfo named python_zip_file. However, this archive is different than the one produced when building the py_binary with --build_python_zip, since it does not include the shebang line needed to make the stub execute when invoked on the command line. This is because the action that prepends the shebang is gated on the command line flag:
https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java#L252

This code path should be amended such that the artifact in the output group is the same as the one produced when building with the flag (i.e., remove the gating on the flag and have the resulting artifact populate the OutputGroupInfo and optionally be output directly when the flag is provided).

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

main.py

print("hello world")

BUILD

filegroup(
name = "main_zip",
srcs = [":main"],
output_group = "python_zip_file",
)

py_binary(
name = "main",
srcs = ["main.py"],
)

Invoke:

bazel build :main_zip

Inspect:

hexdump -Cv bazel-bin/main.zip | head -n 10

Observe no shebang line.

Invoke:

bazel build :main --build_python_zip

Inspect:

hexdump -Cv bazel-bin/main | head -n 10

Observe a shebang line.

Which operating system are you running Bazel on?

Linux

What is the output of bazel info release?

release 5.2.0

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

No response

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

No response

Have you found anything relevant by searching the web?

No response

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

No response

@ajonnavi
Copy link

ajonnavi commented Aug 1, 2023

Seeing the same issue. Would really if there was a way around this issue.

More importantly, is it possible for a downstream target to depend on the zip output of py_binary?

The way we are tackling this right now is to use a filegroup that generates this zip, and the downstream target(container rule) uses that filegroup output.

Because the zip lacks the shebang, it needs to be invoked using a python interpreter inside the container.

@ahumesky
Copy link
Contributor

ahumesky commented Jul 3, 2024

One workaround is to add the shebang manually in a genrule:

py_binary(
  name = "program",
  srcs = ["program.py"],
)

filegroup(
  name = "program_zip",
  srcs = [":program"],
  output_group = "python_zip_file",
)

genrule(
  name = "program_zip_py_executable",
  srcs = [":program_zip"],
  outs = ["program_zip_py_executable.zip"],
  cmd = "echo '#!/usr/bin/env python3' | cat - $< >$@",
  executable = True,
)

This will probably beak things on Windows though, since on windows an exe is produced

@allsey87
Copy link

allsey87 commented Jul 4, 2024

If an exe is already produced under Windows, then the genrule step here can be skipped, right?

Also, if this is the case, that sounds like an inconsistency if the python_zip_file output group is executable on Windows but not Linux/Mac (without this hack).

@allsey87
Copy link

allsey87 commented Jul 4, 2024

@aignas should this issue be transferred to rules_python perhaps?

@aignas
Copy link

aignas commented Jul 4, 2024

This issue is about the native implementation, so it should be probably closed as won't do. Users should migrate to rules_python instead of using the native rules. Feel free to create issues if they do not work in your context.

@ahumesky
Copy link
Contributor

ahumesky commented Jul 9, 2024

I believe that this is a problem in rules_python as well:

WORKSPACE:

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

http_archive(
    name = "rules_python",
    sha256 = "778aaeab3e6cfd56d681c89f5c10d7ad6bf8d2f1a72de9de55b23081b2d31618",
    strip_prefix = "rules_python-0.34.0",
    url = "https://github.com/bazelbuild/rules_python/releases/download/0.34.0/rules_python-0.34.0.tar.gz",
)

load("@rules_python//python:repositories.bzl", "py_repositories")

py_repositories()

BUILD:

load("@rules_python//python:defs.bzl", "py_binary", "py_library", "py_test")

py_binary(
  name = "bin",
  srcs = ["bin.py"],
)

filegroup(
  name = "bin_zip",
  srcs = [":bin"],
  output_group = "python_zip_file",
)

bin.py:

print("hello world")
$ bazel build bin_zip
Starting local Bazel server and connecting to it...
INFO: Analyzed target //:bin_zip (90 packages loaded, 760 targets configured).
INFO: Found 1 target...
Target //:bin_zip up-to-date:
  bazel-bin/bin.zip
INFO: Elapsed time: 5.339s, Critical Path: 0.06s
INFO: 6 processes: 5 internal, 1 linux-sandbox.
INFO: Build completed successfully, 6 total actions

$ bazel-bin/bin.zip
invalid file (bad magic number): Exec format error

$ hexdump -C -n 16 bazel-bin/bin.zip
00000000  50 4b 03 04 0a 00 00 00  08 00 00 00 21 3c da 6b  |PK..........!<.k|
00000010

$ python bazel-bin/bin.zip
hello world

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

No branches or pull requests

6 participants