Skip to content

[not for merge] Test a change for #3439 #3440

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.

Sign up for GitHub

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

Closed
shayanhoshyari wants to merge 1 commit into from
+15 −25
Closed

[not for merge] Test a change for #3439 #3440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions python/private/py_executable.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ def _create_executable(
[stage2_bootstrap] + (
venv.files_without_interpreter if venv else []
),
).merge(venv.lib_runfiles)
)
zip_main = _create_zip_main(
ctx,
stage2_bootstrap = stage2_bootstrap,
Expand Down Expand Up @@ -637,11 +637,6 @@ def _create_venv(ctx, output_prefix, imports, runtime_details, add_runfiles_root
venv,
),
),
# venv files for user library dependencies (files that are specific
# to the executable bootstrap and python runtime aren't here).
lib_runfiles = ctx.runfiles(
symlinks = venv_app_files.runfiles_symlinks,
),
)

def _map_each_identity(v):
Expand Down
33 changes: 14 additions & 19 deletions python/private/venv_runfiles.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,6 @@ def create_venv_app_files(ctx, deps, venv_dir_map):
{type}`struct` with the following attributes:
* {type}`list[File]` `venv_files` additional files created for
the venv.
* {type}`dict[str, File]` `runfiles_symlinks` map intended for
the `runfiles.symlinks` argument. A map of main-repo
relative paths to File.
"""

# maps venv-relative path to the runfiles path it should point to
Expand All @@ -49,31 +46,29 @@ def create_venv_app_files(ctx, deps, venv_dir_map):

link_map = build_link_map(ctx, entries)
venv_files = []
runfiles_symlinks = {}

for kind, kind_map in link_map.items():
base = venv_dir_map[kind]
for venv_path, link_to in kind_map.items():
bin_venv_path = paths.join(base, venv_path)
if is_file(link_to):
symlink_from = "{}/{}".format(ctx.label.package, bin_venv_path)
runfiles_symlinks[symlink_from] = link_to

else:
venv_link = ctx.actions.declare_symlink(bin_venv_path)
venv_link_rf_path = runfiles_root_path(ctx, venv_link.short_path)
rel_path = relative_path(
# dirname is necessary because a relative symlink is relative to
# the directory the symlink resides within.
from_ = paths.dirname(venv_link_rf_path),
to = link_to,
)
ctx.actions.symlink(output = venv_link, target_path = rel_path)
venv_files.append(venv_link)
# Just for demonstration purposes,
# there is definitely a better thing to do upstream that removeprefix("../")
link_to_path = link_to.short_path.removeprefix("../") if is_file(link_to) else link_to
Copy link
Collaborator

@rickeylev rickeylev Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to carry a patch, then I suggest calling runfiles_root_path, something like this:

link_to = runfiles_root_path(ctx, link_to.short_path) if is_file(link_to) else link_to

The case not being handled is if the File being mapped into the venv is coming from the main repository. In that case, short_path is foo/bar.txt, but the runfiles root path is e.g. _main/foo/bar.txt. Most things that go into the venv come from external repos, so you rarely see entries without ../ in their short path.


venv_link = ctx.actions.declare_symlink(bin_venv_path)
venv_link_rf_path = runfiles_root_path(ctx, venv_link.short_path)
rel_path = relative_path(
# dirname is necessary because a relative symlink is relative to
# the directory the symlink resides within.
from_ = paths.dirname(venv_link_rf_path),
to = link_to_path,
)
ctx.actions.symlink(output = venv_link, target_path = rel_path)
venv_files.append(venv_link)

return struct(
venv_files = venv_files,
runfiles_symlinks = runfiles_symlinks,
)

# Visible for testing
Expand Down