From 94e6cf6bb9aadbd2c8eb95f7bae85be5a3650092 Mon Sep 17 00:00:00 2001 From: Klaus Aehlig Date: Fri, 20 Sep 2019 04:13:07 -0700 Subject: [PATCH] Apply repository-mapping also to target patterns Target patterns are to be read in the name space of the main repository. This namespace, however, can also contain a mapping of repository names, in particular that of the main repository (#7130, #3155). While there, also make an error message more informative. (This message change is related as the modified evaluation order causes another error to be found first in a test example.) Change-Id: I022240e5c201d33e31f2a818f74a91af3b6f7b3d PiperOrigin-RevId: 270240453 --- .../build/lib/packages/WorkspaceFactory.java | 5 ++- .../skyframe/TargetPatternPhaseFunction.java | 27 ++++++++++++-- src/test/shell/bazel/workspace_test.sh | 37 +++++++++++++++++++ 3 files changed, 65 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java index fafa3a9194b1ef..3d3ac718ef603f 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java @@ -323,7 +323,10 @@ public Object invoke(Map kwargs, FuncallExpression ast, Environm ast); if (!WorkspaceGlobals.isLegalWorkspaceName(rule.getName())) { throw new EvalException( - ast.getLocation(), rule + "'s name field must be a legal workspace name"); + ast.getLocation(), + rule + + "'s name field must be a legal workspace name;" + + " workspace names may contain only A-Z, a-z, 0-9, '-', '_' and '.'"); } } catch (RuleFactory.InvalidRuleException | Package.NameConflictException diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java index c73836e7ad5a71..a3d4a79de69caf 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java @@ -18,14 +18,18 @@ import com.google.auto.value.AutoValue; import com.google.common.base.Preconditions; import com.google.common.base.Predicates; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.Iterables; import com.google.common.collect.Sets; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelConstants; +import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.cmdline.ResolvedTargets; import com.google.devtools.build.lib.cmdline.TargetParsingException; +import com.google.devtools.build.lib.cmdline.TargetPattern; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.packages.NoSuchPackageException; @@ -89,9 +93,17 @@ public TargetPatternPhaseValue compute(SkyKey key, Environment env) throws Inter workspaceName = packageValue.getPackage().getWorkspaceName(); } + RepositoryMappingValue repositoryMappingValue = + (RepositoryMappingValue) env.getValue(RepositoryMappingValue.key(RepositoryName.MAIN)); + if (repositoryMappingValue == null) { + return null; + } + // Determine targets to build: List failedPatterns = new ArrayList(); - List expandedPatterns = getTargetsToBuild(env, options, failedPatterns); + List expandedPatterns = + getTargetsToBuild( + env, options, repositoryMappingValue.getRepositoryMapping(), failedPatterns); ResolvedTargets targets = env.valuesMissing() ? null @@ -258,12 +270,21 @@ private static void maybeReportDeprecation( * @param failedPatterns a list into which failed patterns are added */ private static List getTargetsToBuild( - Environment env, TargetPatternPhaseKey options, List failedPatterns) + Environment env, + TargetPatternPhaseKey options, + ImmutableMap repoMapping, + List failedPatterns) throws InterruptedException { + + ImmutableList.Builder canonicalPatterns = new ImmutableList.Builder<>(); + for (String rawPattern : options.getTargetPatterns()) { + canonicalPatterns.add(TargetPattern.renameRepository(rawPattern, repoMapping)); + } + List patternSkyKeys = new ArrayList<>(options.getTargetPatterns().size()); for (TargetPatternSkyKeyOrException keyOrException : TargetPatternValue.keys( - options.getTargetPatterns(), + canonicalPatterns.build(), options.getBuildManualTests() ? FilteringPolicies.NO_FILTER : FilteringPolicies.FILTER_MANUAL, diff --git a/src/test/shell/bazel/workspace_test.sh b/src/test/shell/bazel/workspace_test.sh index c0190e3d51d9f9..c0412edb183de1 100755 --- a/src/test/shell/bazel/workspace_test.sh +++ b/src/test/shell/bazel/workspace_test.sh @@ -1125,5 +1125,42 @@ EOF bazel build @foo//:y || fail "Expected success" } +function test_renaming_visibility_main() { + mkdir local_a + touch local_a/WORKSPACE + cat > local_a/BUILD <<'EOF' +genrule( + name = "x", + outs = ["x.txt"], + cmd = "echo Hello World > $@", + visibility = ["@foo//:__pkg__"], +) +EOF + mkdir mainrepo + cd mainrepo + cat > WORKSPACE <<'EOF' +workspace(name="foo") +local_repository( + name = "source", + path = "../local_a", +) +EOF + cat > BUILD <<'EOF' +genrule( + name = "y", + srcs = ["@source//:x"], + cmd = "cp $< $@", + outs = ["y.txt"], +) +EOF + echo; echo not remapping main repo; echo + bazel build --incompatible_remap_main_repo=false @foo//:y \ + || fail "Expected success" + + echo; echo remapping main repo; echo + bazel build --incompatible_remap_main_repo=true @foo//:y \ + || fail "Expected success" + +} run_suite "workspace tests"