Reload project view, and make sure that BazelImportSettings respect p…

[dkashyn-sfdc]

…roject view choice before performing sync calls.

Checklist

• I have filed an issue about this change and discussed potential changes with the maintainers.
• I have received the approval from the maintainers to make this change.
• This is not a stylistic, refactoring, or cleanup change.

Please note that the maintainers will not be reviewing this change until all checkboxes are ticked. See
the Contributions section in the README for more
details.

Discussion thread for this change

Issue number: 7087

Description of this change

[dkashyn-sfdc]

Last 2 runs after git pull and it is obvious that our project will benefit from QS.

However, there are some gaps so "toggle" mechanism is essential.

[ujohnny]

I've been also looking for a way to jump between qs/as on the fly, thanks for submitting this PR! Can we add some javadoc here to reflect somthing like returns the project type with project view file lookup? and maybe make the name more matching the action it does rather than a point it code where it should be called?

[dkashyn-sfdc]

There are 15 places where Blaze.getProjectType*(project) == ProjectType.QUERY_SYNC check is performed. So I wanted to have a special case for sync calls only. This is why name reflects this.

I don't really know how frequent the rest of places are hitting this method and saw it called from action apperance update, which cannot afford to reload file for sure.

So instead of having "get type with refresh" I'd rather keep "get type for sync and make sure the proper sync mode is used". Also we are updating BazelImportSettings here and it is ok to update those on sync but for the rest of ops I'd like to avoid this.

[dkashyn-sfdc]

I can rename it to getSelectedProjectTypeBeforeSync or so. The fact that BazelImportSetings are updated is important, yet subtle here. Before, this change it was set once during project creation, and not touched afterwards, for whatever reason.

Overall, Blaze.getProjectType*(project) == ProjectType.QUERY_SYNC is someway out of control... It is peppered over the code quite randomly, and I'm a little bit worried about it causing trouble in the future even for cases beyond sync.

[dkashyn-sfdc]

getRefreshedProjectType or getNotCachedProjectType is another option... But then it will be a dilemma where to call those :)

[dkashyn-sfdc]

Probably we need to call save here for the old model... But this is a thing only for Aspect one.

[dkashyn-sfdc]

With 50% of non-test impls not using saveProject this interface definitely needs some cleanup...

// TODO(mathewi) Tidy up the interface to remove this unnecessary stuff.

[tpasternak]

So when I switch from legacy sync to aspect sync, QS is able to detect it's the first sync and falls to non-incremental sync. It happens here

intellij/base/src/com/google/idea/blaze/base/sync/actions/IncrementalSyncProjectAction.java

Lines 54 to 57 in 5ad3c3d

	if (!qsm.isProjectLoaded()) {
	qsm.onStartup(scope);
	} else {
	qsm.deltaSync(scope, TaskOrigin.USER_ACTION);

---

On the other hand when I switch back from QS to AS, it results in Incremental Sync which might cause some unexpected issues (For example leftoveers after QS in project model)

[dkashyn-sfdc]

  private static boolean allTargetsBuild(SyncMode mode) {
    return mode == SyncMode.FULL || mode == SyncMode.INCREMENTAL;
  }

With

    } else {
      BlazeSyncManager.getInstance(project)
          .incrementalProjectSync(/* reason= */ "IncrementalSyncProjectAction");
    }

  public void incrementalProjectSync(String reason) {
    BlazeSyncParams syncParams =
        BlazeSyncParams.builder()
            .setTitle("Sync")
            .setSyncMode(SyncMode.INCREMENTAL)
            .setSyncOrigin(reason)
            .setAddProjectViewTargets(true)
            .setAddWorkingSet(BlazeUserSettings.getInstance().getExpandSyncToWorkingSet())
            .build();
    requestProjectSync(syncParams);
  }

And "Full" is:

  public void fullProjectSync(String reason) {
    BlazeSyncParams syncParams =
        BlazeSyncParams.builder()
            .setTitle("Full Sync")
            .setSyncMode(SyncMode.FULL)
            .setSyncOrigin(reason)
            .setAddProjectViewTargets(true)
            .setAddWorkingSet(BlazeUserSettings.getInstance().getExpandSyncToWorkingSet())
            .build();
    requestProjectSync(syncParams);
  }

[dkashyn-sfdc]

TBH I've been using "INCREMENTAL" all this time and there is nothing really incremental about it :)

[tpasternak]

well, this means INCREMENTAL was pretty good, theoretically it should always work

[tpasternak]

but I would fall back to FULL, for sure external repo completions in starlark won't work

[dkashyn-sfdc]

After checking the code I'm not that sure about

QS is able to detect it's the first sync and falls to non-incremental sync

since nothing resets com.google.idea.blaze.base.qsync.QuerySyncManager#loadedProject for QS to do FULL or onStartup

[tpasternak]

well, here it ends up with calling onStartup

 if (!qsm.isProjectLoaded()) { 
   qsm.onStartup(scope); 
 } else { 
   qsm.deltaSync(scope, TaskOrigin.USER_ACTION); 

it just seems safer to me