Skip to content

Commit

Permalink
Merge RepositoryValue into RepositoryDirectoryValue
Browse files Browse the repository at this point in the history
RV is a simple wrapper around RDV.

RV is returned by RepositoryLoaderFunction, whereas RDV is returned by RepositoryDelegatorFunction. As expected, RLF simply calls RDF and does nothing else. So we can just remove RV+RLF and replace any usages with RDV+RDF.

Note that this didn't always use to be the case; the comment in RLF.java indicates that this split was done to prevent SkyFunctions restarts causing repos to be fetched multiple times. This was at one point possible (see [this revision](https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryLoaderFunction.java;l=55;drc=63c60a0c0f7806e7909e1c4242263fc49fe585d5) for example).

As a next step, we could maybe rename RepositoryDelegatorFunction to RepositoryLoaderFunction, and RepositoryDirectoryValue to RepositoryValue; the latter pair of names is arguably better.

PiperOrigin-RevId: 361830387
  • Loading branch information
Wyverald authored and copybara-github committed Mar 9, 2021
1 parent fcf9dd5 commit 01e0f43
Show file tree
Hide file tree
Showing 22 changed files with 63 additions and 338 deletions.
1 change: 0 additions & 1 deletion src/main/java/com/google/devtools/build/lib/bazel/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/rules:repository/new_local_repository_function",
"//src/main/java/com/google/devtools/build/lib/rules:repository/new_local_repository_rule",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_loader_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:mutable_supplier",
"//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryDirtinessChecker;
import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
import com.google.devtools.build.lib.rules.repository.RepositoryLoaderFunction;
import com.google.devtools.build.lib.runtime.BlazeModule;
import com.google.devtools.build.lib.runtime.BlazeRuntime;
import com.google.devtools.build.lib.runtime.Command;
Expand Down Expand Up @@ -193,7 +192,6 @@ public void workspaceInit(
directories.getWorkspace(), managedDirectoriesKnowledge);
builder.addCustomDirtinessChecker(customDirtinessChecker);
// Create the repository function everything flows through.
builder.addSkyFunction(SkyFunctions.REPOSITORY, new RepositoryLoaderFunction());
RepositoryDelegatorFunction repositoryDelegatorFunction =
new RepositoryDelegatorFunction(
repositoryHandlers,
Expand Down
17 changes: 0 additions & 17 deletions src/main/java/com/google/devtools/build/lib/rules/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ java_library(
":repository/new_repository_file_handler",
":repository/repository_directory_value",
":repository/repository_function",
":repository/repository_loader_function",
":repository/resolved_file_value",
":repository/resolved_hashes_value",
":repository/workspace_attribute_mapper",
Expand Down Expand Up @@ -444,22 +443,6 @@ java_library(
],
)

java_library(
name = "repository/repository_loader_function",
srcs = ["repository/RepositoryLoaderFunction.java"],
deps = [
":repository/repository_directory_value",
":repository/workspace_file_helper",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/skyframe:repository_value",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/skyframe",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//third_party:jsr305",
],
)

java_library(
name = "repository/resolved_file_value",
srcs = ["repository/ResolvedFileValue.java"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,10 @@ public static Key key(RepositoryName repository) {
return Key.create(repository);
}

/** The SkyKey for retrieving the local directory of an external repository. */
@AutoCodec.VisibleForSerialization
@AutoCodec
static class Key extends AbstractSkyKey<RepositoryName> {
public static class Key extends AbstractSkyKey<RepositoryName> {
private static final Interner<Key> interner = BlazeInterners.newWeakInterner();

private Key(RepositoryName arg) {
Expand Down

This file was deleted.

21 changes: 3 additions & 18 deletions src/main/java/com/google/devtools/build/lib/skyframe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -1780,14 +1780,14 @@ java_library(
":local_repository_lookup_value",
":package_lookup_value",
":precomputed_value",
":repository_value",
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/cmdline:LabelValidator",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/packages/semantics",
"//src/main/java/com/google/devtools/build/lib/pkgcache",
"//src/main/java/com/google/devtools/build/lib/repository:external_package_helper",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/build/skyframe",
Expand All @@ -1802,11 +1802,11 @@ java_library(
name = "package_lookup_value",
srcs = ["PackageLookupValue.java"],
deps = [
":repository_value",
":sky_functions",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
Expand Down Expand Up @@ -2287,21 +2287,6 @@ java_library(
],
)

java_library(
name = "repository_value",
srcs = ["RepositoryValue.java"],
deps = [
":sky_functions",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//third_party:guava",
],
)

java_library(
name = "root_package_extractor",
srcs = ["RootPackageExtractor.java"],
Expand Down Expand Up @@ -2389,12 +2374,12 @@ java_library(
deps = [
":abstract_label_cycle_reporter",
":bzl_load_value",
":repository_value",
":sky_functions",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/repository:request_repository_information_event",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value",
"//src/main/java/com/google/devtools/build/skyframe",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//third_party:guava",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
import com.google.devtools.build.lib.repository.ExternalPackageHelper;
import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.lib.vfs.RootedPath;
Expand Down Expand Up @@ -326,11 +327,11 @@ private PackageLookupValue computeExternalPackageLookupValue(
SkyKey skyKey, Environment env, PackageIdentifier packageIdentifier)
throws PackageLookupFunctionException, InterruptedException {
PackageIdentifier id = (PackageIdentifier) skyKey.argument();
SkyKey repositoryKey = RepositoryValue.key(id.getRepository());
RepositoryValue repositoryValue;
SkyKey repositoryKey = RepositoryDirectoryValue.key(id.getRepository());
RepositoryDirectoryValue repositoryValue;
try {
repositoryValue =
(RepositoryValue)
(RepositoryDirectoryValue)
env.getValueOrThrow(
repositoryKey,
NoSuchPackageException.class,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.concurrent.BlazeInterners;
import com.google.devtools.build.lib.packages.BuildFileName;
import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
Expand Down Expand Up @@ -68,7 +69,7 @@ enum ErrorReason {
protected PackageLookupValue() {}

public static PackageLookupValue success(
RepositoryValue repository, Root root, BuildFileName buildFileName) {
RepositoryDirectoryValue repository, Root root, BuildFileName buildFileName) {
return new SuccessfulPackageLookupValue(repository, root, buildFileName);
}

Expand Down Expand Up @@ -162,20 +163,20 @@ public static class SuccessfulPackageLookupValue extends PackageLookupValue {
* controlling a symbolic link the path goes trough). Can be {@code null}, if does not depend on
* such a repository; will always be {@code null} for packages in the main repository.
*/
@Nullable private final RepositoryValue repository;
@Nullable private final RepositoryDirectoryValue repository;

private final Root root;
private final BuildFileName buildFileName;

SuccessfulPackageLookupValue(
@Nullable RepositoryValue repository, Root root, BuildFileName buildFileName) {
@Nullable RepositoryDirectoryValue repository, Root root, BuildFileName buildFileName) {
this.repository = repository;
this.root = root;
this.buildFileName = buildFileName;
}

@Nullable
public RepositoryValue repository() {
public RepositoryDirectoryValue repository() {
return repository;
}

Expand Down
Loading

0 comments on commit 01e0f43

Please sign in to comment.