-
Notifications
You must be signed in to change notification settings - Fork 305
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
[#6664]: Invoke blaze mod
correctly
#6729
Changes from 1 commit
23e08ae
7c6e25a
22b9a42
63e3e66
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,15 +38,32 @@ | |
|
||
public class BlazeModRunnerImpl extends BlazeModRunner { | ||
|
||
private static final String DUMP_REPO_MAPPING = "dump_repo_mapping"; | ||
private static final String ROOT_WORKSPACE = ""; | ||
|
||
/** | ||
* {@code bazel mod dump_repo_mapping} takes a canonical repository name and will dump a map | ||
* from repoName -> canonicalName of all the external repositories available to that repository | ||
* The name {@code ""} is special and considered to be <em>the main workspace</em> so in order to dump the main | ||
* repository map we would invoke it like {@code bazel mod dump_repo_mapping ""}. | ||
* <p /> | ||
* Additionally the flag {@code --enable_workspace} needs to be off for this to work. The flag is default | ||
* off in bazel 8.0.0 but it is on between 7.1.0 and 8.0.0. So we need to also pass this along in | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this flag supported by Bazel <7.1.0? If not, doesn't it break Bazel 6 and 5? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will not get called at all before 7.1.0 since the dump command itself didn't exist before. |
||
* between those versions for the command to work well. | ||
*/ | ||
@Override | ||
public ListenableFuture<ExternalWorkspaceData> dumpRepoMapping( | ||
Project project, | ||
BuildSystem.BuildInvoker invoker, | ||
BlazeContext context, | ||
BuildSystemName buildSystemName, | ||
List<String> flags) { | ||
|
||
// TODO: when 8.0.0 is released add this only if it's disabled explicitly for the repo | ||
flags.add("--noenable_workspace"); | ||
|
||
return Futures.transform( | ||
runBlazeModGetBytes(project, invoker, context, ImmutableList.of( "dump_repo_mapping", "workspace"), flags), | ||
runBlazeModGetBytes(project, invoker, context, ImmutableList.of(DUMP_REPO_MAPPING, ROOT_WORKSPACE), flags), | ||
bytes -> { | ||
JsonObject json = JsonParser.parseString(new String(bytes, StandardCharsets.UTF_8).trim()).getAsJsonObject(); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
previously it was "workspace", not an empty string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the context there: #6664 (comment)
Could you summarize that in a code comment?