Skip to content

Commit 27e15ad

Browse files
katrecopybara-github
authored andcommitted
Clean up ConfiguredTargetValueAccessor and ConfiguredTargetAccessor
- Fix looking up a target from the walkable graph. - This prevents ConfiguredTargetValueAccessor from needing to look into ConfiguredTargetAccessor. - And removes duplicated code. - Move CTVA into the correct package. Part of #11993. Closes #12549. PiperOrigin-RevId: 344151199
1 parent 72cd5b2 commit 27e15ad

File tree

5 files changed

+31
-25
lines changed

5 files changed

+31
-25
lines changed

src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphQueryEnvironment.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import com.google.devtools.build.lib.packages.Target;
3131
import com.google.devtools.build.lib.pkgcache.PackageManager;
3232
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
33-
import com.google.devtools.build.lib.query2.ConfiguredTargetValueAccessor;
3433
import com.google.devtools.build.lib.query2.NamedThreadSafeOutputFormatterCallback;
3534
import com.google.devtools.build.lib.query2.PostAnalysisQueryEnvironment;
3635
import com.google.devtools.build.lib.query2.SkyQueryEnvironment;
@@ -102,7 +101,7 @@ public ActionGraphQueryEnvironment(
102101
.build();
103102
this.accessor =
104103
new ConfiguredTargetValueAccessor(
105-
walkableGraphSupplier.get(), this.configuredTargetKeyExtractor);
104+
walkableGraphSupplier.get(), this::getTarget, this.configuredTargetKeyExtractor);
106105
}
107106

