[#6664]: Collapse two bazel info invocations into one #6728
Conversation
github-actions
bot
added
product: CLion
mtoader
force-pushed
the
mtoader/collapse-bazel-info-invocations
branch
from
8e28621
to
15b0eed
Compare
| return blazeInfoStream.readAllBytes(); | ||
| } | ||
| }); | ||
| .submit( |
There was a problem hiding this comment.
where does this formatting change come from?
There was a problem hiding this comment.
I can't figure out how to set up the ide formatter to match what the project has in all details. Some things slip.
There was a problem hiding this comment.
do you have https://github.com/google/google-java-format ?
There was a problem hiding this comment.
That's where it was :) Let me use it.
There was a problem hiding this comment.
rebased and reformatted
mtoader
force-pushed
the
mtoader/collapse-bazel-info-invocations
branch
from
15b0eed
to
ec55262
Compare
In PR bazelbuild#6711 i added two back to back bazel info calls. This PR collapsed those calls into one.
mtoader
force-pushed
the
mtoader/collapse-bazel-info-invocations
branch
from
ec55262
to
7df9d91
Compare
mtoader
changed the title
github-actions
bot
removed
the
awaiting-review
| @@ -114,7 +114,7 @@ private static String getOrThrow(ImmutableMap<String, String> map, String key) { | |||
|
|
|||
| abstract ImmutableMap<String, String> getBlazeInfoMap(); | |||
|
|
|||
| public String get(String key) { | |||
| protected String get(String key) { | |||
There was a problem hiding this comment.
We were using this method in our extension to load the java-home from bazel info. Was there a reson to restrict its visibility?
There was a problem hiding this comment.
Yes. I made accessors for individual values and restricted the set of values pulled from bazel info when i merged the two calls to it generated by the fact that i needed to pull info about startlark state (to see if bzlmod and other similar flags are set or not).
I also switched the callers of that to use the accessors. Not sure how i missed yours. What is the context of that miss ?
There was a problem hiding this comment.
This PR also moved to a white list of things pulled from bazel info. I'm pretty sure java-home was not there since it was not used in the plugin codebase. I can add that and make an accessor for it. Would that help you?
There was a problem hiding this comment.
Its a internal plugin we have on top of bazel, so no easy to see it was being used :)
I'll let you know, Right not at least its only a compile error and should not be affecting runtime. I'm seeing whether we can use the projects SDK for this usecase instead of using this from bazelInfo.
There was a problem hiding this comment.
Went ahead and created a PR to add java-home to the info cache: #6810
Our use case does require to use the same java that bazel uses. Thanks for the explanation, I was able to understand the changes better.
Checklist
Discussion thread for this change
Issue number: #6664
Description of this change
In PR #6711 I added two back to back bazel info calls.
This PR collapsed those calls into one.
Also trying to remove an obsolete Service registration.