pkg_zip creates the zip file with broken link inside venv

[gfrankliu]

Using latest rules_python 1.8.2 (venv enabled) and rules_pkg 1.2.0

py_binary(
    name = "app",
    ...
)

pkg_zip(
    name = "app_zip",
    srcs = [":app"],
    include_runfiles = True,
)

When I unzip the app_zip.zip file, I can see all the symlinks inside app.runfiles/_main/utils/_app.venv are broken. They all have one extra ../

I checked unzipped directory structure, those rules_python++pip* directories are wrongly located inside app.runfiles/_main/ while on the build host bazel-bin, they are directly inside app.runfiles, parallel to _main. This caused all symlinks in venv broken.

[gfrankliu]

Next I tried to use pkg_tar. The resulting tar file doesn't seem to have this broken relative symlink issue from within _app.venv/lib/python3.10/site-packages/, but there is another problem. My py_binary has a deps of "google-cloud-bigquery" and "google-cloud-storage", but inside the tar file, I don't even see a directory _app.venv/lib/python3.10/site-packages/google/cloud/ , even though on the host, under bazel-bin, I can see _app.venv/lib/python3.10/site-packages/google/cloud/bigquery and _app.venv/lib/python3.10/site-packages/google/cloud/storage, etc.

[gfrankliu]

My real goal is to create a self contained zipapp for this one py_binary. Before I enabled venv, I could do it:

filegroup(
    name = "app_zip",
    srcs = [":app"],
    output_group = "python_zip_file",
)

Is there an equivalent to build this one target after venv is enabled? I don't need this for every py_binary target.

[rickeylev]

I think #3539 will fix this. That's a bit away from landing, though (probably a couple weeks). While working on that, I realized that the zipapp+venv doesn't work 😬.

Do you just want zip or tar as a packaging mechanism, do you want a self-executable zip/tar, or do you want a zipapp zip (one with __main__.py) ?

If just a packaging mechanism, then you can copy/paste the core bits of logic from the above PR into a custom rule. The two parts you need are:

1. In py_zipapp_rule.bzl, the build_manifest function: this correctly processes a runfiles object into a manifest for a tool to process
2. In zipper.py, the logic of how to handle the different lines in the manifest.

(all the extra logic about PyExecutableInfo and zipper cli args shouldn't be necessary if you just want to package an arbitrary target's runfiles into a zip)

If you also want __main__.py and/or self-executable-ness, then that'll probably require more. Patching the above parts into rules_python should be possible, too (it should just need the custom zipper tool and the extra runfiles-mapping-manifest parts). Happy to approve a PR to that effect in the interim.

pkg_zip put files under main, when they should be above

It sounds like pkg_zip isn't handling external files correctly. I wouldn't be surprised if its not correctly handling the ../ part of paths that Bazel reports for external files.

google-cloud-storage etc missing from site-packages

Just a guess, but it may be that rules_pkg isn't handling runfiles.symlinks and/or runfiles.root_symlinks (at all or correctly).

[gfrankliu]

While waiting for your #3539 , a simple packaging mechanism will work for me. I can have a separate helper script to unzip/untar the package, then run the bootstrap script. That's why I am trying the pkg_zip and then pkg_tar, only to find out neither works. For pkg_zip, I can understand maybe it can't handle the ../ path properly, but I would expect pkg_tar can natively support symlinks.

While debugging the pkg_tar issue, I see it actually uses _app.venv that is parallel to the app.runfiles. We thought that venv was useless and should be ignored since it has broken symlinks. In reality, it seems pkg_tar uses it and magically packs it as the _app.venv inside app.runfiles/_main/.... Due to this magic "move", those invalid relative symlinks become valid. The only problem is that this _app.venv doesn't have the site-packages/google/cloud/ directory. If pkg_tar were to directly use the _app.venv from under the app.runfiles/main directly, it would've got site-packages/google/cloud/ I guess the real fix would be making both venv directory match. Then it wouldn't matter which one pkg_tar uses.

[gfrankliu]

While troubleshooting, I was playing with the "venvs_use_declare_symlink". Based on doc, if I set it no, venv will be created at runtime, but I still see it venv directories created in build time on the build host under bazel-bin/runfiles. Am I missing something?

[gfrankliu]

ok, I throw this to Gemini who created py_runfiles.bzl for me, and I can now use

py_runfiles_pkg(
    name = "app_pkg",
    binary = ":app",
)

which creates the app_pkg.tar.gz for me that includes runfiles and venv symlinks.

We still need to at least fix pkg_tar to support venv, if we think pkg_zip is not fixable.

[rickeylev]

FYI: @gfrankliu in the py_runfiles.bzl code you posted: runfiles.symlinks: the dest need to have ctx.workspace prepended to them (runfiles.symlink entries are always relative to the main repo; it's a weird historical thing). Also empty_filenames are missing, but if you have the auto-init.py generation disabled, that isn't an issue.

[gfrankliu]

I have build --incompatible_default_to_explicit_init_py in my .bazelrc. Is that what you meant by "auto-init.py generation disabled"? Regarding "have ctx.workspace prepended", in my wrapper script where I untar this distribution tar file to a temp location, I had to set "RUNFILES_DIR" to that temp location before running the bootstrap script. That may be why I didn't run into any issues even though I didn't prepend the ctx.workspace?

Again, it is not my code. Gemini wrote it based on my requirement. You are welcome to include it in #3539 so I can switch to the official implementation :)

[gfrankliu]

FYI: @gfrankliu in the py_runfiles.bzl code you posted: runfiles.symlinks: the dest need to have ctx.workspace prepended to them (runfiles.symlink entries are always relative to the main repo; it's a weird historical thing). Also empty_filenames are missing, but if you have the auto-init.py generation disabled, that isn't an issue.

I gave your suggestions to Gemini and we have an updated py_runfiles.bzl.

[kwlzn]

I think this issue may not be specific to zip packaging? we are also seeing dangling symlinks in bazel-bin runfiles - the symlinks point to a ../../... depth that descends lower than / which I think points to a symlink construction bug somewhere?

[gfrankliu]

I think this issue may not be specific to zip packaging? we are also seeing dangling symlinks in bazel-bin runfiles - the symlinks point to a ../../... depth that descends lower than / which I think points to a symlink construction bug somewhere?

Are you referring to the "other venv" directory that is parallel to runfiles, not the venv inside runfiles/_main/ ? If so, check this other discussion.