Skip to content
This repository has been archived by the owner on Aug 5, 2024. It is now read-only.

Support Bazel 6.0.0 #318

Merged
merged 3 commits into from
Jan 2, 2023
Merged

Conversation

tpasternak
Copy link
Contributor

@tpasternak tpasternak commented Dec 28, 2022

  1. Main Repository Target Label Prefixes.
    In Bazel 5 and older, the target labels were strigified to "//"-prefixed names. Since
    6.0.0, they are stringified to "@//"-prefixed names. Unfortunately, this does not apply
    to data coming from BEP. Targets references in BEP events payload are "//"-prefixed.
    For this reason we need to add "@" manually after retrieving data from BEP.

  2. aspects.bzl: Retrieve runtime toolchain in the new way (Bazel 6.0.0)

  3. cpp-project: Old GTest version is incompatible with Bazel 6.0.0, the
    new one required cpp version bump (--std=c++17)

  4. e2e tests are now adapted to run across multiple Bazel versions (currently 5 and 6)

  5. JdkResolver: JDK version was inferred from source version, which is not always true. That's
    why it is safer just to pass null.

@tpasternak tpasternak force-pushed the support-bazel-6 branch 19 times, most recently from fbb1c7b to 8088e91 Compare December 29, 2022 15:22
1. Main Repository Target Label Prefixes.
In Bazel 5 and older, the target labels were strigified to "//"-prefixed names. Since
6.0.0, they are stringified to "@//"-prefixed names. Unfortunately, this does not apply
to data coming from BEP. Targets references in BEP events payload are "//"-prefixed.
For this reason we need to add "@" manually after retrieving data from BEP.

2. aspects.bzl: Retrieve runtime toolchain in the new way (Bazel 6.0.0)

3. cpp-project: Old GTest version is incompatible with Bazel 6.0.0, the
new one required cpp version bump (--std=c++17)

4. e2e tests are now adapted to run across multiple Bazel versions (currently 5 and 6)

5. JdkResolver: JDK version was inferred from source version, which is not always true. That's
why it is safer just to pass null.
@tpasternak tpasternak force-pushed the support-bazel-6 branch 2 times, most recently from a049af4 to 1da7dc2 Compare December 29, 2022 15:35
@tpasternak tpasternak marked this pull request as ready for review December 29, 2022 15:37
@tpasternak tpasternak requested a review from abrams27 December 29, 2022 15:37
@jastice jastice self-requested a review January 2, 2023 10:05
Copy link
Member

@abrams27 abrams27 left a comment

Choose a reason for hiding this comment

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

i feel that we can do it without bazel version - the bazel version is used only in 3 places:

  • isWorkspaceTarget - we can just check does the id starts with // or @//
  • similar for extracting the relative path

but then we could do it without passing BazelInfo around, what d u think about it?

Tomasz Pasternak added 2 commits January 2, 2023 12:06
It was based on "source" version of the jdk, which was not always equal
to the JDK version. Now, we always call "java -version" to retrieve the
version number
@tpasternak tpasternak requested a review from abrams27 January 2, 2023 12:05
@tpasternak
Copy link
Contributor Author

i feel that we can do it without bazel version - the bazel version is used only in 3 places:

isWorkspaceTarget - we can just check does the id starts with // or @//
similar for extracting the relative path
but then we could do it without passing BazelInfo around, what d u think about it?

@abrams27 It would be good and less code indeed, but when I've been working on it, I considered // form as a legacy one, which we want to support only as long as we are going to support Bazel 5. Once we drop support 5, we may extract this code and use @// everywhere directly, without version checking

@tpasternak tpasternak merged commit f5bd4c4 into JetBrains:master Jan 2, 2023
@tpasternak tpasternak deleted the support-bazel-6 branch January 2, 2023 13:44
@tpasternak tpasternak restored the support-bazel-6 branch March 3, 2023 13:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants