New Sync View #6813

LeFrosch · 2024-09-30T07:06:44Z

Adds the option to use the platform sync view instead of the bazel custom one. Errors are also collected using BEP in the new view to improve the quality of reported issues.

Registry key: bazel.new.sync.view

Uses the platform sync view to display progress. However, progress no longer follows a tree structure.

The platform sync view offers sufficient space for BEP's detailed error messages.

LeFrosch · 2024-10-02T09:00:08Z

Rebased master

mai93 · 2024-10-07T17:35:09Z

why is this better than the existing way? is the plan to make it the default?

LeFrosch · 2024-10-08T07:59:20Z

@mai93 The Idea was to show error messages received by BEP rather than parsing the command line output to increase the quality and relevance of reported errors. In my option, the sync and build view provided by the platform was better suited for displaying these longer messages. Furthermore, I hope that using the platform sync view can save us some maintenance work in the long run, since it is maintained by the platform team.

With regard to making this view the default, this depends, of course, on the feedback we receive. If it is well perceived, I see no reason for not doing so. But maybe it makes sense to make it opt-in in the beginning.

mai93 · 2024-10-08T22:37:25Z

Thanks for the explanation! I'd propose guarding it with an experiment instead and enabling it incrementally because we have a lot of features added for specific users behind a key and we never got a sense if they are ready for general rollout.

tpasternak · 2024-10-09T04:01:49Z

Btw please hold on, does it work with query sync?

LeFrosch · 2024-10-09T09:55:06Z

@tpasternak I am not sure how the sync UI for query sync works but the new sync view is just a scope that can be added to the BlazeContext and processes IssueOutputs. If the view works well for regular sync I was planning to also adopt it for running builds and maybe we can also use it for query sync.

tpasternak · 2024-10-09T11:55:40Z

Ok, you can just add use_query_sync: true to the projectview file to test

ujohnny · 2024-10-14T08:56:49Z

base/src/com/google/idea/blaze/base/async/executor/ProgressIndicatorStub.kt

+import com.intellij.openapi.progress.TaskInfo
+import com.intellij.openapi.wm.ex.ProgressIndicatorEx
+
+object ProgressIndicatorStub : ProgressIndicatorEx {


can we use EmptyProgressIndicator instead of this one?

ujohnny · 2024-10-14T09:01:26Z

base/src/com/google/idea/blaze/base/buildview/BuildViewMigration.kt

+
+object BuildViewMigration {
+  @JvmStatic
+  val enabled get(): Boolean = Registry.`is`("bazel.new.sync.view")


val enabled get(): Boolean = Registry.`is`("bazel.new.sync.view")

ujohnny · 2024-10-14T09:03:02Z

base/src/com/google/idea/blaze/base/sync/BlazeSyncManager.java

                      .submitTask(
                          indicator ->
                              Scope.root(
                                  context -> {
+                                    if (Registry.is("bazel.new.sync.view")) {


let's use com.google.idea.blaze.base.buildview.BuildViewMigration#getEnabled here

ujohnny · 2024-10-14T09:09:07Z

base/src/com/google/idea/blaze/base/buildview/events/BuildEventParser.kt

+import com.intellij.openapi.diagnostic.Logger
+import com.intellij.openapi.extensions.ExtensionPointName
+
+internal val LOG = Logger.getInstance(BuildEventParser::class.java)


internal val LOG = logger<BuildEventParser>()

ujohnny · 2024-10-14T09:40:54Z

base/src/com/google/idea/blaze/base/buildview/BazelService.kt

+      val exitCode = execute(ctx, cmdBuilder.build())
+      val result = BuildResult.fromExitCode(exitCode)
+
+      parseJob.cancelAndJoin()


this would cancel all the parsing because it can trigger a CancellationException from parseEvent

As far as I understand this can only trigger a CancellationException in parseEvent if parseEvent is waiting for more data. And in this case we actually want to cancel parseEvent because there is not going to be more data after the bazel process finished. The actual parsing cannot be cancelled as far as I can tell.

ujohnny · 2024-10-14T09:45:05Z

base/src/com/google/idea/blaze/base/buildview/ContextExt.kt

+  name: String = "BazelContext",
+  block: suspend CoroutineScope.() -> T,
+): T {
+  val deferred = scope.async(Dispatchers.IO + CoroutineName(name)) { block() }


There's no need in IO use right here

ujohnny · 2024-10-14T09:57:04Z

base/src/com/google/idea/blaze/base/sync/BuildPhaseSyncTask.java

-                    var newScope = new ToolWindowScope.Builder(project, task)
-                        .setProgressIndicator(indicator)
-                        .setPopupBehavior(BlazeUserSettings.FocusBehavior.ON_ERROR)
-                        .setIssueParsers(targetDetectionQueryParsers(project, WorkspaceRoot.fromProject(project)))


could you please double check this line should be removed?

ujohnny

looks good, let's add got it promotion as a separate PR

# Conflicts: # base/src/com/google/idea/blaze/base/wizard2/ui/BlazeEditProjectViewControl.java

# Conflicts: # base/src/com/google/idea/blaze/base/sync/SyncProjectTargetsHelper.java

# Conflicts: # base/src/com/google/idea/blaze/base/sync/aspects/BlazeIdeInterfaceAspectsImpl.java

SergejSalnikov · 2024-11-14T22:48:32Z

LeFrosch, could you please explain how to use the now final BuildResultHelper class with the remote builds? Yes, the BuildInvoker.createBuildResultHelper() method can be overridden, but it only can return an instance of final BuildResultHelper class that is cannot be customized.

LeFrosch · 2024-11-15T08:40:00Z

@SergejSalnikov If I remember correctly I just inlined the only implementation of the BuildResultHelper. I figured that the other implementation must have been google internal. Are you maintaining a fork?

LeFrosch force-pushed the updated-sync-view branch 3 times, most recently from 7732e35 to 2072ae8 Compare October 2, 2024 08:59

LeFrosch force-pushed the updated-sync-view branch 4 times, most recently from 1717140 to 74f63c4 Compare October 7, 2024 13:44

LeFrosch marked this pull request as ready for review October 7, 2024 14:00

LeFrosch requested review from mai93, tpasternak and agluszak as code owners October 7, 2024 14:00

github-actions bot added product: CLion CLion plugin product: IntelliJ IntelliJ plugin product: GoLand GoLand plugin awaiting-review Awaiting review from Bazel team on PRs labels Oct 7, 2024

github-actions bot assigned mai93, tpasternak and ujohnny Oct 7, 2024

ujohnny reviewed Oct 14, 2024

View reviewed changes

ujohnny requested changes Oct 14, 2024

View reviewed changes

LeFrosch force-pushed the updated-sync-view branch 3 times, most recently from 51586a2 to eec4260 Compare October 15, 2024 07:53

ujohnny approved these changes Oct 15, 2024

View reviewed changes

LeFrosch added 19 commits October 18, 2024 10:53

removed unused consumed bytes from build event stream

77781b0

Rename .java to .kt

49a5792

refactor for IssueOutput to extend BuildIssue

e3145e8

# Conflicts: # base/src/com/google/idea/blaze/base/wizard2/ui/BlazeEditProjectViewControl.java

added build view

87eefd9

# Conflicts: # base/src/com/google/idea/blaze/base/sync/SyncProjectTargetsHelper.java

added event parser

f82eda7

Rename .java to .kt

7bf8b66

added bazel build service

81848a0

# Conflicts: # base/src/com/google/idea/blaze/base/sync/aspects/BlazeIdeInterfaceAspectsImpl.java

fixed clwb tests

87a3ea2

fixed ijwb tests

6a0e1bd

improved backwards compatability

1c147ac

added progress indicator

7a19708

fixed invalid protocol buffer exception due to end of input

650e376

fixes kotlin configuration deadlock

f264562

improved event parsing

ff26aec

fixed log scope

27c2877

replaced SupervisorJob with try catch

590a382

use project scope for navigation

2611aef

addressed review comments

d8c2ad4

fix kotlin build issue

dff0223

LeFrosch force-pushed the updated-sync-view branch from eec4260 to dff0223 Compare October 18, 2024 08:57

LeFrosch merged commit c1a3c5a into bazelbuild:master Oct 18, 2024
6 checks passed

LeFrosch deleted the updated-sync-view branch October 18, 2024 09:48

github-actions bot removed the awaiting-review Awaiting review from Bazel team on PRs label Oct 18, 2024

mai93 mentioned this pull request Oct 22, 2024

Update changelog for v2024.10.22 release #6920

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Sync View #6813

New Sync View #6813

LeFrosch commented Sep 30, 2024 •

edited

Loading

LeFrosch commented Oct 2, 2024

mai93 commented Oct 7, 2024

LeFrosch commented Oct 8, 2024

mai93 commented Oct 8, 2024

tpasternak commented Oct 9, 2024

LeFrosch commented Oct 9, 2024

tpasternak commented Oct 9, 2024

ujohnny Oct 14, 2024

ujohnny Oct 14, 2024

ujohnny Oct 14, 2024

ujohnny Oct 14, 2024

ujohnny Oct 14, 2024

LeFrosch Oct 15, 2024

ujohnny Oct 14, 2024

ujohnny Oct 14, 2024

ujohnny left a comment

SergejSalnikov commented Nov 14, 2024 •

edited

Loading

LeFrosch commented Nov 15, 2024

New Sync View #6813

New Sync View #6813

Conversation

LeFrosch commented Sep 30, 2024 • edited Loading

LeFrosch commented Oct 2, 2024

mai93 commented Oct 7, 2024

LeFrosch commented Oct 8, 2024

mai93 commented Oct 8, 2024

tpasternak commented Oct 9, 2024

LeFrosch commented Oct 9, 2024

tpasternak commented Oct 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ujohnny left a comment

Choose a reason for hiding this comment

SergejSalnikov commented Nov 14, 2024 • edited Loading

LeFrosch commented Nov 15, 2024

LeFrosch commented Sep 30, 2024 •

edited

Loading

SergejSalnikov commented Nov 14, 2024 •

edited

Loading