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

Copy over runfiles library from Bazel #239

Closed
wants to merge 3 commits into from
Closed

Conversation

fmeum
Copy link
Contributor

@fmeum fmeum commented Oct 31, 2024

The runfiles library can be maintained independently of Bazel releases and bazel_tools can refer to it via an alias.

Also set flags to build and test with a hermetic JDK 8 to ensure compatibility with that version.

@fmeum fmeum requested review from comius and a team as code owners October 31, 2024 08:31
@fmeum
Copy link
Contributor Author

fmeum commented Oct 31, 2024

@hvadehra @meteorcloudy I just realized that we could also move this over and delete it from bazel_tools. What do you think? We could fix bazelbuild/bazel#24150 here.

@comius
Copy link
Collaborator

comius commented Oct 31, 2024

The move sounds fine to me. I'll let @hvadehra do the review.

@comius comius requested review from hvadehra and removed request for comius October 31, 2024 09:27
@fzakaria
Copy link

@hvadehra @meteorcloudy I just realized that we could also move this over and delete it from bazel_tools. What do you think? We could fix bazelbuild/bazel#24150 here.

I included the fix in bazelbuild/bazel#24161
It doesn't have any unit tests since there seem to be none to add.
A further contribution may be to include https://github.com/google/compile-testing at some point

@fmeum fmeum force-pushed the runfiles branch 2 times, most recently from b2b36ed to debe9bd Compare November 5, 2024 09:01
@fmeum
Copy link
Contributor Author

fmeum commented Nov 5, 2024

Stacked on #238

No longer stacked

@fmeum fmeum force-pushed the runfiles branch 3 times, most recently from 3cc5085 to 2cbac3a Compare November 5, 2024 09:12
@fmeum
Copy link
Contributor Author

fmeum commented Nov 5, 2024

@hvadehra This is ready for review now, CI is green.

@fmeum
Copy link
Contributor Author

fmeum commented Nov 12, 2024

@hvadehra Friendly ping

Copy link
Member

@hvadehra hvadehra left a comment

Choose a reason for hiding this comment

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

Sorry about the delay.

.bazelrc Outdated Show resolved Hide resolved
test/repositories.bzl Show resolved Hide resolved
@fmeum fmeum force-pushed the runfiles branch 3 times, most recently from ff52380 to 77d33a8 Compare November 13, 2024 16:22
@fmeum fmeum requested a review from hvadehra November 13, 2024 16:22
@fmeum
Copy link
Contributor Author

fmeum commented Nov 13, 2024

@Wyverald http_jar in rules_java doesn't work with Bazel 6 as it loads cache.bzl, which isn't available in that Bazel version.

The runfiles library can be maintained independently of Bazel releases and `bazel_tools` can refer to it via an alias.

Also set flags to build and test with a hermetic JDK 8 to ensure compatibility with that version.
@Wyverald
Copy link
Member

@Wyverald http_jar in rules_java doesn't work with Bazel 6 as it loads cache.bzl, which isn't available in that Bazel version.

I'm aware of this (decided not to try to fix it since it's too much work). Is it blocking you?

@fmeum
Copy link
Contributor Author

fmeum commented Nov 13, 2024

If you are aware of this it's fine, it just means that rules_java can't use it yet.

hvadehra
hvadehra previously approved these changes Nov 14, 2024
@hvadehra
Copy link
Member

@fmeum This is causing a few issues on import, could you please change the newly added BUILD files to BUILD.bazel?

@fmeum
Copy link
Contributor Author

fmeum commented Nov 15, 2024

@hvadehra Done

@fmeum fmeum deleted the runfiles branch November 15, 2024 22:16
@fmeum
Copy link
Contributor Author

fmeum commented Nov 15, 2024

@hvadehra Could you cut a new release so that this can be integrated into bazelbuild/bazel#24219?

@hvadehra
Copy link
Member

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.

5 participants