py_binary and oci_image don't play nice when mediated via pkg_tar

[bcsgh]

I don't know that this is a bug, of who's bug it is, but this seems like a good place to start trying to make thins better. (Feel free to point me at someone else.)

tl;dr; when pkg_tar is wrapped in the most obvious way around a py_binary so that it can be stuffed in an oci_image, files from the local workspace end up in a different location than the wrapper script that py_binary generates expect them to be in. (Non local files might as well, but things started working for me before I ever observed that.) I make no claim to it being correct, but the following commit seems to resolve the problem by inserting the workspace_name into the a suitable spot in the paths that will used in the tar.

bcsgh@8adcd2a

I expect that would be a breaking change for more than a few users. But then again, the existing behavior is wacky enough that it's possible that nobody has anything that works depending on it. (Or maybe not.)

[bcsgh]

Note: seems the relevant code is somewhat new: #754

[storm-of-fists]

Something interesting here is I put a git_override in my MODULE.bazel

bazel_dep(name = "rules_pkg")
git_override(
    module_name = "rules_pkg",
    remote = "https://github.com/bazelbuild/rules_pkg",
    commit = "994a1f5b94f5baa4904d9487622c2908fe44d2d1",
)

so that it would pull in the changes from that pr above, but im still having issues with certain files not being found. Here is how I build the image up.

py_binary(
    name = "sadie_bot",
    srcs = ["sadie_bot.py"],
    tags = [
        "manual",
        "no-sandbox",
    ],
    deps = [
        "@external_py//discord_py",
    ],
)

pkg_tar(
    name = "sadie_bot_pkg",
    srcs = [
        ":sadie_bot",
    ],
    include_runfiles = True,
)

oci_image(
    name = "sadie_bot_image",
    base = "@python_base",
    entrypoint = ["./sadie_bot"],
    tars = [":sadie_bot_pkg"],

)

And here is the output when I try to run up the container. It seems to enter the script just fine, but we don't seem to be replicating the runfiles exactly.

(.venv) [clatham@clatham-dev sadie_bot]$ docker compose up
[+] Running 1/0
 ✔ Container sadie_bot-sadie_bot-1  Recreated                         0.0s 
Attaching to sadie_bot-sadie_bot-1
sadie_bot-sadie_bot-1  | Traceback (most recent call last):
sadie_bot-sadie_bot-1  |   File "//./sadie_bot", line 559, in <module>
sadie_bot-sadie_bot-1  |     Main()
sadie_bot-sadie_bot-1  |   File "//./sadie_bot", line 490, in Main
sadie_bot-sadie_bot-1  |     assert os.path.exists(main_filename), \
sadie_bot-sadie_bot-1  | AssertionError: Cannot exec() '/./sadie_bot.runfiles/_main/projects/friend_zone/sadie_bot/sadie_bot.py': file not found.

We seem to be missing the _main directory inside the .runfiles directory.

[aiuto]

Is oci_image relevant here? What I mean, is can you repro your issue this way

bazel build //some:foo.tar
mkdir /tmp/test
cp bazel-bin/some/foo.tar /tmp/test
cd /tmp/test
tar xf foo.tar
path/to/pybin_wrapper_script

[bcsgh]

Without actually trying that (I'm ooo right now) I think so. My debugging involved extracting a listing from the tar file. The open question for me is what the layout is supposed to be, which is where OCI enters the picture.

[aiuto]

The only thing that really matters is if you untar a file and expect to run it with a single command line invocation (that is, the wrapper script) it runs. I don't know enough about docker to have much of an opinion on what is going on here. Have you asked in the rules_oci project?

…

On Tue, Jan 2, 2024 at 2:32 PM bcsgh ***@***.***> wrote: Without actually trying that (I'm ooo right now) I think so. My debugging involved extracting a listing from the tar file. The open question for me is what the layout is *supposed* to be, which is where OCI enters the picture. — Reply to this email directly, view it on GitHub <#803 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXHHHGTGQ23TDPEEJXY5TTYMROEHAVCNFSM6AAAAABBH72KXSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZUGQ2TMMZQGM> . You are receiving this because you commented.Message ID: ***@***.***>

[bcsgh]

My educated guess us that docker/oci isn't at fault here beyond (correctly) placing things beyond access of the untared stuff. (I mostly mentioned it because it's part of the motivating case and I'm not sure that it isn't relevant.)

BTW: another options that might simplify things a lot would be to switch from using File.short_path to File.path and then figure out what prefix to strip off, likely based on something like ctx.bin_dir or ctx.genfiles_dir.

[bcsgh]

Side note: there are some interesting similarities between this issue and another one in a different project: bazelbuild/rules_closure#597

The solution might not be the same across both, but if I had to guess they might be mutually informative.

[aiuto]

It is very likely related. The change that calls the workspace "_main" rather than the WORKSPACE(name="x"), and thus gives different paths between --bzlmod and non-bzlmod build broke a lot of assumptions.

…

On Wed, Jan 3, 2024 at 1:24 PM bcsgh ***@***.***> wrote: Side note: there are some interesting similarities between this issue and another one in a different project: bazelbuild/rules_closure#597 <bazelbuild/rules_closure#597> The solution might not be the same across both, but if I had to guess they might be mutually informative. — Reply to this email directly, view it on GitHub <#803 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXHHHAF2P4JYUDFVA64MI3YMWO43AVCNFSM6AAAAABBH72KXSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZVG44DKNRUGI> . You are receiving this because you commented.Message ID: ***@***.***>

[ewianda]

Seeing the same error. The following patch works for us.

--- a/pkg/private/pkg_files.bzl
+++ b/pkg/private/pkg_files.bzl
@@ -69,6 +69,7 @@
         "content_map": "in/out The content_map we are building up",
         "file_deps": "in/out list of file Depsets represented in the map",
         "label": "ctx.label",
+        "workspace_name": "ctx.workspace_name",

         # Behaviors
         "allow_duplicates_with_different_content": "bool: don't fail when you double mapped files",
@@ -116,6 +117,7 @@
         content_map = dict(),
         file_deps = list(),
         label = label,
+        workspace_name = ctx.workspace_name,
         allow_duplicates_with_different_content = allow_duplicates_with_different_content,
         strip_prefix = strip_prefix,
         include_runfiles = include_runfiles,
@@ -429,7 +431,7 @@
             base_file = the_executable or all_files[0]
             base_file_path = mapping_context.path_mapper(
                 dest_path(base_file, data_path, data_path_without_prefix))
-            base_path = base_file_path + ".runfiles"
+            base_path = base_file_path + ".runfiles/" + mapping_context.workspace_name

[bcsgh]

So what's next on this?