Proper resolution / completion of external workspaces / repositories in label definitions

[mtoader]

Description of the feature request:

As of current master, when looking at a label definition in a BUILD.xxx file (and especially inside bzlmod enabled workspaces) I would like to be able to complete/resolve the visible repository / external workspaces. This doesn't really work as of now

Which category does this issue belong to?

Intellij

What underlying problem are you trying to solve with this feature?

No response

What operating system, Intellij IDE and programming languages are you using? Please provide specific versions.

Current plugin HEAD on mac

Have you found anything relevant by searching the web?

There are some issues related to more general bzl mod support but they seem to have been pushed in the future since internally google didn't adopt this at scale.

• bzlmod: Starlark navigation for external workspaces doesn't work  #6590 is another recent instance of this.

Any other information, logs, or outputs that you want to share?

I'm open to work on some of these if acceptable.

[mtoader]

To add more context on my thinking here:

I want to call into bazel/blaze mod dump_repo_mapping on sync, store that in a similar way as one of the BlazeProjectData parts and use it inside an "fixed" ExternalWorkspaceFragment reference to provide ergonomic completion and resolution into <executionRoot>/external/workspace name

One open question I have is: should I try to do this using the async mode or the aspect mode?

[mai93]

Thanks @mtoader, I think we can accept contributions addressing this issue cc @tpasternak
can you clarify what do you mean by async mode vs aspect mode?

[mtoader]

intellij/base/src/com/google/idea/blaze/base/model/BlazeProjectData.java

Line 29 in 2020a23

/** Interface to persistent project data. */

is kind of basis of a lot of resolving in the current plugin. It has two implementations:

