Skip to content

Add support for repo mapping#878

Merged
aiuto merged 4 commits intobazelbuild:mainfrom
lamcw:support-repo-mapping
Aug 14, 2024
Merged

Add support for repo mapping#878
aiuto merged 4 commits intobazelbuild:mainfrom
lamcw:support-repo-mapping

Conversation

@lamcw
Copy link
Contributor

@lamcw lamcw commented Jul 25, 2024

Context

When a binary is packaged with pkg_* rules, the repo_mapping manifest file is not included in the archive. For example

java_binary(
    name = "example",
    main_class = "...",
)

pkg_tar(
    name = "example_tar",
    srcs = [":example"],
    include_runfiles = True,
)

The above generates an archive that does not have _repo_mapping under 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_pkg such that when

  1. bzlmod is enabled, and
  2. include_runfiles = True

it copies the repo mapping manifest into the archive at the correct location

i.e.

example
example.repo_mapping
example.runfiles/
├─ _repo_mapping

Closes #769

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")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This only adds example.runfiles/_repo_mapping -- example.repo_mapping is still missing.

@lamcw lamcw marked this pull request as ready for review July 29, 2024 02:57
@lamcw lamcw requested review from aiuto and cgrindel as code owners July 29, 2024 02:57
Copy link
Collaborator

@cgrindel cgrindel left a comment

Choose a reason for hiding this comment

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

LGTM. I will wait to merge it so that @tonyaiuto can take a look.

@lamcw
Copy link
Contributor Author

lamcw commented Aug 7, 2024

@aiuto can you PTAL?

@lamcw lamcw requested a review from aiuto August 14, 2024 04:04
@aiuto aiuto merged commit 447fb8e into bazelbuild:main Aug 14, 2024
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")

Choose a reason for hiding this comment

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

@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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Umm, because we don't control the definition of DefaultInfo. In bazel 6, you fail if you reference repo_mapping_manifest.

Choose a reason for hiding this comment

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

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:

Suggested change
return getattr(files_to_run_provider, "repo_mapping_manifest")
return getattr(files_to_run_provider, "repo_mapping_manifest", None)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you have a reproducible failure that this corrects?

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.

Look into repo-mapping support

5 participants