108107
public ActionGraphQueryEnvironment(

src/main/java/com/google/devtools/build/lib/query2/aquery/AqueryThreadsafeCallback.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
package com.google.devtools.build.lib.query2.aquery;
1515

1616
import com.google.devtools.build.lib.events.ExtendedEventHandler;
17-
import com.google.devtools.build.lib.query2.ConfiguredTargetValueAccessor;
1817
import com.google.devtools.build.lib.query2.NamedThreadSafeOutputFormatterCallback;
1918
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor;
2019
import com.google.devtools.build.lib.skyframe.ConfiguredTargetValue;

src/main/java/com/google/devtools/build/lib/query2/ConfiguredTargetValueAccessor.java renamed to src/main/java/com/google/devtools/build/lib/query2/aquery/ConfiguredTargetValueAccessor.java

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
14-
package com.google.devtools.build.lib.query2;
14+
package com.google.devtools.build.lib.query2.aquery;
1515

1616
import com.google.common.collect.ImmutableList;
1717
import com.google.common.collect.ImmutableSet;
@@ -21,9 +21,10 @@
2121
import com.google.devtools.build.lib.packages.Rule;
2222
import com.google.devtools.build.lib.packages.Target;
2323
import com.google.devtools.build.lib.packages.TargetUtils;
24-
import com.google.devtools.build.lib.query2.cquery.ConfiguredTargetAccessor;
2524
import com.google.devtools.build.lib.query2.engine.KeyExtractor;
2625
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor;
26+
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetLookup;
27+
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetNotFoundException;
2728
import com.google.devtools.build.lib.query2.engine.QueryException;
2829
import com.google.devtools.build.lib.query2.engine.QueryExpression;
2930
import com.google.devtools.build.lib.query2.engine.QueryVisibility;
@@ -48,13 +49,16 @@
4849
public class ConfiguredTargetValueAccessor implements TargetAccessor<ConfiguredTargetValue> {
4950

5051
private final WalkableGraph walkableGraph;
52+
private final TargetLookup targetLookup;
5153
private final KeyExtractor<ConfiguredTargetValue, ConfiguredTargetKey>
5254
configuredTargetKeyExtractor;
5355

5456
public ConfiguredTargetValueAccessor(
5557
WalkableGraph walkableGraph,
58+
TargetLookup targetLookup,
5659
KeyExtractor<ConfiguredTargetValue, ConfiguredTargetKey> configuredTargetKeyExtractor) {
5760
this.walkableGraph = walkableGraph;
61+
this.targetLookup = targetLookup;
5862
this.configuredTargetKeyExtractor = configuredTargetKeyExtractor;
5963
}
6064

@@ -138,8 +142,15 @@ public ImmutableSet<QueryVisibility<ConfiguredTargetValue>> getVisibility(
138142
}
139143

140144
private Target getTargetFromConfiguredTargetValue(ConfiguredTargetValue configuredTargetValue) {
141-
return ConfiguredTargetAccessor.getTargetFromConfiguredTarget(
142-
configuredTargetValue.getConfiguredTarget(), walkableGraph);
145+
// Dereference any aliases that might be present.
146+
Label label = configuredTargetValue.getConfiguredObject().getOriginalLabel();
147+
try {
148+
return targetLookup.getTarget(label);
149+
} catch (InterruptedException e) {
150+
throw new IllegalStateException("Thread interrupted in the middle of getting a Target.", e);
151+
} catch (TargetNotFoundException e) {
152+
throw new IllegalStateException("Unable to get target from package in accessor.", e);
153+
}
143154
}
144155

145156
/** Returns the AspectValues that are attached to the given configuredTarget. */

src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetAccessor.java

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -32,19 +32,18 @@
3232
import com.google.devtools.build.lib.cmdline.Label;
3333
import com.google.devtools.build.lib.packages.ConfiguredAttributeMapper;
3434
import com.google.devtools.build.lib.packages.ExecGroup;
35-
import com.google.devtools.build.lib.packages.NoSuchTargetException;
3635
import com.google.devtools.build.lib.packages.Rule;
3736
import com.google.devtools.build.lib.packages.Target;
3837
import com.google.devtools.build.lib.packages.TargetUtils;
3938
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetAccessor;
39+
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.TargetNotFoundException;
4040
import com.google.devtools.build.lib.query2.engine.QueryException;
4141
import com.google.devtools.build.lib.query2.engine.QueryExpression;
4242
import com.google.devtools.build.lib.query2.engine.QueryVisibility;
4343
import com.google.devtools.build.lib.server.FailureDetails.ConfigurableQuery;
4444
import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
4545
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
4646
import com.google.devtools.build.lib.skyframe.ConfiguredTargetValue;
47-
import com.google.devtools.build.lib.skyframe.PackageValue;
4847
import com.google.devtools.build.lib.skyframe.ToolchainContextKey;
4948
import com.google.devtools.build.lib.skyframe.UnloadedToolchainContext;
5049
import com.google.devtools.build.skyframe.WalkableGraph;
@@ -169,25 +168,15 @@ public ImmutableSet<QueryVisibility<ConfiguredTarget>> getVisibility(
169168
}
170169

171170
public Target getTargetFromConfiguredTarget(ConfiguredTarget configuredTarget) {
172-
return getTargetFromConfiguredTarget(configuredTarget, walkableGraph);
173-
}
174-
175-
public static Target getTargetFromConfiguredTarget(
176-
ConfiguredTarget configuredTarget, WalkableGraph walkableGraph) {
177-
Target target = null;
171+
// Dereference any aliases that might be present.
172+
Label label = configuredTarget.getOriginalLabel();
178173
try {
179-
// Dereference any aliases that might be present.
180-
Label label = configuredTarget.getOriginalLabel();
181-
target =
182-
((PackageValue) walkableGraph.getValue(PackageValue.key(label.getPackageIdentifier())))
183-
.getPackage()
184-
.getTarget(label.getName());
185-
} catch (NoSuchTargetException e) {
174+
return queryEnvironment.getTarget(label);
175+
} catch (InterruptedException e) {
176+
throw new IllegalStateException("Thread interrupted in the middle of getting a Target.", e);
177+
} catch (TargetNotFoundException e) {
186178
throw new IllegalStateException("Unable to get target from package in accessor.", e);
187-
} catch (InterruptedException e2) {
188-
throw new IllegalStateException("Thread interrupted in the middle of getting a Target.", e2);
189179
}
190-
return target;
191180
}
192181

193182
/** Returns the rule that generates the given output file. */

src/main/java/com/google/devtools/build/lib/query2/engine/QueryEnvironment.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@
1717
import com.google.common.base.Preconditions;
1818
import com.google.common.collect.ImmutableList;
1919
import com.google.common.collect.ImmutableSet;
20+
import com.google.devtools.build.lib.cmdline.Label;
2021
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
22+
import com.google.devtools.build.lib.packages.Target;
2123
import com.google.devtools.build.lib.util.DetailedExitCode;
2224
import java.util.Collection;
2325
import java.util.List;
@@ -159,6 +161,12 @@ public final FilteringQueryFunction asFilteringFunction() {
159161
public abstract int getExpressionToFilterIndex();
160162
}
161163

164+
/** Functional interface for classes that need to look up a Target from a Label. */
165+
@FunctionalInterface
166+
interface TargetLookup {
167+
Target getTarget(Label label) throws TargetNotFoundException, InterruptedException;
168+
}
169+
162170
/**
163171
* Exception type for the case where a target cannot be found. It's basically a wrapper for
164172
* whatever exception is internally thrown.

0 commit comments

Comments
 (0)