have explicit symlinks add files to runfiles to ensure target exists

Author: rickeylev

python/private/py_executable.bzl

@@ -79,12 +79,21 @@ ExplicitSymlink = provider(
 A runfile that should be created as a symlink pointing to a specific location.
 
 This is only needed on Windows, where Bazel doesn't preserve declare_symlink
-with relative paths.
+with relative paths. This is basically manually captures what using
+declare_symlink(), symlink() and runfiles like so would capture:
+
+```
+link = declare_symlink(...)
+link_to_path = relative_path(from=link, to=target)
+symlink(output=link, target_path=link_to_path)
+runfiles.add([link, target])
+```
 """,
     fields = {
-        "link_to": "Path the symlink should point to",
-        "rf_path": "runfile-root-relative path for the link",
-        "venv_rel_path": "venv-root-relative path for the link",
+        "link_to_path": "Path the symlink should point to",
+        "runfiles_path": "runfiles-root-relative path for the symlink",
+        "venv_path": "venv-root-relative path for the symlink",
+        "files": "depset[File] of files that should be included if this symlink is used",
     },
 )
 
@@ -736,7 +745,7 @@ def _create_venv_unixy(ctx, *, venv_ctx_rel_root, runtime, interpreter_actual_pa
 
 def _create_venv_windows(ctx, *, venv_ctx_rel_root, runtime, interpreter_actual_path):
     interpreter_runfiles = builders.RunfilesBuilder()
-    interpreter_symlinks = []
+    interpreter_symlinks = builders.DepsetBuilder()
 
     # Some wrappers around the interpreter (e.g. pyenv) use the program
     # name to decide what to do, so preserve the name.
@@ -751,17 +760,15 @@ def _create_venv_windows(ctx, *, venv_ctx_rel_root, runtime, interpreter_actual_
         ctx.actions.symlink(output = interpreter, target_file = runtime.interpreter)
 
         rf_path = runfiles_root_path(ctx, interpreter.short_path)
-        interpreter_symlinks.append(ExplicitSymlink(
-            rf_path = rf_path,
-            venv_rel_path = venv_rel_path,
-            link_to = relative_path(
-                # dirname is necessary because a relative symlink is relative to
-                # the directory the symlink resides within.
-                from_ = paths.dirname(rf_path),
-                to = interpreter_actual_path,
-            ),
+        interpreter_symlinks.add(ExplicitSymlink(
+            runfiles_path = rf_path,
+            venv_path = venv_rel_path,
+            link_to_path = interpreter_actual_path,
+            files = depset([runtime.interpreter]),
         ))
     else:
+        # It's OK to use declare_symlink here because an absolute path
+        # will be written to it, so Bazel won't mangle it.
         interpreter = ctx.actions.declare_symlink("{}/{}".format(venv_bin_ctx_rel_path, py_exe_basename))
         interpreter_runfiles.add(interpreter)
         ctx.actions.symlink(output = interpreter, target_path = runtime.interpreter_path)
@@ -771,19 +778,18 @@ def _create_venv_windows(ctx, *, venv_ctx_rel_root, runtime, interpreter_actual_
     for f in runtime.venv_bin_files:
         venv_rel_path = paths.join(venv_bin_rel_path, f.basename)
         venv_ctx_rel_path = paths.join(venv_ctx_rel_root, venv_rel_path)
+
         venv_file = ctx.actions.declare_file(venv_ctx_rel_path)
         ctx.actions.symlink(output = venv_file, target_file = f)
+
         interpreter_runfiles.add(venv_file)
+
         rf_path = runfiles_root_path(ctx, venv_file.short_path)
-        interpreter_symlinks.append(ExplicitSymlink(
-            rf_path = rf_path,
-            venv_rel_path = venv_rel_path,
-            link_to = relative_path(
-                # dirname is necessary because a relative symlink is relative to
-                # the directory the symlink resides within.
-                from_ = paths.dirname(rf_path),
-                to = runfiles_root_path(ctx, f.short_path),
-            ),
+        interpreter_symlinks.add(ExplicitSymlink(
+            runfiles_path = rf_path,
+            venv_path = venv_rel_path,
+            link_to_path = runfiles_root_path(ctx, f.short_path),
+            files = depset([f])
         ))
 
     # See site.py logic: Windows uses a version/build agnostic site-packages path
@@ -796,7 +802,7 @@ def _create_venv_windows(ctx, *, venv_ctx_rel_root, runtime, interpreter_actual_
         bin_dir = venv_bin_ctx_rel_path,
         recreate_venv_at_runtime = True,
         interpreter_runfiles = interpreter_runfiles.build(ctx),
-        interpreter_symlinks = depset(interpreter_symlinks),
+        interpreter_symlinks = interpreter_symlinks.build(),
     )
 
 def _venv_details(
@@ -972,7 +978,7 @@ def _create_stage1_bootstrap(
     )
 
 def _map_runtime_venv_symlink(entry):
-    return entry.venv_rel_path + "|" + entry.link_to
+    return entry.venv_path + "|" + entry.link_to_path
 
 def _create_zip_file(ctx, *, output, zip_main, runfiles):
     """Create a Python zipapp (zip with __main__.py entry point)."""

python/private/python_bootstrap_template.txt

@@ -371,22 +371,25 @@ print(site.getsitepackages(["{venv_src}"])[-1])
             target = join(python_home, f)
             _symlink_exist_ok(from_=venv_path, to=target)
 
-    for venv_rel_path, link_to in RUNTIME_VENV_SYMLINKS.items():
-        venv_abs_path = join(venv, venv_rel_path)
-        link_to = normpath(join(runfiles_venv, dirname(venv_rel_path), link_to))
-        _symlink_exist_ok(from_=venv_abs_path, to=link_to)
-
     runfiles_venv_bin = join(runfiles_venv, BIN_DIR_NAME)
     # The runfiles bin directory may not exist if it's empty, e.g.
     # supports_build_time_venv=False and the interpreter is resolved at
     # runtime.
     if os.path.exists(runfiles_venv_bin):
-      # Add any missing build-time entries under bin/
+      # Add any missing build-time entries under bin/. Do this before
+      # the manual symlink creation to better mimic what the
+      # regular runfiles directory looks like.
       for f_basename in os.listdir(runfiles_venv_bin):
           venv_path = join(venv, BIN_DIR_NAME, f_basename)
           target = join(runfiles_venv_bin, f_basename)
           _symlink_exist_ok(from_=venv_path, to=target)
 
+    for venv_rel_path, link_to_rf_path in RUNTIME_VENV_SYMLINKS.items():
+        venv_abs_path = join(venv, venv_rel_path)
+        link_to = normpath(join(runfiles_venv, link_to_rf_path))
+        os.makedirs(dirname(venv_abs_path), exist_ok=True)
+        _symlink_exist_ok(from_=venv_abs_path, to=link_to)
+
     _symlink_exist_ok(from_=join(venv, "lib"), to=join(runfiles_venv, "lib"))
     _symlink_exist_ok(from_=venv_site_packages, to=runfiles_venv_site_packages)
 

python/private/zipapp/py_zipapp_rule.bzl

@@ -138,9 +138,12 @@ def _create_zip(ctx, py_runtime, py_executable, stage2_bootstrap):
     )
     inputs = builders.DepsetBuilder()
     manifest.add("regular|0|__main__.py|{}".format(zip_main.path))
-    is_windows = target_platform_has_any_constraint(ctx, ctx.attr._windows_constraints)
+
+    # unfortunately, we have to flatten to add the files to inputs.
     for entry in py_executable.venv_interpreter_symlinks.to_list():
-        manifest.add("symlink|{}|{}".format(entry.rf_path, entry.link_to))
+        manifest.add("symlink|{}|{}".format(entry.runfiles_path, entry.link_to_path))
+        inputs.add(entry.files)
+
     inputs.add(zip_main)
     _build_manifest(ctx, manifest, runfiles, inputs)
 
@@ -155,6 +158,7 @@ def _create_zip(ctx, py_runtime, py_executable, stage2_bootstrap):
         zipper_args.add(ctx.attr.compression, format = "--compression=%s")
     zipper_args.add("--runfiles-dir=runfiles")
 
+    is_windows = target_platform_has_any_constraint(ctx, ctx.attr._windows_constraints)
     zipper_args.add("\\" if is_windows else "/", format = "--pathsep=%s")
 
     actions_run(

tools/private/zipapp/zipper.py

@@ -61,8 +61,10 @@ def _parse_entry(
         _, is_symlink_str, runfile_path, content_path = parts
         zip_path = unix_join(runfiles_dir, runfile_path)
     elif type_ == "symlink":
-        _, runfile_path, content_path = parts
+        _, runfile_path, link_to_rf_path = parts
         zip_path = unix_join(runfiles_dir, runfile_path)
+        link_to_rf_path = unix_join(runfiles_dir, link_to_rf_path)
+        content_path = os.path.relpath(link_to_rf_path, start=zip_path)
         is_symlink_str = "2"
     else:
         raise ValueError(
@@ -214,8 +216,8 @@ def main():
 5. `rf-root-symlink|is_symlink|runfile_root_path|content_path`: Store a
    runfiles-root-relative path in the zip.
 
-6. `symlink|runfile_root_path|link_to`: Store a symlink with the value
-    of `link_to`.
+6. `symlink|runfile_root_path|link_to_path_rf_path`: Store a symlink that
+   stores a relative path from `runfile_root_path` to `link_to_rf_path`
 
 In all cases, `is_symlink` has the following values:
 * `1` means it should be stored as a symlink whose value is read