diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibrary.java index 9f6163cc111cd4..4396107bb43ea0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibrary.java @@ -32,6 +32,10 @@ public class BazelProtoLibrary implements RuleConfiguredTargetFactory { @Override public ConfiguredTarget create(RuleContext ruleContext) throws ActionConflictException { ProtoInfo protoInfo = ProtoCommon.createProtoInfo(ruleContext); + if (ruleContext.hasErrors()) { + return null; + } + ProtoCompileActionBuilder.writeDescriptorSet(ruleContext, protoInfo, Services.ALLOW); Runfiles dataRunfiles = diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryRule.java b/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryRule.java index 8a9c044aa9b20f..4ef8952c56445c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryRule.java @@ -83,6 +83,29 @@ the source root will be the workspace directory (default). source. */ .add(attr("exports", LABEL_LIST).allowedRuleClasses("proto_library").allowedFileTypes()) + /* + The prefix to strip from the paths of the .proto files in this rule. + +

When set, .proto source files in the srcs attribute of this rule are + accessible at their path with this prefix cut off. + +

If it's a relative path (not starting with a slash), it's taken as a package-relative + one. If it's an absolute one, it's understood as a repository-relative path. + +

The prefix in the import_prefix attribute is added after this prefix is + stripped. + */ + .add(attr("strip_import_prefix", STRING)) + /* + The prefix to add to the paths of the .proto files in this rule. + +

When set, the .proto source files in the srcs attribute of this rule are + accessible at is the value of this attribute prepended to their repository-relative path. + +

The prefix in the strip_import_prefix attribute is removed before this + prefix is added. + */ + .add(attr("import_prefix", STRING)) .advertiseProvider(ProtoInfo.class) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java index 060b23dd7c90aa..261efb58f0fceb 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java @@ -16,6 +16,7 @@ import static com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode.TARGET; import static com.google.devtools.build.lib.collect.nestedset.Order.STABLE_ORDER; +import static com.google.devtools.build.lib.syntax.Type.STRING; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; @@ -24,13 +25,13 @@ import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.Runfiles; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; +import com.google.devtools.build.lib.analysis.actions.SymlinkAction; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.packages.BuildType; -import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.PathFragment; import javax.annotation.Nullable; @@ -116,16 +117,44 @@ private static NestedSet computeTransitiveProtoSourceRoots( } /** - * Returns the source root of the currentl {@code proto_library} or "." if it's the source root. + * The set of .proto files in a single proto_library rule. + * + *

In addition to the artifacts of the .proto files, this also includes the proto source root + * so that rules depending on this know how to include them. + */ + // TODO(lberki): Would be nice if had these in ProtoInfo instead of that haphazard set of fields + // Unfortunately, ProtoInfo has a Starlark interface so that requires a migration. + static final class Library { + private final ImmutableList sources; + private final String sourceRoot; + + Library(ImmutableList sources, String sourceRoot) { + this.sources = sources; + this.sourceRoot = sourceRoot; + } + + public ImmutableList getSources() { + return sources; + } + + public String getSourceRoot() { + return sourceRoot; + } + } + + /** + * Returns the {@link Library} representing this proto_library rule. + * + *

Assumes that strip_import_prefix and import_prefix are unset. * *

Build will fail if the {@code proto_source_root} of the current library is neither the * package name nor the source root. */ // TODO(lberki): This should really be a PathFragment. Unfortunately, it's on the Starlark API of // ProtoInfo so it's not an easy change :( - private static String getProtoSourceRoot(RuleContext ruleContext) { - String protoSourceRoot = - ruleContext.attributes().get("proto_source_root", Type.STRING); + private static Library getProtoSourceRoot( + RuleContext ruleContext, ImmutableList directSources) { + String protoSourceRoot = ruleContext.attributes().get("proto_source_root", STRING); if (!protoSourceRoot.isEmpty()) { checkProtoSourceRootIsTheSameAsPackage(protoSourceRoot, ruleContext); } @@ -140,7 +169,110 @@ private static String getProtoSourceRoot(RuleContext ruleContext) { .getPathUnderExecRoot() .getRelative(protoSourceRoot) .getPathString(); - return result.isEmpty() ? "." : result; + return new Library(directSources, result.isEmpty() ? "." : result); + } + + private static PathFragment getPathFragmentAttribute( + RuleContext ruleContext, String attributeName) { + if (!ruleContext.attributes().has(attributeName)) { + return null; + } + + if (!ruleContext.attributes().isAttributeValueExplicitlySpecified(attributeName)) { + return null; + } + + String asString = ruleContext.attributes().get(attributeName, STRING); + if (!PathFragment.isNormalized(asString)) { + ruleContext.attributeError( + attributeName, "should be normalized (without uplevel references or '.' path segments)"); + return null; + } + + return PathFragment.create(asString); + } + + /** + * Returns the {@link Library} representing this proto_library rule if import prefix + * munging is done. Otherwise, returns null. + */ + private static Library createVirtualImportDirectoryMaybe( + RuleContext ruleContext, ImmutableList protoSources) { + PathFragment importPrefix = getPathFragmentAttribute(ruleContext, "import_prefix"); + PathFragment stripImportPrefix = getPathFragmentAttribute(ruleContext, "strip_import_prefix"); + + if (importPrefix == null && stripImportPrefix == null) { + // Simple case, no magic required. + return null; + } + + if (!ruleContext.attributes().get("proto_source_root", STRING).isEmpty()) { + ruleContext.ruleError( + "the 'proto_source_root' attribute is incompatible with " + + "'strip_import_prefix' and 'import_prefix"); + return null; + } + + if (stripImportPrefix == null) { + stripImportPrefix = PathFragment.EMPTY_FRAGMENT; + } else if (stripImportPrefix.isAbsolute()) { + stripImportPrefix = + ruleContext + .getLabel() + .getPackageIdentifier() + .getRepository() + .getSourceRoot() + .getRelative(stripImportPrefix.toRelative()); + } else { + stripImportPrefix = ruleContext.getPackageDirectory().getRelative(stripImportPrefix); + } + + if (importPrefix == null) { + importPrefix = PathFragment.EMPTY_FRAGMENT; + } + + if (importPrefix.isAbsolute()) { + ruleContext.attributeError("import_prefix", "should be a relative path"); + return null; + } + + ImmutableList.Builder symlinks = ImmutableList.builder(); + PathFragment sourceRootPath = ruleContext.getUniqueDirectory("_virtual_imports"); + + for (Artifact realProtoSource : protoSources) { + if (!realProtoSource.getRootRelativePath().startsWith(stripImportPrefix)) { + ruleContext.ruleError( + String.format( + ".proto file '%s' is not under the specified strip prefix '%s'", + realProtoSource.getExecPathString(), stripImportPrefix.getPathString())); + continue; + } + + PathFragment importPath = + importPrefix.getRelative( + realProtoSource.getRootRelativePath().relativeTo(stripImportPrefix)); + + Artifact virtualProtoSource = + ruleContext.getDerivedArtifact( + sourceRootPath.getRelative(importPath), ruleContext.getBinOrGenfilesDirectory()); + + ruleContext.registerAction( + SymlinkAction.toArtifact( + ruleContext.getActionOwner(), + realProtoSource, + virtualProtoSource, + "Symlinking virtual .proto sources for " + ruleContext.getLabel())); + + symlinks.add(virtualProtoSource); + } + + String sourceRoot = + ruleContext + .getBinOrGenfilesDirectory() + .getExecPath() + .getRelative(sourceRootPath) + .getPathString(); + return new Library(symlinks.build(), sourceRoot); } /** @@ -224,22 +356,33 @@ public static ProtoInfo createProtoInfo(RuleContext ruleContext) { checkSourceFilesAreInSamePackage(ruleContext); ImmutableList directProtoSources = ruleContext.getPrerequisiteArtifacts("srcs", Mode.TARGET).list(); - String protoSourceRoot = ProtoCommon.getProtoSourceRoot(ruleContext); + Library library = createVirtualImportDirectoryMaybe(ruleContext, directProtoSources); + if (ruleContext.hasErrors()) { + return null; + } + + if (library == null) { + library = getProtoSourceRoot(ruleContext, directProtoSources); + } + + if (ruleContext.hasErrors()) { + return null; + } NestedSet transitiveProtoSources = - computeTransitiveProtoSources(ruleContext, directProtoSources); + computeTransitiveProtoSources(ruleContext, library.getSources()); NestedSet transitiveProtoSourceRoots = - computeTransitiveProtoSourceRoots(ruleContext, protoSourceRoot); + computeTransitiveProtoSourceRoots(ruleContext, library.getSourceRoot()); NestedSet strictImportableProtosForDependents = - computeStrictImportableProtosForDependents(ruleContext, directProtoSources); + computeStrictImportableProtosForDependents(ruleContext, library.getSources()); NestedSet strictImportableProtos = computeStrictImportableProtos(ruleContext); NestedSet strictImportableProtoSourceRoots = - computeStrictImportableProtoSourceRoots(ruleContext, protoSourceRoot); + computeStrictImportableProtoSourceRoots(ruleContext, library.getSourceRoot()); NestedSet exportedProtos = ProtoCommon.computeExportedProtos(ruleContext); NestedSet exportedProtoSourceRoots = - computeExportedProtoSourceRoots(ruleContext, protoSourceRoot); + computeExportedProtoSourceRoots(ruleContext, library.getSourceRoot()); Artifact directDescriptorSet = ruleContext.getGenfilesArtifact( @@ -251,8 +394,8 @@ public static ProtoInfo createProtoInfo(RuleContext ruleContext) { ProtoInfo protoInfo = ProtoInfo.create( - directProtoSources, - protoSourceRoot, + library.getSources(), + library.getSourceRoot(), transitiveProtoSources, transitiveProtoSourceRoots, strictImportableProtosForDependents, diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java index f7138d6b22bfe2..e49c55d2c51f29 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java @@ -257,6 +257,10 @@ private SpawnAction.Builder createAction() throws MissingPrerequisiteException { return result; } + private static String getOutputDirectory(RuleContext ruleContext) { + return ruleContext.getBinDirectory().getExecPath().getSegment(0); + } + @Nullable private FilesToRunProvider getLangPluginTarget() throws MissingPrerequisiteException { if (langPluginName == null) { @@ -301,6 +305,7 @@ CustomCommandLine.Builder createProtoCompilerCommandLine() throws MissingPrerequ // Add include maps addIncludeMapArguments( + getOutputDirectory(ruleContext), result, areDepsStrict ? protoInfo.getStrictImportableProtoSources() : null, protoInfo.getStrictImportableProtoSourceRoots(), @@ -329,7 +334,10 @@ CustomCommandLine.Builder createProtoCompilerCommandLine() throws MissingPrerequ "--allowed_public_imports", VectorArg.join(":") .each(protosInExports) - .mapped(new ExpandToPathFn(protoInfo.getTransitiveProtoSourceRoots()))); + .mapped( + new ExpandToPathFn( + getOutputDirectory(ruleContext), + protoInfo.getTransitiveProtoSourceRoots()))); } } @@ -481,6 +489,7 @@ private static SpawnAction.Builder createActions( .addCommandLine( createCommandLineFromToolchains( toolchainInvocations, + getOutputDirectory(ruleContext), protoInfo, ruleLabel, areDepsStrict(ruleContext) ? Deps.STRICT : Deps.NON_STRICT, @@ -521,6 +530,7 @@ public static boolean arePublicImportsStrict(RuleContext ruleContext) { @VisibleForTesting static CustomCommandLine createCommandLineFromToolchains( List toolchainInvocations, + String outputDirectory, ProtoInfo protoInfo, Label ruleLabel, Deps strictDeps, @@ -567,6 +577,7 @@ static CustomCommandLine createCommandLineFromToolchains( // Add include maps addIncludeMapArguments( + outputDirectory, cmdLine, strictDeps == Deps.STRICT ? protoInfo.getStrictImportableProtoSources() : null, protoInfo.getStrictImportableProtoSourceRoots(), @@ -585,7 +596,8 @@ static CustomCommandLine createCommandLineFromToolchains( "--allowed_public_imports", VectorArg.join(":") .each(protoInfo.getExportedProtoSources()) - .mapped(new ExpandToPathFn(protoInfo.getExportedProtoSourceRoots()))); + .mapped( + new ExpandToPathFn(outputDirectory, protoInfo.getExportedProtoSourceRoots()))); } } @@ -602,6 +614,7 @@ static CustomCommandLine createCommandLineFromToolchains( @VisibleForTesting static void addIncludeMapArguments( + String outputDirectory, CustomCommandLine.Builder commandLine, @Nullable NestedSet protosInDirectDependencies, NestedSet directProtoSourceRoots, @@ -610,14 +623,15 @@ static void addIncludeMapArguments( // protoSourceRoot. This ensures that protos can reference either the full path or the short // path when including other protos. commandLine.addAll( - VectorArg.of(transitiveImports).mapped(new ExpandImportArgsFn(directProtoSourceRoots))); + VectorArg.of(transitiveImports) + .mapped(new ExpandImportArgsFn(outputDirectory, directProtoSourceRoots))); if (protosInDirectDependencies != null) { if (!protosInDirectDependencies.isEmpty()) { commandLine.addAll( "--direct_dependencies", VectorArg.join(":") .each(protosInDirectDependencies) - .mapped(new ExpandToPathFn(directProtoSourceRoots))); + .mapped(new ExpandToPathFn(outputDirectory, directProtoSourceRoots))); } else { // The proto compiler requires an empty list to turn on strict deps checking @@ -626,6 +640,29 @@ static void addIncludeMapArguments( } } + private static String guessProtoPathUnderRoot( + String outputDirectory, PathFragment sourceRootPath, Artifact proto) { + // TODO(lberki): Instead of guesswork like this, we should track which proto belongs to + // which source root. Unfortunately, that's a non-trivial migration since + // ProtoInfo is on the Starlark API. Therefore, we hack: + // - If the source root is under the output directory (itself determined in a hacky way and + // relying on the fact that the output roots of all repositories are under the same directory + // under the exec root), we check whether the .proto file is under it. If so, we have a match. + // - Otherwise, we check whether the .proto file is either under that source directory or under + // bin or genfiles by prefix-matching its root-relative path. + if (sourceRootPath.segmentCount() > 0 && sourceRootPath.getSegment(0).equals(outputDirectory)) { + if (proto.getExecPath().startsWith(sourceRootPath)) { + return proto.getExecPath().relativeTo(sourceRootPath).getPathString(); + } + } else { + if (proto.getRootRelativePath().startsWith(sourceRootPath)) { + return proto.getRootRelativePath().relativeTo(sourceRootPath).getPathString(); + } + } + + return null; + } + @AutoCodec @AutoCodec.VisibleForSerialization static final CommandLineItem.MapFn EXPAND_TRANSITIVE_PROTO_PATH_FLAGS = (flag, args) -> { @@ -634,12 +671,15 @@ static void addIncludeMapArguments( } }; + @AutoCodec @AutoCodec.VisibleForSerialization static final class ExpandImportArgsFn implements CapturingMapFn { + private final String outputDirectory; private final NestedSet directProtoSourceRoots; - public ExpandImportArgsFn(NestedSet directProtoSourceRoots) { + public ExpandImportArgsFn(String outputDirectory, NestedSet directProtoSourceRoots) { + this.outputDirectory = outputDirectory; this.directProtoSourceRoots = directProtoSourceRoots; } @@ -654,12 +694,9 @@ public void expandToCommandLine(Artifact proto, Consumer args) { String pathIgnoringRepository = getPathIgnoringRepository(proto); for (String directProtoSourceRoot : directProtoSourceRoots) { - // TODO(lberki): Instead of guesswork like this, we should track which proto belongs to - // which source root. Unfortunately, that's a non-trivial migration since - // ProtoInfo is on the Starlark API. PathFragment sourceRootPath = PathFragment.create(directProtoSourceRoot); - if (proto.getRootRelativePath().startsWith(sourceRootPath)) { - String arg = proto.getRootRelativePath().relativeTo(sourceRootPath).getPathString(); + String arg = guessProtoPathUnderRoot(outputDirectory, sourceRootPath, proto); + if (arg != null) { if (arg.equals(pathIgnoringRepository)) { repositoryPathAdded = true; } @@ -678,9 +715,11 @@ public void expandToCommandLine(Artifact proto, Consumer args) { @AutoCodec @AutoCodec.VisibleForSerialization static final class ExpandToPathFn implements CapturingMapFn { + private final String outputDirectory; private final NestedSet directProtoSourceRoots; - public ExpandToPathFn(NestedSet directProtoSourceRoots) { + public ExpandToPathFn(String outputDirectory, NestedSet directProtoSourceRoots) { + this.outputDirectory = outputDirectory; this.directProtoSourceRoots = directProtoSourceRoots; } @@ -691,11 +730,8 @@ public void expandToCommandLine(Artifact proto, Consumer args) { for (String directProtoSourceRoot : directProtoSourceRoots) { PathFragment sourceRootPath = PathFragment.create(directProtoSourceRoot); - // TODO(lberki): Instead of guesswork like this, we should track which proto belongs to - // which source root. Unfortunately, that's a non-trivial migration since - // ProtoInfo is on the Starlark API. - if (proto.getRootRelativePath().startsWith(sourceRootPath)) { - String arg = proto.getRootRelativePath().relativeTo(sourceRootPath).getPathString(); + String arg = guessProtoPathUnderRoot(outputDirectory, sourceRootPath, proto); + if (arg != null) { if (arg.equals(pathIgnoringRepository)) { repositoryPathAdded = true; } diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD index 89da2191d1d47c..e09849b44e91de 100644 --- a/src/test/java/com/google/devtools/build/lib/BUILD +++ b/src/test/java/com/google/devtools/build/lib/BUILD @@ -1552,6 +1552,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib:build-base", "//src/main/java/com/google/devtools/build/lib:proto-rules", "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/vfs", ], ) diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java index 8be039ee40faf8..4a7a51fe81d882 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java @@ -24,6 +24,7 @@ import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.actions.FileWriteAction; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; +import com.google.devtools.build.lib.vfs.FileSystemUtils; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -32,6 +33,9 @@ /** Unit tests for {@code proto_library}. */ @RunWith(JUnit4.class) public class BazelProtoLibraryTest extends BuildViewTestCase { + private boolean isThisBazel() { + return getAnalysisMock().isThisBazel(); + } @Before public void setUp() throws Exception { @@ -385,6 +389,254 @@ public void testProtoSourceRoot() throws Exception { assertThat(sourcesProvider.getDirectProtoSourceRoot()).isEqualTo("x/foo"); } + @Test + public void testProtoSourceRootWithImportPrefix() throws Exception { + if (!isThisBazel()) { + return; + } + + scratch.file( + "a/BUILD", + "proto_library(", + " name = 'a',", + " srcs = ['a.proto'],", + " proto_source_root = 'a',", + " import_prefix = 'foo')"); + + reporter.removeHandler(failFastHandler); + getConfiguredTarget("//a"); + assertContainsEvent("the 'proto_source_root' attribute is incompatible"); + } + + @Test + public void testProtoSourceRootWithStripImportPrefix() throws Exception { + if (!isThisBazel()) { + return; + } + + scratch.file( + "a/BUILD", + "proto_library(", + " name = 'a',", + " srcs = ['a.proto'],", + " proto_source_root = 'a',", + " strip_import_prefix = 'a')"); + + reporter.removeHandler(failFastHandler); + getConfiguredTarget("//a"); + assertContainsEvent("the 'proto_source_root' attribute is incompatible"); + } + + @Test + public void testIllegalStripImportPrefix() throws Exception { + if (!isThisBazel()) { + return; + } + + scratch.file( + "a/BUILD", + "proto_library(", + " name = 'a',", + " srcs = ['a.proto'],", + " strip_import_prefix = 'foo')"); + + reporter.removeHandler(failFastHandler); + getConfiguredTarget("//a"); + assertContainsEvent(".proto file 'a/a.proto' is not under the specified strip prefix"); + } + + @Test + public void testIllegalImportPrefix() throws Exception { + if (!isThisBazel()) { + return; + } + + scratch.file( + "a/BUILD", + "proto_library(", + " name = 'a',", + " srcs = ['a.proto'],", + " import_prefix = '/foo')"); + + reporter.removeHandler(failFastHandler); + getConfiguredTarget("//a"); + assertContainsEvent("should be a relative path"); + } + + @Test + public void testRelativeStripImportPrefix() throws Exception { + if (!isThisBazel()) { + return; + } + + scratch.file( + "a/b/BUILD", + "proto_library(", + " name = 'd',", + " srcs = ['c/d.proto'],", + " strip_import_prefix = 'c')"); + + Iterable commandLine = paramFileArgsForAction(getDescriptorWriteAction("//a/b:d")); + String genfiles = getTargetConfiguration().getGenfilesFragment().toString(); + assertThat(commandLine).contains("-Id.proto=" + genfiles + "/a/b/_virtual_imports/d/d.proto"); + } + + @Test + public void testAbsoluteStripImportPrefix() throws Exception { + if (!isThisBazel()) { + return; + } + + scratch.file( + "a/b/BUILD", + "proto_library(", + " name = 'd',", + " srcs = ['c/d.proto'],", + " strip_import_prefix = '/a')"); + + Iterable commandLine = paramFileArgsForAction(getDescriptorWriteAction("//a/b:d")); + String genfiles = getTargetConfiguration().getGenfilesFragment().toString(); + assertThat(commandLine) + .contains("-Ib/c/d.proto=" + genfiles + "/a/b/_virtual_imports/d/b/c/d.proto"); + } + + @Test + public void testStripImportPrefixAndImportPrefix() throws Exception { + if (!isThisBazel()) { + return; + } + + scratch.file( + "a/b/BUILD", + "proto_library(", + " name = 'd',", + " srcs = ['c/d.proto'],", + " import_prefix = 'foo',", + " strip_import_prefix = 'c')"); + + Iterable commandLine = paramFileArgsForAction(getDescriptorWriteAction("//a/b:d")); + String genfiles = getTargetConfiguration().getGenfilesFragment().toString(); + assertThat(commandLine) + .contains("-Ifoo/d.proto=" + genfiles + "/a/b/_virtual_imports/d/foo/d.proto"); + } + + @Test + public void testImportPrefixWithoutStripImportPrefix() throws Exception { + if (!isThisBazel()) { + return; + } + + scratch.file( + "a/b/BUILD", + "proto_library(", + " name = 'd',", + " srcs = ['c/d.proto'],", + " import_prefix = 'foo')"); + + Iterable commandLine = paramFileArgsForAction(getDescriptorWriteAction("//a/b:d")); + String genfiles = getTargetConfiguration().getGenfilesFragment().toString(); + assertThat(commandLine) + .contains("-Ifoo/a/b/c/d.proto=" + genfiles + "/a/b/_virtual_imports/d/foo/a/b/c/d.proto"); + } + + @Test + public void testDotInStripImportPrefix() throws Exception { + if (!isThisBazel()) { + return; + } + + scratch.file( + "a/b/BUILD", + "proto_library(", + " name = 'd',", + " srcs = ['c/d.proto'],", + " strip_import_prefix = './c')"); + + reporter.removeHandler(failFastHandler); + getConfiguredTarget("//a/b:d"); + assertContainsEvent("should be normalized"); + } + + @Test + public void testDotDotInStripImportPrefix() throws Exception { + if (!isThisBazel()) { + return; + } + + scratch.file( + "a/b/BUILD", + "proto_library(", + " name = 'd',", + " srcs = ['c/d.proto'],", + " strip_import_prefix = '../b/c')"); + + reporter.removeHandler(failFastHandler); + getConfiguredTarget("//a/b:d"); + assertContainsEvent("should be normalized"); + } + + @Test + public void testDotInImportPrefix() throws Exception { + if (!isThisBazel()) { + return; + } + + scratch.file( + "a/b/BUILD", + "proto_library(", + " name = 'd',", + " srcs = ['c/d.proto'],", + " import_prefix = './e')"); + + reporter.removeHandler(failFastHandler); + getConfiguredTarget("//a/b:d"); + assertContainsEvent("should be normalized"); + } + + @Test + public void testDotDotInImportPrefix() throws Exception { + if (!isThisBazel()) { + return; + } + + scratch.file( + "a/b/BUILD", + "proto_library(", + " name = 'd',", + " srcs = ['c/d.proto'],", + " import_prefix = '../e')"); + + reporter.removeHandler(failFastHandler); + getConfiguredTarget("//a/b:d"); + assertContainsEvent("should be normalized"); + } + + @Test + public void testStripImportPrefixForExternalRepositories() throws Exception { + if (!isThisBazel()) { + return; + } + + FileSystemUtils.appendIsoLatin1( + scratch.resolve("WORKSPACE"), "local_repository(name = 'foo', path = '/foo')"); + invalidatePackages(); + + scratch.file("/foo/WORKSPACE"); + scratch.file( + "/foo/x/y/BUILD", + "proto_library(", + " name = 'q',", + " srcs = ['z/q.proto'],", + " strip_import_prefix = '/x')"); + + scratch.file("a/BUILD", "proto_library(name='a', srcs=['a.proto'], deps=['@foo//x/y:q'])"); + + Iterable commandLine = paramFileArgsForAction(getDescriptorWriteAction("//a:a")); + String genfiles = getTargetConfiguration().getGenfilesFragment().toString(); + assertThat(commandLine) + .contains("-Iy/z/q.proto=" + genfiles + "/external/foo/x/y/_virtual_imports/q/y/z/q.proto"); + } + private Artifact getDescriptorOutput(String label) throws Exception { return getFirstArtifactEndingWith(getFilesToBuild(getConfiguredTarget(label)), ".proto.bin"); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java index fd368fb51de02a..ac0b39421bd0c9 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java @@ -101,6 +101,7 @@ public void commandLine_basic() throws Exception { new ToolchainInvocation( "dontcare_because_no_plugin", toolchainNoPlugin, "foo.srcjar"), new ToolchainInvocation("pluginName", toolchainWithPlugin, "bar.srcjar")), + "bazel-out", protoInfo( /* directProtos */ ImmutableList.of(artifact("//:dont-care", "source_file.proto")), /* transitiveProtos */ NestedSetBuilder.create( @@ -135,6 +136,7 @@ public void commandline_derivedArtifact() { CustomCommandLine cmdLine = createCommandLineFromToolchains( /* toolchainInvocations= */ ImmutableList.of(), + "bazel-out", protoInfo( /* directProtos */ ImmutableList.of( derivedArtifact("//:dont-care", "source_file.proto")), @@ -164,6 +166,7 @@ public void commandLine_strictDeps() throws Exception { CustomCommandLine cmdLine = createCommandLineFromToolchains( ImmutableList.of(new ToolchainInvocation("dontcare", toolchain, "foo.srcjar")), + "bazel-out", protoInfo( /* directProtos */ ImmutableList.of(artifact("//:dont-care", "source_file.proto")), /* transitiveProtos */ NestedSetBuilder.create( @@ -197,6 +200,7 @@ public void otherParameters() throws Exception { CustomCommandLine cmdLine = createCommandLineFromToolchains( ImmutableList.of(), + "bazel-out", protoInfo( /* directProtos */ ImmutableList.of(), /* transitiveProtos */ NestedSetBuilder.emptySet(STABLE_ORDER), @@ -236,6 +240,7 @@ public String toString() { CustomCommandLine cmdLine = createCommandLineFromToolchains( ImmutableList.of(new ToolchainInvocation("pluginName", toolchain, outReplacement)), + "bazel-out", protoInfo( /* directProtos*/ ImmutableList.of(), /* transitiveProtos */ NestedSetBuilder.emptySet(STABLE_ORDER), @@ -278,6 +283,7 @@ public void exceptionIfSameName() throws Exception { ImmutableList.of( new ToolchainInvocation("pluginName", toolchain1, "outReplacement"), new ToolchainInvocation("pluginName", toolchain2, "outReplacement")), + "bazel-out", protoInfo( /* directProtos */ ImmutableList.of(), /* transitiveProtos */ NestedSetBuilder.emptySet(STABLE_ORDER), @@ -386,6 +392,7 @@ private static Iterable protoArgv( NestedSet transitiveImportsNestedSet = NestedSetBuilder.wrap(STABLE_ORDER, transitiveImports); ProtoCompileActionBuilder.addIncludeMapArguments( + "blaze-out", commandLine, protosInDirectDependenciesBuilder, NestedSetBuilder.wrap(Order.STABLE_ORDER, protoSourceRoots), diff --git a/src/test/shell/bazel/bazel_proto_library_test.sh b/src/test/shell/bazel/bazel_proto_library_test.sh index 32c093cb217929..04784abe0ad7aa 100755 --- a/src/test/shell/bazel/bazel_proto_library_test.sh +++ b/src/test/shell/bazel/bazel_proto_library_test.sh @@ -432,4 +432,81 @@ function test_proto_source_root_multiple_workspaces() { bazel build @main_repo//src:all_protos >& "$TEST_log" || fail "Expected success" } +function test_import_prefix_stripping() { + mkdir -p e + touch e/WORKSPACE + write_workspace "" + + cat >> WORKSPACE < e/f/BUILD < e/f/bad/f.proto < g/BUILD << EOF +proto_library( + name = 'g', + strip_import_prefix = "/g/bad", + import_prefix = "good", + srcs = ['bad/g.proto'], + visibility = ["//visibility:public"], +) +EOF + + cat > g/bad/g.proto < h/BUILD < h/h.proto <