Skip to content

Commit

Permalink
Split ArtifactTracker interface
Browse files Browse the repository at this point in the history
into `ArtifactTracker` and `RenderJarArtifactTracker` interfaces.

Even though their implementations can share a common infrastructure they do
not need to share it. These interfaces are used by different callers and
thus making them separate makes it easier to understand the intended usage
of each method.

1. Do not rename `ArtifactTracker` to `DependencyArtifactTracker` to avoid
   too much noise in this change.

2. Do not split the implementation yet. A few more changes are needed before it
   can happen.

PiperOrigin-RevId: 575854279
  • Loading branch information
Googler authored and copybara-github committed Oct 26, 2023
1 parent f6a9844 commit 22fe29e
Show file tree
Hide file tree
Showing 11 changed files with 117 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@
import com.google.idea.blaze.base.ideinfo.TargetKey;
import com.google.idea.blaze.base.io.VirtualFileSystemProvider;
import com.google.idea.blaze.base.model.BlazeProjectData;
import com.google.idea.blaze.base.qsync.ArtifactTracker;
import com.google.idea.blaze.base.qsync.QuerySync;
import com.google.idea.blaze.base.qsync.QuerySyncManager;
import com.google.idea.blaze.base.qsync.RenderJarArtifactTracker;
import com.google.idea.blaze.base.settings.Blaze;
import com.google.idea.blaze.base.settings.BlazeImportSettings.ProjectType;
import com.google.idea.blaze.base.sync.BlazeSyncModificationTracker;
Expand Down Expand Up @@ -143,12 +143,12 @@ public VirtualFile findClass(String fqcn) {

if (Blaze.getProjectType(project).equals(ProjectType.QUERY_SYNC)) {
if (QuerySync.isComposeEnabled()) {
ArtifactTracker artifactTracker =
QuerySyncManager.getInstance(project).getArtifactTracker();
RenderJarArtifactTracker renderJarArtifactTracker =
QuerySyncManager.getInstance(project).getRenderJarArtifactTracker();
// TODO(b/283280194): Setup fqcn -> target and target -> Render jar mappings to avoid
// iterating over all render jars when trying to locate class for fqcn.
// TODO(b/284002836): Collect metrics on time taken to iterate over the jars
for (File renderJar : artifactTracker.getRenderJars()) {
for (File renderJar : renderJarArtifactTracker.getRenderJars()) {
VirtualFile renderResolveJarVf =
VirtualFileSystemProvider.getInstance().getSystem().findFileByIoFile(renderJar);
if (renderResolveJarVf != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,12 @@
import com.google.idea.blaze.base.ideinfo.TargetKey;
import com.google.idea.blaze.base.ideinfo.TargetMap;
import com.google.idea.blaze.base.model.BlazeProjectData;
import com.google.idea.blaze.base.qsync.QuerySync;
import com.google.idea.blaze.base.qsync.ArtifactTracker;
import com.google.idea.blaze.base.qsync.QuerySyncManager;
import com.google.idea.blaze.base.settings.Blaze;
import com.google.idea.blaze.base.settings.BlazeImportSettings.ProjectType;
import com.google.idea.blaze.base.qsync.RenderJarArtifactTracker;
import com.google.idea.blaze.base.sync.data.BlazeProjectDataManager;
import com.google.idea.blaze.base.sync.workspace.ArtifactLocationDecoder;
import com.google.idea.blaze.base.targetmaps.TransitiveDependencyMap;
Expand Down Expand Up @@ -74,8 +76,9 @@ public List<File> getModuleExternalLibraries(Module module) {
// As Query Sync has a single workspace module but multiple resource modules
// (TODO(b/283282438): for setting up the resources). All render jars are mapped to the same
// workspace module
ArtifactTracker artifactTracker = QuerySyncManager.getInstance(project).getArtifactTracker();
return artifactTracker.getRenderJars();
RenderJarArtifactTracker renderJarArtifactTracker =
QuerySyncManager.getInstance(project).getRenderJarArtifactTracker();
return renderJarArtifactTracker.getRenderJars();
}

BlazeProjectData blazeProjectData =
Expand Down
27 changes: 2 additions & 25 deletions base/src/com/google/idea/blaze/base/qsync/ArtifactTracker.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
*/
package com.google.idea.blaze.base.qsync;

import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.idea.blaze.base.scope.BlazeContext;
import com.google.idea.blaze.common.Context;
Expand All @@ -25,7 +23,6 @@
import com.google.idea.blaze.qsync.cc.CcDependenciesInfo;
import com.google.idea.blaze.qsync.project.BuildGraphData;
import com.google.idea.blaze.qsync.project.ProjectProto;
import java.io.File;
import java.io.IOException;
import java.nio.file.Path;
import java.util.Optional;
Expand All @@ -38,12 +35,8 @@ public interface ArtifactTracker {
void clear() throws IOException;

/** Fetches, caches and sets up new artifacts. */
UpdateResult update(Set<Label> targets, OutputInfo outputInfo, BlazeContext context)
throws BuildException;

/** Fetches, caches and sets up new render jar artifacts. */
UpdateResult update(Set<Label> targets, RenderJarInfo renderJarInfo, BlazeContext context)
throws BuildException;
ArtifactTrackerUpdateResult update(
Set<Label> targets, OutputInfo outputInfo, BlazeContext context) throws BuildException;

/**
* Makes the project snapshot reflect the current state of tracked artifacts.
Expand Down Expand Up @@ -90,22 +83,6 @@ ProjectProto.Project updateProjectProto(
*/
CcDependenciesInfo getCcDependenciesInfo();

/** A data class representing the result of updating artifacts. */
@AutoValue
abstract class UpdateResult {
public abstract ImmutableSet<Path> updatedFiles();

public abstract ImmutableSet<String> removedKeys();

public static UpdateResult create(
ImmutableSet<Path> updatedFiles, ImmutableSet<String> removedKeys) {
return new AutoValue_ArtifactTracker_UpdateResult(updatedFiles, removedKeys);
}
}

/** Returns the list of render jars */
ImmutableList<File> getRenderJars();

/** Returns the count of .jar files. */
Integer getJarsCount();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright 2023 The Bazel Authors. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.google.idea.blaze.base.qsync;

import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableSet;
import java.nio.file.Path;

/**
* A result of updating artifact cache.
*
* <p>Instances of this class describe the difference between the state of the artifact cache before
* and after the operation. They do not represent the result of build action that produced these
* artifacts.
*
* <p><b>DO NOT USE</b> for purposes other than logging or displaying the result in the UI.
*/
@AutoValue
public abstract class ArtifactTrackerUpdateResult {
public abstract ImmutableSet<Path> updatedFiles();

public abstract ImmutableSet<String> removedKeys();

public static ArtifactTrackerUpdateResult create(
ImmutableSet<Path> updatedFiles, ImmutableSet<String> removedKeys) {
return new AutoValue_ArtifactTrackerUpdateResult(updatedFiles, removedKeys);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ public QuerySyncProject loadProject(BlazeContext context) throws IOException {
importSettings,
workspaceRoot,
artifactTracker,
artifactTracker,
dependencyTracker,
renderJarTracker,
projectQuerier,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ public ArtifactTracker getArtifactTracker() {
return loadedProject.getArtifactTracker();
}

public RenderJarArtifactTracker getRenderJarArtifactTracker() {
assertProjectLoaded();
return loadedProject.getRenderJarArtifactTracker();
}

public SourceToTargetMap getSourceToTargetMap() {
assertProjectLoaded();
return loadedProject.getSourceToTargetMap();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ public class QuerySyncProject {
private final BlazeImportSettings importSettings;
private final WorkspaceRoot workspaceRoot;
private final ArtifactTracker artifactTracker;
private final RenderJarArtifactTracker renderJarArtifactTracker;
private final DependencyTracker dependencyTracker;
private final RenderJarTracker renderJarTracker;
private final ProjectQuerier projectQuerier;
Expand All @@ -107,6 +108,7 @@ public QuerySyncProject(
BlazeImportSettings importSettings,
WorkspaceRoot workspaceRoot,
ArtifactTracker artifactTracker,
RenderJarArtifactTracker renderJarArtifactTracker,
DependencyTracker dependencyTracker,
RenderJarTracker renderJarTracker,
ProjectQuerier projectQuerier,
Expand All @@ -125,6 +127,7 @@ public QuerySyncProject(
this.importSettings = importSettings;
this.workspaceRoot = workspaceRoot;
this.artifactTracker = artifactTracker;
this.renderJarArtifactTracker = renderJarArtifactTracker;
this.dependencyTracker = dependencyTracker;
this.renderJarTracker = renderJarTracker;
this.projectQuerier = projectQuerier;
Expand Down Expand Up @@ -180,6 +183,10 @@ public ArtifactTracker getArtifactTracker() {
return artifactTracker;
}

public RenderJarArtifactTracker getRenderJarArtifactTracker() {
return renderJarArtifactTracker;
}

public SourceToTargetMap getSourceToTargetMap() {
return sourceToTargetMap;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright 2023 The Bazel Authors. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.google.idea.blaze.base.qsync;

import com.google.common.collect.ImmutableList;
import com.google.idea.blaze.base.scope.BlazeContext;
import com.google.idea.blaze.common.Label;
import com.google.idea.blaze.exception.BuildException;
import java.io.File;
import java.util.Set;

/** A local cache of built render jarss. */
public interface RenderJarArtifactTracker {

/** Fetches, caches and sets up new render jar artifacts. */
ArtifactTrackerUpdateResult update(
Set<Label> targets, RenderJarInfo renderJarInfo, BlazeContext context) throws BuildException;

/** Returns the list of render jars */
ImmutableList<File> getRenderJars();
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import com.google.common.base.Preconditions;
import com.google.idea.blaze.base.bazel.BazelExitCode;
import com.google.idea.blaze.base.logging.utils.querysync.BuildDepsStatsScope;
import com.google.idea.blaze.base.qsync.ArtifactTracker.UpdateResult;
import com.google.idea.blaze.base.scope.BlazeContext;
import com.google.idea.blaze.common.Label;
import com.google.idea.blaze.common.PrintOutput;
Expand All @@ -39,15 +38,15 @@ public class RenderJarTrackerImpl implements RenderJarTracker {

private final BlazeProject blazeProject;
private final RenderJarBuilder renderJarBuilder;
private final ArtifactTracker artifactTracker;
private final RenderJarArtifactTracker renderJarArtifactTracker;

public RenderJarTrackerImpl(
BlazeProject blazeProject,
RenderJarBuilder renderJarBuilder,
ArtifactTracker artifactTracker) {
RenderJarArtifactTracker renderJarArtifactTracker) {
this.blazeProject = blazeProject;
this.renderJarBuilder = renderJarBuilder;
this.artifactTracker = artifactTracker;
this.renderJarArtifactTracker = renderJarArtifactTracker;
}

/** Builds the render jars of the given files and adds then to the cache */
Expand Down Expand Up @@ -95,7 +94,8 @@ public void buildRenderJarForFile(BlazeContext context, List<Path> workspaceRela
context.output(PrintOutput.error("There were build errors when generating render jar."));
}

UpdateResult updateResult = artifactTracker.update(targets, renderJarInfo, context);
ArtifactTrackerUpdateResult updateResult =
renderJarArtifactTracker.update(targets, renderJarInfo, context);
if (updateResult.updatedFiles().isEmpty()) {
context.output(
PrintOutput.log(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,10 @@
import com.google.idea.blaze.base.logging.utils.querysync.BuildDepsStats;
import com.google.idea.blaze.base.logging.utils.querysync.BuildDepsStatsScope;
import com.google.idea.blaze.base.qsync.ArtifactTracker;
import com.google.idea.blaze.base.qsync.ArtifactTrackerUpdateResult;
import com.google.idea.blaze.base.qsync.OutputGroup;
import com.google.idea.blaze.base.qsync.OutputInfo;
import com.google.idea.blaze.base.qsync.RenderJarArtifactTracker;
import com.google.idea.blaze.base.qsync.RenderJarInfo;
import com.google.idea.blaze.base.qsync.cache.ArtifactFetcher.ArtifactDestination;
import com.google.idea.blaze.base.qsync.cache.FileCache.CacheLayout;
Expand Down Expand Up @@ -106,7 +108,7 @@
*
* <p>This class maps all the targets that have been built to their artifacts.
*/
public class ArtifactTrackerImpl implements ArtifactTracker {
public class ArtifactTrackerImpl implements ArtifactTracker, RenderJarArtifactTracker {

private static final BoolExperiment ATTACH_DEP_SRCJARS =
new BoolExperiment("querysync.attach.dep.srcjars", true);
Expand Down Expand Up @@ -377,7 +379,7 @@ private ImmutableMap<Path, Path> prepareFinalLayouts(

/** Fetches, caches and sets up new render jar artifacts. */
@Override
public UpdateResult update(
public ArtifactTrackerUpdateResult update(
Set<Label> targets, RenderJarInfo renderJarInfo, BlazeContext outerContext)
throws BuildException {
ImmutableMap<OutputArtifact, OutputArtifactDestinationAndLayout> artifactMap;
Expand All @@ -389,16 +391,16 @@ public UpdateResult update(
try (BlazeContext context = BlazeContext.create(outerContext)) {
ImmutableMap<Path, Path> updated = cache(context, artifactMap);
saveState();
return UpdateResult.create(updated.keySet(), ImmutableSet.of());
return ArtifactTrackerUpdateResult.create(updated.keySet(), ImmutableSet.of());
} catch (ExecutionException | IOException e) {
throw new BuildException(e);
}
}

/** Fetches, caches and sets up new artifacts. */
@Override
public UpdateResult update(Set<Label> targets, OutputInfo outputInfo, BlazeContext outerContext)
throws BuildException {
public ArtifactTrackerUpdateResult update(
Set<Label> targets, OutputInfo outputInfo, BlazeContext outerContext) throws BuildException {
ImmutableMap<OutputArtifact, OutputArtifactDestinationAndLayout> artifactMap;
try {
artifactMap = outputInfoToArtifactMap(outputInfo);
Expand All @@ -417,7 +419,7 @@ public UpdateResult update(Set<Label> targets, OutputInfo outputInfo, BlazeConte
ccDepencenciesInfo = ccDepsBuilder.build();

saveState();
return UpdateResult.create(updated.keySet(), ImmutableSet.of());
return ArtifactTrackerUpdateResult.create(updated.keySet(), ImmutableSet.of());
} catch (ExecutionException | IOException e) {
throw new BuildException(e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
import com.google.devtools.intellij.qsync.ArtifactTrackerData.JavaTargetArtifacts;
import com.google.idea.blaze.base.command.buildresult.OutputArtifact;
import com.google.idea.blaze.base.filecache.ArtifactState;
import com.google.idea.blaze.base.qsync.ArtifactTracker.UpdateResult;
import com.google.idea.blaze.base.qsync.ArtifactTrackerUpdateResult;
import com.google.idea.blaze.base.qsync.GroupedOutputArtifacts;
import com.google.idea.blaze.base.qsync.OutputGroup;
import com.google.idea.blaze.base.qsync.OutputInfo;
Expand Down Expand Up @@ -113,7 +113,7 @@ public void failed_fetch_resets_metadata() throws Exception {
ProjectDefinition.EMPTY);
artifactTracker.initialize();

final UpdateResult unused =
final ArtifactTrackerUpdateResult unused =
artifactTracker.update(
ImmutableSet.of(Label.of("//test:test")),
OutputInfo.builder()
Expand All @@ -129,7 +129,7 @@ public void failed_fetch_resets_metadata() throws Exception {

testArtifactFetcher.shouldFail = true;
try {
final UpdateResult unused2 =
final ArtifactTrackerUpdateResult unused2 =
artifactTracker.update(
ImmutableSet.of(Label.of("//test:test2")),
OutputInfo.builder()
Expand Down Expand Up @@ -221,7 +221,7 @@ public void library_sources() throws Throwable {
ProjectDefinition.EMPTY);
artifactTracker.initialize();

final UpdateResult unused =
final ArtifactTrackerUpdateResult unused =
artifactTracker.update(
ImmutableSet.of(Label.of("//test:test"), Label.of("//test:anothertest")),
OutputInfo.builder()
Expand Down Expand Up @@ -276,7 +276,7 @@ public void library_sources_unknown_lib() throws Throwable {
ProjectDefinition.EMPTY);
artifactTracker.initialize();

final UpdateResult unused =
final ArtifactTrackerUpdateResult unused =
artifactTracker.update(
ImmutableSet.of(Label.of("//test:test"), Label.of("//test:anothertest")),
OutputInfo.builder()
Expand Down

0 comments on commit 22fe29e

Please sign in to comment.