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

Use merge_all() in emit_archive. #3068

Merged

Conversation

moisesvega
Copy link
Contributor

@moisesvega moisesvega commented Feb 16, 2022

What type of PR is this?
Other

What does this PR do? Why is it needed?
This PR updates emit_archive to use the new api merge_all() added in bazel 5.0.

Because of the changes in the Starlak language, it is possible to hit depth_limit while using merge() inside of a loop, the fix is using merge_all() instead of merge().

This PR includes:

  • Use merge_all() in emit_archive

@linzhp
Copy link
Contributor

linzhp commented Feb 18, 2022

It appears that we can get Bazel version using native.bazel_version like this. Can you use so we don't need to bump the minimal bazel version now?

go/private/actions/archive.bzl Outdated Show resolved Hide resolved
@moisesvega
Copy link
Contributor Author

It appears that we can get Bazel version using native.bazel_version like this. Can you use so we don't need to bump the minimal bazel version now?

At the moment native.bazel_version is not available for rules. Here is the conversation about allowing native.bazel_version outside of WORKSPACE.

For that reason, using "hasattr(runfiles, "merge_all")" is enough to distinguish between skylark API's.

@linzhp linzhp merged commit e9a7054 into bazel-contrib:master Feb 23, 2022
@moisesvega moisesvega deleted the update-emite_archive-for-bazel5.0 branch February 23, 2022 19:11
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.

2 participants