• intellij/base/src/com/google/idea/blaze/base/model/AspectSyncProjectData.java

  Line 48 in 2020a23

  public final class AspectSyncProjectData implements BlazeProjectData {

  which is built using bazel aspects by running build that will output both the results and the ide data needed to build the project model

• intellij/base/src/com/google/idea/blaze/base/qsync/QuerySyncProjectData.java

  Line 37 in 2020a23

  import org.jetbrains.annotations.Nullable;

  which seems to try to do the same thing by using mostly bazel query info

However, the latter is off (the experiment flag is false as the default), as it only seems enabled within Google. It is also incomplete in the current master (see below), which makes me vary about implementing things within it.

@Override
  public TargetMap getTargetMap() {
    throw new NotSupportedWithQuerySyncException("getTargetMap");
  }

  @Override
  public BlazeInfo getBlazeInfo() {
    throw new NotSupportedWithQuerySyncException("getBlazeInfo");
  }
 // ...
  @Override
  public ArtifactLocationDecoder getArtifactLocationDecoder() {
    throw new NotSupportedWithQuerySyncException("getArtifactLocationDecoder");
  }

  @Override
  public RemoteOutputArtifacts getRemoteOutputs() {
    throw new NotSupportedWithQuerySyncException("getRemoteOutputs");
  }

  @Override
  public ExternalWorkspaceData getExternalWorkspaceData() {
    throw new NotSupportedWithQuerySyncException("getExternalWorkspaceData");
  }

  @Override
  public SyncState getSyncState() {
    throw new NotSupportedWithQuerySyncException("getSyncState");
  }

I'm not sure what is the rollout/completion plan of said functionality since if I want to plumb some metadata into BlazeProjectData (as I would have to) it would imply touching this too in some way.

[tpasternak]

So the qsync module has been introduced some time ago by Google, however at the moment it works well only with Android Studio, as we didn't have capacity to provide the proper support for it. This means, you can skip the qsync part for now

[tpasternak]

Also, the issue occurs for bzlmod imports. Legacy imports still work well. This comes from the fact that bzlmod stores external repositories in directories, which names are not same as the repositories.

It just fails here, because it merges "external" stem with the repository name, resulting in a directory that doesn't exist

intellij/base/src/com/google/idea/blaze/base/sync/workspace/WorkspaceHelper.java

Lines 213 to 219 in b67720a

	File externalDir = new File(getBlazeProjectData(project).getBlazeInfo().getOutputBase(),
	"external/" + workspaceName);
	if (externalDir.exists() || isInTestMode()) {
	root = new WorkspaceRoot(externalDir);
	workspaceRootCache.put(workspaceName, root);
	}

[mtoader]

When i checked that didn't do completion at all (neither in classical or bzlmod situatiosn). I recall solving completion when i was within Goog but i don't know what happened with that.

However in order to do it right in bzlmod world we need to use the output of bazel mod dump_repo_mapping workspace (which is what i'm gonna do).

[tpasternak]

oh, wait, you're right, I meant navigation, not completion

[mtoader]

As of now i have this big change here: master...mtoader:intellij:mtoader/support-module-based-repo-completion. It is still incomplete since I have to fix resolving and tweak the completion UX (fix the tab vs enter vs inside/outside quotes, etc behavior).

My q is: would the reviewers be ok with such a big change or should I make into more manageable pieces?

Note: this is fundamentally big because the ceremony is needed to hook into the sync machinery.

[tpasternak]

Managable pieces highly appreciated! Thank you for asking btw

[mtoader]

Posted the first PR in the series

[tpasternak]

Just loose thoughts for later:

1. we will for sure need a registry flag. I'm almost sure there will be some use case where it just crashes and we will need to provide immediate troubleshooting
2. Probably we need to handle bazel 5 (still supported)

[mtoader]

So far, I'm making this work only if bazel has mod dump_repo_mapping (aka bazel > 7.1.0). For the others, it will just behave as before.

[tpasternak]

I know we only have to ensure it doesn't crash

[mtoader]

The plugin? I'm trying to default the ExternalWorkspaceData to EMPTY in all computations. And that should only change if i can infer things from within bazel mod ... (which I gate with bazel version for now).

I could make that execution success optional and warn if it fails. I will see how hard that is.

If your bazel is > 7.1.0 and the blaze mod ... invocation fails, it will break sync. I had to add some workarounds, for example, in PR no: 3, to not have it crash for Clion (where there is a cpp-specific BlazeFlagsProvider that would add a flag unknown to mod).

[tpasternak]

It would be enough to just check the bazel version and skip the call if it's too low.

[mtoader]

It would be enough to just check the bazel version and skip the call if it's too low.

The last PR actually does that

[mtoader]

workspace works just as any other non-existent repo name: it will result in an error with --noenable_workspace (default in Bazel 8) and return a fallback only relevant with WORKSPACE otherwise. Please test with the flag and you should also see these "implicit" repos go away.

I assumed that running Bazel on demand as an external repo is accessed would be at odds with the plugins sync approach, but if it works well enough, I would recommend that approach. But if that doesn't work out, we can certainly add the recursive lookup to Bazel.

Running it on demand is making things more complex in a sense (from the IDE / resolving POV), but one doesn't usually edit imported modules (which is the main case for completion). Resolving otoh would need that info but it can be a on demand cost paid once you try to do it first time.

It would be way better if i could take once from a cmd output :)

[mtoader]

@tpasternak I have a q. how are changes from this repo "migrated" to https://github.com/JetBrains/hirschgarten ?

[tpasternak]

Usually they are not, hirschgarten is a full rewrite

[mtoader]

Usually they are not, hirschgarten is a full rewrite

So anything i would want there would have to be redone there? I like clean rewrites :)

[tpasternak]

Cc @jastice

[tpasternak]

And cc @ujohnny

[mtoader]

I see no real completion in there as of now (neither respecting bzlmod info nor just basic local repository) so I assume this is not yet there.

[jastice]

HI @mtoader! Thanks for your contributions to this plugin, they're appreciated. But to give you a bit of context, we are spending over 90% of our efforts on the new plugin in the hirschgarten repo, so mid-term it might be more productive to contribute there. We're happy to work with you to address any feature gaps to help migrate to the new plugin.

[mtoader]

HI @mtoader! Thanks for your contributions to this plugin, they're appreciated. But to give you a bit of context, we are spending over 90% of our efforts on the new plugin in the hirschgarten repo, so mid-term it might be more productive to contribute there. We're happy to work with you to address any feature gaps to help migrate to the new plugin.

For now, I'm happy to complete the work I want here (moving the ideas there would be easier afterward, and I would have to learn Kotlin to be effective).

This was more of a generic question: What is the current progress state? For example, is any completion work being done? Reference searches, etc, yet? (i couldn't tell from the current beta)

[mpiekutowski]

Hi @mtoader! Currently we have completion/resolution for local references and those from load statements in the main repository. At the moment I am working on completion for labels and keyword arguments. We are also working on adding these features for external workspaces. So yes, completion work is being done right now :)