Skip to content

Commit f91c008

Browse files
kchodorowvladmos
authored andcommitted
Automated g4 rollback of commit d5ee3b5.
*** Reason for rollback *** The go rules have tons of transitive dependencies that are not declared in their "local" WORKSPACE files, so this broke lots of projects on ci.bazel.build I'll fix up the go rules, update all of _their_ reverse-dep projects, and resubmit. *** Original change description *** Repositories can only be accessed in projects that define them in their WORKSPACE file This is prep for #1943 - hierarchical workspace loading. RELNOTES[INC]: Remote repositories must define any remote repositories they themselves use (e.g., if @x//:foo depends on @y//:bar, @y must be defined in @x's WORKSPACE file). PiperOrigin-RevId: 154321845
1 parent 496d3de commit f91c008

File tree

8 files changed

+3
-251
lines changed

8 files changed

+3
-251
lines changed

src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryVisibilityFunction.java

-127
This file was deleted.

src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java

-34
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import com.google.common.base.VerifyException;
2323
import com.google.common.collect.ImmutableList;
2424
import com.google.common.collect.ImmutableMap;
25-
import com.google.common.collect.ImmutableSet;
2625
import com.google.common.collect.Iterables;
2726
import com.google.common.collect.LinkedHashMultimap;
2827
import com.google.common.collect.LinkedListMultimap;
@@ -51,7 +50,6 @@
5150
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
5251
import com.google.devtools.build.lib.analysis.config.PatchTransition;
5352
import com.google.devtools.build.lib.cmdline.Label;
54-
import com.google.devtools.build.lib.cmdline.RepositoryName;
5553
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
5654
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
5755
import com.google.devtools.build.lib.collect.nestedset.Order;
@@ -71,8 +69,6 @@
7169
import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier;
7270
import com.google.devtools.build.lib.packages.Target;
7371
import com.google.devtools.build.lib.packages.TargetUtils;
74-
import com.google.devtools.build.lib.rules.repository.RepositoryVisibilityFunction;
75-
import com.google.devtools.build.lib.rules.repository.RepositoryVisibilityFunction.RepositoryVisibilityValue;
7672
import com.google.devtools.build.lib.skyframe.AspectFunction.AspectCreationException;
7773
import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
7874
import com.google.devtools.build.lib.skyframe.SkyframeExecutor.BuildViewProvider;
@@ -95,7 +91,6 @@
9591
import java.util.LinkedHashMap;
9692
import java.util.List;
9793
import java.util.Map;
98-
import java.util.Map.Entry;
9994
import java.util.Objects;
10095
import java.util.Set;
10196
import java.util.concurrent.Semaphore;
@@ -341,35 +336,6 @@ static OrderedSetMultimap<Attribute, ConfiguredTarget> computeDependencies(
341336
throw new DependencyEvaluationException(e);
342337
}
343338

344-
RepositoryName ctgRepository = ctgValue.getLabel().getPackageIdentifier().getRepository();
345-
ImmutableSet.Builder<SkyKey> pairsToCheck = ImmutableSet.builder();
346-
for (Dependency dep : depValueNames.values()) {
347-
RepositoryName depRepository = dep.getLabel().getPackageIdentifier().getRepository();
348-
if (ctgRepository.equals(depRepository) || depRepository.isMain()) {
349-
continue;
350-
}
351-
pairsToCheck.add(RepositoryVisibilityFunction.key(ctgRepository, depRepository));
352-
}
353-
Map<SkyKey, SkyValue> pairs = env.getValues(pairsToCheck.build());
354-
if (env.valuesMissing()) {
355-
return null;
356-
}
357-
boolean hadError = false;
358-
for (Entry<SkyKey, SkyValue> entry : pairs.entrySet()) {
359-
if (!((RepositoryVisibilityValue) entry.getValue()).ok()) {
360-
RepositoryVisibilityFunction.Key key =
361-
(RepositoryVisibilityFunction.Key) entry.getKey().argument();
362-
String message = ctgValue.getLabel() + " has a dependency on "
363-
+ key.child() + " but does not define " + key.child() + " in its WORKSPACE";
364-
env.getListener().handle(Event.error(message));
365-
hadError = true;
366-
}
367-
}
368-
if (hadError) {
369-
throw new DependencyEvaluationException(
370-
new ConfiguredValueCreationException("Missing external repository definitions"));
371-
}
372-
373339
// Trim each dep's configuration so it only includes the fragments needed by its transitive
374340
// closure (only dynamic configurations support this).
375341
if (useDynamicConfigurations(ctgValue.getConfiguration())) {

src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java

-2
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,6 @@ public final class SkyFunctions {
109109
SkyFunctionName.create("ACTION_TEMPLATE_EXPANSION");
110110
public static final SkyFunctionName LOCAL_REPOSITORY_LOOKUP =
111111
SkyFunctionName.create("LOCAL_REPOSITORY_LOOKUP");
112-
public static final SkyFunctionName REPOSITORY_DEPENDENCY =
113-
SkyFunctionName.create("REPOSITORY_DEPENDENCY");
114112

115113
public static Predicate<SkyKey> isSkyFunction(final SkyFunctionName functionName) {
116114
return new Predicate<SkyKey>() {

src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java

-2
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,6 @@
105105
import com.google.devtools.build.lib.pkgcache.TestFilter;
106106
import com.google.devtools.build.lib.pkgcache.TransitivePackageLoader;
107107
import com.google.devtools.build.lib.profiler.AutoProfiler;
108-
import com.google.devtools.build.lib.rules.repository.RepositoryVisibilityFunction;
109108
import com.google.devtools.build.lib.skyframe.AspectValue.AspectValueKey;
110109
import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.FileDirtinessChecker;
111110
import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction;
@@ -447,7 +446,6 @@ private ImmutableMap<SkyFunctionName, SkyFunction> skyFunctions(
447446
SkyFunctions.ACTION_TEMPLATE_EXPANSION,
448447
new ActionTemplateExpansionFunction(removeActionsAfterEvaluation));
449448
map.put(SkyFunctions.LOCAL_REPOSITORY_LOOKUP, new LocalRepositoryLookupFunction());
450-
map.put(SkyFunctions.REPOSITORY_DEPENDENCY, new RepositoryVisibilityFunction());
451449
map.putAll(extraSkyFunctions);
452450
return map.build();
453451
}

src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java

+1-3
Original file line numberDiff line numberDiff line change
@@ -551,9 +551,7 @@ public void testCcLibraryExternalIncludesNotWarned() throws Exception {
551551
new ModifiedFileSet.Builder().modify(PathFragment.create("WORKSPACE")).build(),
552552
rootDirectory);
553553
FileSystemUtils.createDirectoryAndParents(scratch.resolve("/foo/bar"));
554-
scratch.file("/foo/WORKSPACE",
555-
"workspace(name = 'pkg')",
556-
"local_repository(name = 'bazel_tools', path = '/whatever')");
554+
scratch.file("/foo/WORKSPACE", "workspace(name = 'pkg')");
557555
scratch.file(
558556
"/foo/bar/BUILD",
559557
"cc_library(name = 'lib',",

src/test/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibraryTest.java

+1-3
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,7 @@ public void outputDirectoryForProtoCompileAction_externalRepos() throws Exceptio
149149
scratch.file(
150150
"x/BUILD", "cc_proto_library(name = 'foo_cc_proto', deps = ['@bla//foo:bar_proto'])");
151151

152-
scratch.file("/bla/WORKSPACE",
153-
"local_repository(name = 'com_google_protobuf', path = '/whatever')",
154-
"local_repository(name = 'com_google_protobuf_cc', path = '/whatever')");
152+
scratch.file("/bla/WORKSPACE");
155153
// Create the rule '@bla//foo:bar_proto'.
156154
scratch.file(
157155
"/bla/foo/BUILD",

src/test/shell/bazel/external_correctness_test.sh

+1-4
Original file line numberDiff line numberDiff line change
@@ -176,15 +176,12 @@ function test_refs_btwn_repos() {
176176
REMOTE1=$TEST_TMPDIR/remote1
177177
REMOTE2=$TEST_TMPDIR/remote2
178178
mkdir -p $REMOTE1 $REMOTE2
179-
touch $REMOTE1/WORKSPACE
179+
touch $REMOTE1/WORKSPACE $REMOTE2/WORKSPACE
180180
cat > $REMOTE1/input <<EOF
181181
1.0
182182
EOF
183183
cat > $REMOTE1/BUILD <<EOF
184184
exports_files(['input'])
185-
EOF
186-
cat > $REMOTE2/WORKSPACE <<EOF
187-
local_repository(name = "remote1", path = "/whatever")
188185
EOF
189186
cat > $REMOTE2/BUILD <<EOF
190187
genrule(

src/test/shell/bazel/workspace_test.sh

-76
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,6 @@ EOF
190190
touch foo/baz
191191
cat > bar/WORKSPACE <<EOF
192192
workspace(name = "bar")
193-
194-
# Needs to be defined, since @foo is referenced from the genrule.
195-
local_repository(name = "foo", path = "/whatever")
196193
EOF
197194
cat > bar/BUILD <<EOF
198195
genrule(
@@ -246,77 +243,4 @@ EOF
246243
assert_contains "override" bazel-genfiles/external/o/gen.out
247244
}
248245

249-
function test_direct_deps() {
250-
REPO1="$PWD/repo1"
251-
REPO2="$PWD/repo2"
252-
mkdir -p "$REPO1" "$REPO2"
253-
254-
# repo1 has dependencies on the main repo and repo2.
255-
cat > "$REPO1/WORKSPACE" <<EOF
256-
workspace(name = "repo1")
257-
258-
local_repository(name = "repo2", path = "/whatever")
259-
# Definition of main_repo purposely omitted to make build fail.
260-
EOF
261-
cat > "$REPO1/BUILD" << EOF
262-
genrule(
263-
name = "bar",
264-
srcs = ["@repo2//:baz", "@main_repo//:qux"],
265-
outs = ["bar.out"],
266-
cmd = "echo \$(SRCS) > \$@",
267-
visibility = ["//visibility:public"],
268-
)
269-
EOF
270-
271-
# repo2 has no dependencies.
272-
touch "$REPO2/WORKSPACE"
273-
cat > "$REPO2/BUILD" << EOF
274-
genrule(
275-
name = "baz",
276-
outs = ["baz.out"],
277-
cmd = "echo 'baz' > \$@",
278-
visibility = ["//visibility:public"],
279-
)
280-
EOF
281-
282-
# The main repo has dependencies on repo1.
283-
cat > WORKSPACE << EOF
284-
workspace(name = "main_repo")
285-
286-
local_repository(name = "repo1", path = "$REPO1")
287-
# TODO: move this to repo1/WORKSPACE once hierarchical workspaces work.
288-
local_repository(name = "repo2", path = "$REPO2")
289-
EOF
290-
cat > BUILD <<EOF
291-
exports_files(["qux"])
292-
genrule(
293-
name = "foo",
294-
srcs = ["@repo1//:bar"],
295-
outs = ["foo.out"],
296-
cmd = "echo 'hi' > \$@",
297-
)
298-
EOF
299-
touch qux
300-
301-
bazel build //:foo &> $TEST_log && fail "Expected missing main_repo"
302-
expect_log "@repo1//:bar has a dependency on @main_repo but does not define @main_repo in its WORKSPACE"
303-
304-
cat > "$REPO1/WORKSPACE" <<EOF
305-
workspace(name = "repo1")
306-
307-
local_repository(name = "main_repo", path = "/whatever")
308-
# Definition of repo2 purposely omitted to make build fail.
309-
EOF
310-
bazel build //:foo &> $TEST_log && fail "Expected missing repo2"
311-
expect_log "@repo1//:bar has a dependency on @repo2 but does not define @repo2 in its WORKSPACE"
312-
313-
cat > "$REPO1/WORKSPACE" <<EOF
314-
workspace(name = "repo1")
315-
316-
local_repository(name = "main_repo", path = "/whatever")
317-
local_repository(name = "repo2", path = "/whatever")
318-
EOF
319-
bazel build //:foo &> $TEST_log || fail "Expected success"
320-
}
321-
322246
run_suite "workspace tests"

0 commit comments

Comments
 (0)