intellij icon indicating copy to clipboard operation
intellij copied to clipboard

Run gazelle on project sync

Open blorente opened this issue 2 years ago • 8 comments

Problem

For projects that use gazelle, moving a single file is a two-step process: First they must run gazelle, then they must sync to trigger re-indexing if IntelliJ didn't pick those changes up.

Ideally, this would be a single-click action, whereby a user only has to click "sync" after a refactoring is done.

Proposed Solution

Run Gazelle on sync, at least for full, non-incremental syncs of projects, before we run the bazel build that enables the actual sync.

Regarding performance, Gazelle accepts packages as arguments, so I believe it should be possible to inspect the .bazelproject file to gather which packages it should sync, therefore correlating performance to the size of the indexed project.

The changes I imagine would be needed:

  • Additional entries in the Bazel Plugin preferences, where the user can specify:
    • The label of the Gazelle target to run.
    • The frequency of the gazelle runs (on every sync, or only on full syncs).
  • Custom logic to derive go packages from the directories entry in the .bazelproject.
  • A call to gazelle during project sync, which would run before even the initial query.

I'm happy to spend the effort of implementing this, but as per the contributing guidelines I thought I'd ask first if this would be an interesting contribution, and if I missed important pieces.

blorente avatar Jan 27 '22 11:01 blorente

Hello, folks! I've been experimenting with different implementations for this issue and I think some design input would be great at this stage.

The problem: I'd like to run a (potentially multi-second) process (that happens to be a call to bazel, but doesn't have to be) that can change BUILD files, on every sync. As far as I know:

  • This can create new targets, so it needs to run before the target map is constructed, so before any of the BlazeSyncPlugins have had time to run.
  • It needs a ProjectViewSet (to read the configurable command to run and the directories it should run on).
  • It would be a side-effecting operation, so it shouldn't be a part of the VCS + BazelInfo preparation phase. I think it should come after that, but there's no reason it couldn't come before.

In the existing codebase, the places where it could be implemented are:

  • As a BlazeSyncPlugin. This gives us language constraints (this command should only run on go projects) and a nice way to hook onto ProjectViewSet parsing, but no ordering. Plugins don't have a "before sync starts" hook. It could be implemented, but I wouldn't want to delve into that before confirmation that it's actually a good idea.
  • As a SyncListener. This gives us a hook that runs at the right time (onSyncStart), but doesn't give us easy access to add additional configuration to ProjectViewSet. Additionally, that hook looks like it really wants to be quick, non-blocking, and side-effect free, which this command would definitely not be.
  • As a separate step in ProjectUpdateSyncTask. This would run before any other syncs, possibly exactly after the onSyncStart hooks for listeners are called.
  • As its own separate *SyncTask. Same as above, but updating SyncPhaseCoordinator instead.

I'd appreciate any input on the matter.

blorente avatar Feb 01 '22 11:02 blorente

I'm very much interested in the Gazelle use case as well. IMO the option with SyncListener and having it configured in ProjectViewSet seems like the most clean solution for me. To @blorente points:

  1. We can get ProjectViewSet in the listener via ProjectViewManager.
  2. SyncListener doesn't seem that is designed to be very fast and without side effects. Some listeners do have side effect after the sync. Also there is access to a BlazeContext in onSyncStart so the hooks can be scoped out and execution logs "output"ed.

Gazelle can support not only Go so having it as a "hook" section in ProjectViewSet seems like a good idea. It can be as simple as something like:

# .bazelproject
before_sync_command: run //:gazelle -- update

fkorotkov avatar Feb 04 '22 14:02 fkorotkov

I understand that Gazelle might be a helpful tool for some users, but it's not a standard tool in Bazel-based workflows. At the moment, it would be too early to include Gazelle-specific code and settings as part of the regular Bazel plugin. In addition, I would rather like to see the Bazel plugin move into a direction in which BUILD files are automatically updated upon code refactorings for straight-forward operations (possibly provided by an external tool) and afterwards automatically synced. The AutoSync functionality could already cover the latter to some extend, but it likely requires further polish.

If you want to go forward with a Gazelle integration, I recommend to go for a plugin on top of the Bazel plugin. In such a plugin, you could use the SyncListener extension point and thus hook up the Gazelle execution. As Fedor wrote, you might want to look into using the BlazeContext passed to onSyncStart to inform users about this additional execution as otherwise they might wonder why the Bazel sync is so slow to start.

In a custom plugin, you can also add any settings you want, including the exact Gazelle command you want to run.

alice-ks avatar Feb 09 '22 15:02 alice-ks

@alice-ks Given that Gazelle is a standard tool for Golang Bazel-based workflows (everywhere outside of Google that is), what are your thoughts on integrating Gazelle into Go-based workflows for the Bazel plugin? For example, the Bazel plugin could listen to whenever the import block of a go source file has been modified, and when that occurs run Gazelle on the package where the change took place.

blico avatar Feb 09 '22 20:02 blico

Thanks for answering, @alice-ks ! I understand if this extension is not desired. Do you know if there are plans to implement those automatic regenerations upon refactoring? Is it open to contributions?

@blico While I like the idea of being able to include it in the plugin, I'm not sure of the performance implications of running Gazelle as it exists today on every such operation. As far as I understand it, running Gazelle through Bazel is recommended, and that will incur some amount of fixed startup overhead on every call. The nice thing about doing it on sync is that a penalty of roughly one second is expected, whereas on a refactor it may be too clunky. So I'd prefer to wait until Gazelle could operate as a daemon, if that makes sense.

Plus, we'd have to run gazelle for every other language that supports it, and find a way to unify that. It's doable (the plugin already does it with per-language aspects and package recognition, it only runs bazel build once), but it feels like we'd be merging a global concern ("I want to regenerate BUILD files for this project") into a language-specific piece of code.

blorente avatar Feb 10 '22 10:02 blorente

not a standard tool in Bazel-based workflows.

This is true for languages where a robust Gazelle plugin isn't available yet. However in Go, where it works best, every single Bazel user I know of relies on Gazelle. The python extension is relatively new, and work is going on now for Java, Scala, and TypeScript at least. So my guess is that in a year, Gazelle will be a standard tool in most Bazel workflows.

At the moment, it would be too early to include Gazelle-specific code and settings as part of the regular Bazel plugin.

Yeah I think this is the right call, to incubate the features in a standalone plugin and iterate based on user feedback, but if/when it becomes a standard tool, users will ask for the features to graduate to be part of the standard plugin from Jetbrains.

alexeagle avatar Mar 30 '22 15:03 alexeagle

+1 for GoLand to have gazelle support. Gazelle is standard at Uber and we're a majority golang shop these days.

edwardotis avatar Jun 01 '22 19:06 edwardotis

Re-opening, as per the discussion during the community sync.

blorente avatar Sep 09 '22 14:09 blorente