Skip to content

Fix Bazel build output resolution under --symlink_prefix#67

Open
hartikainen wants to merge 5 commits into
google-deepmind:mainfrom
hartikainen:enable-symlink_prefix
Open

Fix Bazel build output resolution under --symlink_prefix#67
hartikainen wants to merge 5 commits into
google-deepmind:mainfrom
hartikainen:enable-symlink_prefix

Conversation

@hartikainen

@hartikainen hartikainen commented Jun 7, 2026

Copy link
Copy Markdown

Stacked on top of #53.

_build_multiple_targets currently joints the workspace directory with each output's path_prefix (bazel-out/...), which only resolves through the default bazel-out convenience symlink. Under a non-default --symlink_prefix (e.g. --symlink_prefix=.bazel/) that symlink doesn't exist, so the returned paths point nowhere and packaging fails.

This commit refactors the resolution to prefer file:// URI that Bazel emits in the BEP File messages. If I understand correctly, these are the authoritative on-disk paths, independent of --symlink_prefix. It falls back to <execution_root>/<path_prefix>/<name> for non-file:// URIs (e.g. bytestream:// outputs from remote builds that were downloaded locally via --remote_download_outputs=toplevel). Note this only resolves where an output lives; under --remote_download_minimal the bytes are never materialized locally, so downstream consumers still fail regardless of the path returned.

This also extracts the per-File resolution into a pure _resolve_output_path(file, exec_root) helper function and thus makes the logic unit-testable without shelling out to Bazel. The execution root is resolved lazily (and at most once) via functools.lru_cache to keep the unnecessary bazel info off the local-build path. A non-localhost URI authority is preserved as a //host/... UNC path rather than silently dropped. Adds tests covering file:// decoding (including percent-encoding and authorities) and the path_prefix fallback.

Implemented with the help of gemini-cli.

@hartikainen hartikainen force-pushed the enable-symlink_prefix branch 2 times, most recently from 16dce0e to 9cde57d Compare June 7, 2026 19:50
`_build_multiple_targets` currently joints the workspace directory with
each output's `path_prefix` (`bazel-out/...`), which only resolves
through the default `bazel-out` convenience symlink. Under a non-default
`--symlink_prefix` (e.g. `--symlink_prefix=.bazel/`) that symlink
doesn't exist, so the returned paths point nowhere and packaging fails.

This commit refactors the resolution to prefer `file://` URI that Bazel emits in the BEP `File`
messages. If I understand correctly, these are the authoritative on-disk
paths, independent of `--symlink_prefix`. It falls back to
`<execution_root>/<path_prefix>/<name>` for non-`file://` URIs (e.g.
`bytestream://` outputs from remote builds that were downloaded locally
via `--remote_download_outputs=toplevel`). Note this only resolves where
an output lives; under `--remote_download_minimal` the bytes are never
materialized locally, so downstream consumers still fail regardless of
the path returned.

This also extracts the per-`File` resolution into a pure
`_resolve_output_path(file, exec_root)` helper function
and thus makes the logic unit-testable without shelling out to Bazel.
The execution root is resolved lazily (and at most once) via
`functools.lru_cache` to keep the unnecessary `bazel info` off the
local-build path. A non-`localhost` URI authority is preserved as a
`//host/...` UNC path rather than silently dropped. Adds tests covering
`file://` decoding (including percent-encoding and authorities) and the
`path_prefix` fallback.
@hartikainen hartikainen force-pushed the enable-symlink_prefix branch from 9cde57d to f163839 Compare June 7, 2026 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant