Conversation
Esp. when include_runfiles = True Closes bazelbuild#769 Signed-off-by: Thomas Lam <[email protected]>
| target[DefaultInfo].files_to_run.repo_mapping_manifest != None | ||
| ): | ||
| repo_mapping_manifest = target[DefaultInfo].files_to_run.repo_mapping_manifest | ||
| dest_path = paths.join(src_dest_paths_map[src] + ".runfiles", "_repo_mapping") |
There was a problem hiding this comment.
This only adds example.runfiles/_repo_mapping -- example.repo_mapping is still missing.
Signed-off-by: Thomas Lam <[email protected]>
cgrindel
left a comment
There was a problem hiding this comment.
LGTM. I will wait to merge it so that @tonyaiuto can take a look.
|
@aiuto can you PTAL? |
Signed-off-by: Thomas Lam <[email protected]>
Signed-off-by: Thomas Lam <[email protected]>
| if files_to_run_provider: | ||
| # repo_mapping_manifest may not exist in older Bazel versions (<7.0.0) | ||
| # https://github.com/bazelbuild/bazel/issues/19937 | ||
| return getattr(files_to_run_provider, "repo_mapping_manifest") |
There was a problem hiding this comment.
@aiuto Naïve question: why not adding None as the default value when the attribute is not found in the provider?
As mentioned in the comment, there are cases where it doesn't exist.
There was a problem hiding this comment.
Umm, because we don't control the definition of DefaultInfo. In bazel 6, you fail if you reference repo_mapping_manifest.
There was a problem hiding this comment.
The documentation of the get_repo_mapping_manifest function contains the word Safely.
I would expect that this function never throws an error, and returns None when the FilesToRunProvider is not found or when the repo_mapping_manifest attribute is not found on the provider.
This can be achieved with:
| return getattr(files_to_run_provider, "repo_mapping_manifest") | |
| return getattr(files_to_run_provider, "repo_mapping_manifest", None) |
There was a problem hiding this comment.
Do you have a reproducible failure that this corrects?
Context
When a binary is packaged with
pkg_*rules, the repo_mapping manifest file is not included in the archive. For exampleThe above generates an archive that does not have
_repo_mappingunder runfiles directory even when bzlmod is enabled, and therefore runfiles libraries are not able to convert apparent repo names into canonical repo names.Changes
Patch
rules_pkgsuch that wheninclude_runfiles = Trueit copies the repo mapping manifest into the archive at the correct location
i.e.
Closes #769