diff options
Diffstat (limited to 'src')
6 files changed, 207 insertions, 53 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java index 6f238429eb..91106b272c 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java @@ -533,12 +533,32 @@ public final class Label } ) public Label getRelative(String relName) throws LabelSyntaxException { + return getRelativeWithRemapping(relName, /* repositoryMapping= */ ImmutableMap.of()); + } + + /** + * Resolves a relative or absolute label name. If given name is absolute, then this method calls + * {@link #parseAbsolute}. Otherwise, it calls {@link #getLocalTargetLabel}. + * + * <p>For example: {@code :quux} relative to {@code //foo/bar:baz} is {@code //foo/bar:quux}; + * {@code //wiz:quux} relative to {@code //foo/bar:baz} is {@code //wiz:quux}; + * {@code @repo//foo:bar} relative to anything will be {@code @repo//foo:bar} if {@code @repo} is + * not in {@code repositoryMapping} but will be {@code @other_repo//foo:bar} if there is an entry + * {@code @repo -> @other_repo} in {@code repositoryMapping} + * + * @param relName the relative label name; must be non-empty + * @param repositoryMapping the map of local repository names in external repository to global + * repository names in main repo; can be empty, but not null + */ + public Label getRelativeWithRemapping( + String relName, ImmutableMap<RepositoryName, RepositoryName> repositoryMapping) + throws LabelSyntaxException { if (relName.length() == 0) { throw new LabelSyntaxException("empty package-relative label"); } if (LabelValidator.isAbsolute(relName)) { - return resolveRepositoryRelative(parseAbsolute(relName, false)); + return resolveRepositoryRelative(parseAbsolute(relName, false, repositoryMapping)); } else if (relName.equals(":")) { throw new LabelSyntaxException("':' is not a valid package-relative label"); } else if (relName.charAt(0) == ':') { diff --git a/src/main/java/com/google/devtools/build/lib/packages/BuildType.java b/src/main/java/com/google/devtools/build/lib/packages/BuildType.java index 96042edbd0..d7811acc32 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/BuildType.java +++ b/src/main/java/com/google/devtools/build/lib/packages/BuildType.java @@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; +import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.packages.License.DistributionType; import com.google.devtools.build.lib.packages.License.LicenseParsingException; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; @@ -145,7 +146,7 @@ public final class BuildType { * <p>The caller is responsible for casting the returned value appropriately. */ public static <T> Object selectableConvert( - Type type, Object x, Object what, @Nullable Label context) + Type<T> type, Object x, Object what, LabelConversionContext context) throws ConversionException { if (x instanceof com.google.devtools.build.lib.syntax.SelectorList) { return new SelectorList<T>( @@ -196,6 +197,31 @@ public final class BuildType { } } + /** Context in which to evaluate a label with repository remappings */ + public static class LabelConversionContext { + private final Label label; + private final ImmutableMap<RepositoryName, RepositoryName> repositoryMapping; + + public LabelConversionContext( + Label label, ImmutableMap<RepositoryName, RepositoryName> repositoryMapping) { + this.label = label; + this.repositoryMapping = repositoryMapping; + } + + public Label getLabel() { + return label; + } + + public ImmutableMap<RepositoryName, RepositoryName> getRepositoryMapping() { + return repositoryMapping; + } + + @Override + public String toString() { + return label.toString(); + } + } + private static class LabelType extends Type<Label> { private final LabelClass labelClass; @@ -236,10 +262,24 @@ public final class BuildType { return (Label) x; } try { - if (x instanceof String && context == null) { + if (!(x instanceof String)) { + throw new ConversionException(Type.STRING, x, what); + } + // TODO(b/110101445): check if context is ever actually null + if (context == null) { return Label.parseAbsolute((String) x, false); + // TODO(b/110308446): remove instances of context being a Label + } else if (context instanceof Label) { + return ((Label) context).getRelative(STRING.convert(x, what, context)); + } else if (context instanceof LabelConversionContext) { + LabelConversionContext labelConversionContext = (LabelConversionContext) context; + return labelConversionContext + .getLabel() + .getRelativeWithRemapping( + STRING.convert(x, what, context), labelConversionContext.getRepositoryMapping()); + } else { + throw new ConversionException("invalid context '" + context + "' in " + what); } - return ((Label) context).getRelative(STRING.convert(x, what, context)); } catch (LabelSyntaxException e) { throw new ConversionException("invalid label '" + x + "' in " + what + ": " + e.getMessage()); @@ -259,7 +299,7 @@ public final class BuildType { public static <ValueT> LabelKeyedDictType<ValueT> create(Type<ValueT> valueType) { Preconditions.checkArgument( valueType.getLabelClass() == LabelClass.NONE - || valueType.getLabelClass() == LabelClass.DEPENDENCY, + || valueType.getLabelClass() == LabelClass.DEPENDENCY, "Values associated with label keys must not be labels themselves."); return new LabelKeyedDictType<>(valueType); } @@ -428,16 +468,22 @@ public final class BuildType { } try { // Enforce value is relative to the context. - Label currentRule = (Label) context; - Label result = currentRule.getRelative(value); + Label currentRule; + ImmutableMap<RepositoryName, RepositoryName> repositoryMapping = ImmutableMap.of(); + if (context instanceof LabelConversionContext) { + currentRule = ((LabelConversionContext) context).getLabel(); + repositoryMapping = ((LabelConversionContext) context).getRepositoryMapping(); + } else { + throw new ConversionException("invalid context '" + context + "' in " + what); + } + Label result = currentRule.getRelativeWithRemapping(value, repositoryMapping); if (!result.getPackageIdentifier().equals(currentRule.getPackageIdentifier())) { throw new ConversionException("label '" + value + "' is not in the current package"); } return result; } catch (LabelSyntaxException e) { throw new ConversionException( - "illegal output file name '" + value + "' in rule " + context + ": " - + e.getMessage()); + "illegal output file name '" + value + "' in rule " + context + ": " + e.getMessage()); } } } @@ -452,8 +498,9 @@ public final class BuildType { private final List<Selector<T>> elements; @VisibleForTesting - SelectorList(List<Object> x, Object what, @Nullable Label context, - Type<T> originalType) throws ConversionException { + SelectorList( + List<Object> x, Object what, @Nullable LabelConversionContext context, Type<T> originalType) + throws ConversionException { if (x.size() > 1 && originalType.concat(ImmutableList.<T>of()) == null) { throw new ConversionException( String.format("type '%s' doesn't support select concatenation", originalType)); @@ -500,11 +547,11 @@ public final class BuildType { public Set<Label> getKeyLabels() { ImmutableSet.Builder<Label> keys = ImmutableSet.builder(); for (Selector<T> selector : getSelectors()) { - for (Label label : selector.getEntries().keySet()) { - if (!Selector.isReservedLabel(label)) { - keys.add(label); - } - } + for (Label label : selector.getEntries().keySet()) { + if (!Selector.isReservedLabel(label)) { + keys.add(label); + } + } } return keys.build(); } @@ -550,19 +597,24 @@ public final class BuildType { private final String noMatchError; private final boolean hasDefaultCondition; - /** - * Creates a new Selector using the default error message when no conditions match. - */ - Selector(ImmutableMap<?, ?> x, Object what, @Nullable Label context, Type<T> originalType) + /** Creates a new Selector using the default error message when no conditions match. */ + Selector( + ImmutableMap<?, ?> x, + Object what, + @Nullable LabelConversionContext context, + Type<T> originalType) throws ConversionException { this(x, what, context, originalType, ""); } - /** - * Creates a new Selector with a custom error message for when no conditions match. - */ - Selector(ImmutableMap<?, ?> x, Object what, @Nullable Label context, Type<T> originalType, - String noMatchError) throws ConversionException { + /** Creates a new Selector with a custom error message for when no conditions match. */ + Selector( + ImmutableMap<?, ?> x, + Object what, + @Nullable LabelConversionContext context, + Type<T> originalType, + String noMatchError) + throws ConversionException { this.originalType = originalType; LinkedHashMap<Label, T> result = Maps.newLinkedHashMapWithExpectedSize(x.size()); ImmutableSet.Builder<Label> defaultValuesBuilder = ImmutableSet.builder(); diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java index d6f7944288..3d84851a22 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Package.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java @@ -926,6 +926,11 @@ public class Package { return this; } + /** Get the repository mapping for this package */ + ImmutableMap<RepositoryName, RepositoryName> getRepositoryMapping() { + return this.repositoryMapping; + } + /** * Sets the name of this package's BUILD file. */ diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java index fba4c92a65..15349b10a3 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java +++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java @@ -36,11 +36,13 @@ import com.google.devtools.build.lib.analysis.config.transitions.SplitTransition import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.cmdline.PackageIdentifier; +import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.events.NullEventHandler; import com.google.devtools.build.lib.packages.Attribute.SkylarkComputedDefaultTemplate; import com.google.devtools.build.lib.packages.Attribute.SkylarkComputedDefaultTemplate.CannotPrecomputeDefaultsException; +import com.google.devtools.build.lib.packages.BuildType.LabelConversionContext; import com.google.devtools.build.lib.packages.BuildType.SelectorList; import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; @@ -1766,7 +1768,8 @@ public class RuleClass { EventHandler eventHandler) throws InterruptedException, CannotPrecomputeDefaultsException { BitSet definedAttrIndices = - populateDefinedRuleAttributeValues(rule, attributeValues, eventHandler); + populateDefinedRuleAttributeValues( + rule, pkgBuilder.getRepositoryMapping(), attributeValues, eventHandler); populateDefaultRuleAttributeValues(rule, pkgBuilder, definedAttrIndices, eventHandler); // Now that all attributes are bound to values, collect and store configurable attribute keys. populateConfigDependenciesAttribute(rule); @@ -1779,12 +1782,15 @@ public class RuleClass { * <p>Handles the special cases of the attribute named {@code "name"} and attributes with value * {@link Runtime#NONE}. * - * <p>Returns a bitset {@code b} where {@code b.get(i)} is {@code true} if this method set a - * value for the attribute with index {@code i} in this {@link RuleClass}. Errors are reported - * on {@code eventHandler}. + * <p>Returns a bitset {@code b} where {@code b.get(i)} is {@code true} if this method set a value + * for the attribute with index {@code i} in this {@link RuleClass}. Errors are reported on {@code + * eventHandler}. */ private <T> BitSet populateDefinedRuleAttributeValues( - Rule rule, AttributeValues<T> attributeValues, EventHandler eventHandler) { + Rule rule, + ImmutableMap<RepositoryName, RepositoryName> repositoryMapping, + AttributeValues<T> attributeValues, + EventHandler eventHandler) { BitSet definedAttrIndices = new BitSet(); for (T attributeAccessor : attributeValues.getAttributeAccessors()) { String attributeName = attributeValues.getName(attributeAccessor); @@ -1809,7 +1815,8 @@ public class RuleClass { Object nativeAttributeValue; if (attributeValues.valuesAreBuildLanguageTyped()) { try { - nativeAttributeValue = convertFromBuildLangType(rule, attr, attributeValue); + nativeAttributeValue = + convertFromBuildLangType(rule, attr, attributeValue, repositoryMapping); } catch (ConversionException e) { rule.reportError(String.format("%s: %s", rule.getLabel(), e.getMessage()), eventHandler); continue; @@ -2105,13 +2112,19 @@ public class RuleClass { * <p>Throws {@link ConversionException} if the conversion fails, or if {@code buildLangValue} is * a selector expression but {@code attr.isConfigurable()} is {@code false}. */ - private static Object convertFromBuildLangType(Rule rule, Attribute attr, Object buildLangValue) + private static Object convertFromBuildLangType( + Rule rule, + Attribute attr, + Object buildLangValue, + ImmutableMap<RepositoryName, RepositoryName> repositoryMapping) throws ConversionException { - Object converted = BuildType.selectableConvert( - attr.getType(), - buildLangValue, - new AttributeConversionContext(attr.getName(), rule.getRuleClass()), - rule.getLabel()); + LabelConversionContext context = new LabelConversionContext(rule.getLabel(), repositoryMapping); + Object converted = + BuildType.selectableConvert( + attr.getType(), + buildLangValue, + new AttributeConversionContext(attr.getName(), rule.getRuleClass()), + context); if ((converted instanceof SelectorList<?>) && !attr.isConfigurable()) { throw new ConversionException( diff --git a/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java b/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java index 0321498994..eccc5a5a35 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java @@ -23,7 +23,9 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; +import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.Location; +import com.google.devtools.build.lib.packages.BuildType.LabelConversionContext; import com.google.devtools.build.lib.packages.BuildType.Selector; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.EvalUtils; @@ -47,10 +49,13 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) public class BuildTypeTest { private Label currentRule; + private LabelConversionContext labelConversionContext; @Before public final void setCurrentRule() throws Exception { this.currentRule = Label.parseAbsolute("//quux:baz"); + this.labelConversionContext = + new LabelConversionContext(currentRule, /* repositoryMapping= */ ImmutableMap.of()); } @Test @@ -64,8 +69,9 @@ public class BuildTypeTest { .put("d", "//d") .build(); - assertThat(BuildType.LABEL_DICT_UNARY.convert(input, null, null).keySet()) - .containsExactly("c", "b", "a", "f", "e", "d").inOrder(); + assertThat(BuildType.LABEL_DICT_UNARY.convert(input, null, labelConversionContext).keySet()) + .containsExactly("c", "b", "a", "f", "e", "d") + .inOrder(); } @Test @@ -284,6 +290,17 @@ public class BuildTypeTest { entry1Label, entry2Label); } + @Test + public void testLabelWithRemapping() throws Exception { + LabelConversionContext context = + new LabelConversionContext( + currentRule, + ImmutableMap.of( + RepositoryName.create("@orig_repo"), RepositoryName.create("@new_repo"))); + Label label = BuildType.LABEL.convert("@orig_repo//foo:bar", null, context); + assertThat(label).isEquivalentAccordingToCompareTo(Label.parseAbsolute("@new_repo//foo:bar")); + } + /** * Tests basic {@link Selector} functionality. */ @@ -293,7 +310,7 @@ public class BuildTypeTest { "//conditions:a", "//a:a", "//conditions:b", "//b:b", Selector.DEFAULT_CONDITION_KEY, "//d:d"); - Selector<Label> selector = new Selector<>(input, null, currentRule, BuildType.LABEL); + Selector<Label> selector = new Selector<>(input, null, labelConversionContext, BuildType.LABEL); assertThat(selector.getOriginalType()).isEqualTo(BuildType.LABEL); Map<Label, Label> expectedMap = ImmutableMap.of( @@ -313,7 +330,7 @@ public class BuildTypeTest { "//conditions:a", "not a/../label", "//conditions:b", "also not a/../label", BuildType.Selector.DEFAULT_CONDITION_KEY, "whatever"); try { - new Selector<>(input, null, currentRule, BuildType.LABEL); + new Selector<>(input, null, labelConversionContext, BuildType.LABEL); fail("Expected Selector instantiation to fail since the input isn't a selection of labels"); } catch (ConversionException e) { assertThat(e).hasMessageThat().contains("invalid label 'not a/../label'"); @@ -329,7 +346,7 @@ public class BuildTypeTest { "not a/../label", "//a:a", BuildType.Selector.DEFAULT_CONDITION_KEY, "whatever"); try { - new Selector<>(input, null, currentRule, BuildType.LABEL); + new Selector<>(input, null, labelConversionContext, BuildType.LABEL); fail("Expected Selector instantiation to fail since the key isn't a label"); } catch (ConversionException e) { assertThat(e).hasMessageThat().contains("invalid label 'not a/../label'"); @@ -345,7 +362,7 @@ public class BuildTypeTest { "//conditions:a", "//a:a", "//conditions:b", "//b:b", BuildType.Selector.DEFAULT_CONDITION_KEY, "//d:d"); - assertThat(new Selector<>(input, null, currentRule, BuildType.LABEL).getDefault()) + assertThat(new Selector<>(input, null, labelConversionContext, BuildType.LABEL).getDefault()) .isEqualTo(Label.create("@//d", "d")); } @@ -355,8 +372,12 @@ public class BuildTypeTest { ImmutableList.of("//a:a"), "//conditions:b", ImmutableList.of("//b:b")), ""); Object selector2 = new SelectorValue(ImmutableMap.of("//conditions:c", ImmutableList.of("//c:c"), "//conditions:d", ImmutableList.of("//d:d")), ""); - BuildType.SelectorList<List<Label>> selectorList = new BuildType.SelectorList<>( - ImmutableList.of(selector1, selector2), null, currentRule, BuildType.LABEL_LIST); + BuildType.SelectorList<List<Label>> selectorList = + new BuildType.SelectorList<>( + ImmutableList.of(selector1, selector2), + null, + labelConversionContext, + BuildType.LABEL_LIST); assertThat(selectorList.getOriginalType()).isEqualTo(BuildType.LABEL_LIST); assertThat(selectorList.getKeyLabels()) @@ -388,7 +409,10 @@ public class BuildTypeTest { Object selector2 = new SelectorValue(ImmutableMap.of("//conditions:b", "//b:b"), ""); try { - new BuildType.SelectorList<>(ImmutableList.of(selector1, selector2), null, currentRule, + new BuildType.SelectorList<>( + ImmutableList.of(selector1, selector2), + null, + labelConversionContext, BuildType.LABEL_LIST); fail("Expected SelectorList initialization to fail on mixed element types"); } catch (ConversionException e) { @@ -457,14 +481,16 @@ public class BuildTypeTest { ImmutableList.of(Label.create("@//a", "a1"), Label.create("@//a", "a2")); // Conversion to direct type: - Object converted = BuildType - .selectableConvert(BuildType.LABEL_LIST, nativeInput, null, currentRule); + Object converted = + BuildType.selectableConvert( + BuildType.LABEL_LIST, nativeInput, null, labelConversionContext); assertThat(converted instanceof List<?>).isTrue(); assertThat((List<Label>) converted).containsExactlyElementsIn(expectedLabels); // Conversion to selectable type: - converted = BuildType - .selectableConvert(BuildType.LABEL_LIST, selectableInput, null, currentRule); + converted = + BuildType.selectableConvert( + BuildType.LABEL_LIST, selectableInput, null, labelConversionContext); BuildType.SelectorList<?> selectorList = (BuildType.SelectorList<?>) converted; assertThat(((Selector<Label>) selectorList.getSelectors().get(0)).getEntries().entrySet()) .containsExactlyElementsIn( @@ -508,7 +534,10 @@ public class BuildTypeTest { public void testUnconditionalSelects() throws Exception { assertThat( new Selector<>( - ImmutableMap.of("//conditions:a", "//a:a"), null, currentRule, BuildType.LABEL) + ImmutableMap.of("//conditions:a", "//a:a"), + null, + labelConversionContext, + BuildType.LABEL) .isUnconditional()) .isFalse(); assertThat( @@ -519,7 +548,7 @@ public class BuildTypeTest { BuildType.Selector.DEFAULT_CONDITION_KEY, "//b:b"), null, - currentRule, + labelConversionContext, BuildType.LABEL) .isUnconditional()) .isFalse(); @@ -527,7 +556,7 @@ public class BuildTypeTest { new Selector<>( ImmutableMap.of(BuildType.Selector.DEFAULT_CONDITION_KEY, "//b:b"), null, - currentRule, + labelConversionContext, BuildType.LABEL) .isUnconditional()) .isTrue(); diff --git a/src/test/shell/bazel/workspace_test.sh b/src/test/shell/bazel/workspace_test.sh index d7af19dbdd..1250d69ee1 100755 --- a/src/test/shell/bazel/workspace_test.sh +++ b/src/test/shell/bazel/workspace_test.sh @@ -418,6 +418,41 @@ EOF || fail "expected 'y_symbol' in $(cat bazel-genfiles/external/a/result.txt)" } +function test_repository_reassignment_label_in_build() { + # Repository a refers to @x + mkdir -p a + touch a/WORKSPACE + cat > a/BUILD<<EOF +genrule(name = "a", + srcs = ["@x//:x.txt"], + outs = ["result.txt"], + cmd = "echo hello > \$(location result.txt)" +) +EOF + + # Repository b is a substitute for x + mkdir -p b + touch b/WORKSPACE + cat >b/BUILD <<EOF +exports_files(srcs = ["x.txt"]) +EOF + echo "Hello from @b//:x.txt" > b/x.txt + + # Main repo assigns @x to @b within @a + mkdir -p main + cat > main/WORKSPACE <<EOF +workspace(name = "main") + +local_repository(name = "a", path="../a", repo_mapping = {"@x" : "@b"}) +local_repository(name = "b", path="../b") +EOF + touch main/BUILD + + cd main + bazel query --experimental_enable_repo_mapping --output=build @a//:a | grep "@b//:x.txt" \ + || fail "Expected srcs to contain '@b//:x.txt'" +} + function test_workspace_addition_change_aspect() { mkdir -p repo_one mkdir -p repo_two |