Skip to content
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.

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

Add support for repo mapping #878

Merged
merged 4 commits into from
Aug 14, 2024
Merged

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 <thomaslam@canva.com>
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.

Signed-off-by: Thomas Lam <thomaslam@canva.com>
@lamcw lamcw marked this pull request as ready for review 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?

pkg/mappings.bzl Outdated Show resolved Hide resolved
Signed-off-by: Thomas Lam <thomaslam@canva.com>
Signed-off-by: Thomas Lam <thomaslam@canva.com>
@lamcw lamcw requested a review from aiuto August 14, 2024 04:04
@@ -122,5 +122,7 @@ def get_repo_mapping_manifest(src):
"""
files_to_run_provider = get_files_to_run_provider(src)
if files_to_run_provider:
# repo_mapping_manifest may not exist in older Bazel versions (<7.0.0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Now "that" is a comment which makes it clear why you are doing something specal. Thanks.

@aiuto aiuto merged commit 447fb8e into bazelbuild:main Aug 14, 2024
2 checks passed
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
3 participants