From d790ba53444fb8a0676093e709d8d0ecc8265ead Mon Sep 17 00:00:00 2001 From: Marcel Hlopko Date: Fri, 28 Jun 2019 08:40:00 -0700 Subject: [PATCH] Always strip /external/ from proto import path Before this PR, when strip_import_prefix was not specified, and the proto target was declared in an external repository, Bazel generated import path that leaked `external/` path fragment. That path fragment is an implementation detail of Bazel's execroot layout and should not be used in proto source files. This PR adds a correct path fragment to the import paths. It also introduces an incompatible flag `--incompatible_do_not_emit_buggy_external_repo_import` that removes the incorrect import path from Bazel. Fixes https://github.com/bazelbuild/bazel/issues/8030. Incompatible change: https://github.com/bazelbuild/bazel/issues/8711. Closes #8683. PiperOrigin-RevId: 255606742 --- .../lib/rules/cpp/proto/CcProtoAspect.java | 5 + .../build/lib/rules/proto/ProtoCommon.java | 74 +++++-- .../lib/rules/proto/ProtoConfiguration.java | 21 ++ .../java/com/google/devtools/build/lib/BUILD | 1 + .../rules/cpp/proto/CcProtoLibraryTest.java | 88 ++++++++ .../rules/proto/BazelProtoLibraryTest.java | 197 ++++++++++++++++++ 6 files changed, 363 insertions(+), 23 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java index 803506fc00cb21..07becfbf7f6f74 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java @@ -324,6 +324,11 @@ private CcCompilationHelper initializeCompilationHelper( if (protoRootFragment.startsWith(binOrGenfiles)) { protoRootFragment = protoRootFragment.relativeTo(binOrGenfiles); } + PathFragment repositoryPath = + ruleContext.getLabel().getPackageIdentifier().getRepository().getPathUnderExecRoot(); + if (protoRootFragment.startsWith(repositoryPath)) { + protoRootFragment = protoRootFragment.relativeTo(repositoryPath); + } String stripIncludePrefix = PathFragment.create("//").getRelative(protoRootFragment).toString(); 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 1148875e983936..1463bbfc341129 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 @@ -245,10 +245,11 @@ private static PathFragment getPathFragmentAttribute( */ private static Library createVirtualImportDirectoryMaybe( RuleContext ruleContext, ImmutableList protoSources) { - PathFragment importPrefix = getPathFragmentAttribute(ruleContext, "import_prefix"); - PathFragment stripImportPrefix = getPathFragmentAttribute(ruleContext, "strip_import_prefix"); + PathFragment importPrefixAttribute = getPathFragmentAttribute(ruleContext, "import_prefix"); + PathFragment stripImportPrefixAttribute = + getPathFragmentAttribute(ruleContext, "strip_import_prefix"); - if (importPrefix == null && stripImportPrefix == null) { + if (importPrefixAttribute == null && stripImportPrefixAttribute == null) { // Simple case, no magic required. return null; } @@ -260,22 +261,26 @@ private static Library createVirtualImportDirectoryMaybe( return null; } - if (stripImportPrefix == null) { - stripImportPrefix = PathFragment.EMPTY_FRAGMENT; - } else if (stripImportPrefix.isAbsolute()) { + PathFragment stripImportPrefix; + if (stripImportPrefixAttribute == null) { + stripImportPrefix = PathFragment.create(ruleContext.getLabel().getWorkspaceRoot()); + } else if (stripImportPrefixAttribute.isAbsolute()) { stripImportPrefix = ruleContext .getLabel() .getPackageIdentifier() .getRepository() .getSourceRoot() - .getRelative(stripImportPrefix.toRelative()); + .getRelative(stripImportPrefixAttribute.toRelative()); } else { - stripImportPrefix = ruleContext.getPackageDirectory().getRelative(stripImportPrefix); + stripImportPrefix = ruleContext.getPackageDirectory().getRelative(stripImportPrefixAttribute); } - if (importPrefix == null) { + PathFragment importPrefix; + if (importPrefixAttribute == null) { importPrefix = PathFragment.EMPTY_FRAGMENT; + } else { + importPrefix = importPrefixAttribute; } if (importPrefix.isAbsolute()) { @@ -296,12 +301,44 @@ private static Library createVirtualImportDirectoryMaybe( realProtoSource.getExecPathString(), stripImportPrefix.getPathString())); continue; } + Pair importsPair = + computeImports( + ruleContext, realProtoSource, sourceRootPath, importPrefix, stripImportPrefix); + protoSourceImportPair.add(new Pair<>(realProtoSource, importsPair.first.toString())); + symlinks.add(importsPair.second); + + if (!ruleContext.getFragment(ProtoConfiguration.class).doNotUseBuggyImportPath() + && stripImportPrefixAttribute == null) { + Pair oldImportsPair = + computeImports( + ruleContext, + realProtoSource, + sourceRootPath, + importPrefix, + PathFragment.EMPTY_FRAGMENT); + protoSourceImportPair.add(new Pair<>(realProtoSource, oldImportsPair.first.toString())); + symlinks.add(oldImportsPair.second); + } + } - PathFragment importPath = - importPrefix.getRelative( - realProtoSource.getRootRelativePath().relativeTo(stripImportPrefix)); + String sourceRoot = + ruleContext + .getBinOrGenfilesDirectory() + .getExecPath() + .getRelative(sourceRootPath) + .getPathString(); + return new Library(symlinks.build(), sourceRoot, protoSourceImportPair.build()); + } - protoSourceImportPair.add(new Pair<>(realProtoSource, importPath.toString())); + private static Pair computeImports( + RuleContext ruleContext, + Artifact realProtoSource, + PathFragment sourceRootPath, + PathFragment importPrefix, + PathFragment stripImportPrefix) { + PathFragment importPath = + importPrefix.getRelative( + realProtoSource.getRootRelativePath().relativeTo(stripImportPrefix)); Artifact virtualProtoSource = ruleContext.getDerivedArtifact( @@ -314,16 +351,7 @@ private static Library createVirtualImportDirectoryMaybe( 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, protoSourceImportPair.build()); + return Pair.of(importPath, virtualProtoSource); } /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java index 6dfeeb40a274ec..a2650f00205475 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java @@ -40,6 +40,22 @@ public class ProtoConfiguration extends Fragment implements ProtoConfigurationApi { /** Command line options. */ public static class Options extends FragmentOptions { + @Option( + name = "incompatible_do_not_emit_buggy_external_repo_import", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.TOOLCHAIN, + effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, + metadataTags = { + OptionMetadataTag.INCOMPATIBLE_CHANGE, + OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES + }, + help = + "If true, Bazel will not emit import paths for external repos that used to be emitted" + + " because of https://github.com/bazelbuild/bazel/issues/8030. This is now fixed" + + " and those imports are only emitted for backwards compatibility. See" + + " https://github.com/bazelbuild/bazel/issues/8711 for details.") + public boolean doNotUseBuggyImportPath; + @Option( name = "protocopt", allowMultiple = true, @@ -193,6 +209,7 @@ public FragmentOptions getHost() { host.ccProtoLibrarySourceSuffixes = ccProtoLibrarySourceSuffixes; host.experimentalJavaProtoAddAllowedPublicImports = experimentalJavaProtoAddAllowedPublicImports; + host.doNotUseBuggyImportPath = doNotUseBuggyImportPath; return host; } } @@ -286,4 +303,8 @@ public List ccProtoLibrarySourceSuffixes() { public boolean strictPublicImports() { return options.experimentalJavaProtoAddAllowedPublicImports; } + + public boolean doNotUseBuggyImportPath() { + return options.doNotUseBuggyImportPath; + } } diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD index 603931e2b05238..fbf4d421a713a4 100644 --- a/src/test/java/com/google/devtools/build/lib/BUILD +++ b/src/test/java/com/google/devtools/build/lib/BUILD @@ -1497,6 +1497,7 @@ java_test( ":guava_junit_truth", "//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:util", "//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/cpp/proto/CcProtoLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibraryTest.java index 3a7d98d418ed12..eba60ec2d0b482 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibraryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoLibraryTest.java @@ -19,12 +19,14 @@ import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.prettyArtifactNames; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Iterables; import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.bazel.rules.cpp.proto.BazelCcProtoAspect; +import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.packages.util.Crosstool.CcToolchainConfig; import com.google.devtools.build.lib.rules.cpp.CcCompilationContext; @@ -41,6 +43,7 @@ @RunWith(JUnit4.class) public class CcProtoLibraryTest extends BuildViewTestCase { + @Before public void setUp() throws Exception { mockToolsConfig.create("/protobuf/WORKSPACE"); @@ -243,4 +246,89 @@ public void generatedSourcesNotCoverageInstrumented() throws Exception { assertThat(options).doesNotContain("-fprofile-arcs"); assertThat(options).doesNotContain("-ftest-coverage"); } + + @Test + public void importPrefixWorksWithRepositories() throws Exception { + FileSystemUtils.appendIsoLatin1( + scratch.resolve("WORKSPACE"), "local_repository(name = 'yolo_repo', path = '/yolo_repo')"); + invalidatePackages(); + useConfiguration("--incompatible_do_not_emit_buggy_external_repo_import"); + + scratch.file("/yolo_repo/WORKSPACE"); + scratch.file("/yolo_repo/yolo_pkg/yolo.proto"); + scratch.file( + "/yolo_repo/yolo_pkg/BUILD", + "proto_library(", + " name = 'yolo_proto',", + " srcs = ['yolo.proto'],", + " import_prefix = 'bazel.build/yolo',", + ")", + "cc_proto_library(", + " name = 'yolo_cc_proto',", + " deps = [':yolo_proto'],", + ")"); + assertThat(getTarget("@yolo_repo//yolo_pkg:yolo_cc_proto")).isNotNull(); + assertThat(getProtoHeaderExecPath()) + .endsWith("_virtual_includes/yolo_proto/bazel.build/yolo/yolo_pkg/yolo.pb.h"); + } + + @Test + public void stripImportPrefixWorksWithRepositories() throws Exception { + FileSystemUtils.appendIsoLatin1( + scratch.resolve("WORKSPACE"), "local_repository(name = 'yolo_repo', path = '/yolo_repo')"); + invalidatePackages(); + useConfiguration("--incompatible_do_not_emit_buggy_external_repo_import"); + + scratch.file("/yolo_repo/WORKSPACE"); + scratch.file("/yolo_repo/yolo_pkg/yolo.proto"); + scratch.file( + "/yolo_repo/yolo_pkg/BUILD", + "proto_library(", + " name = 'yolo_proto',", + " srcs = ['yolo.proto'],", + " strip_import_prefix = '/yolo_pkg',", + ")", + "cc_proto_library(", + " name = 'yolo_cc_proto',", + " deps = [':yolo_proto'],", + ")"); + assertThat(getTarget("@yolo_repo//yolo_pkg:yolo_cc_proto")).isNotNull(); + assertThat(getProtoHeaderExecPath()).endsWith("_virtual_includes/yolo_proto/yolo.pb.h"); + } + + @Test + public void importPrefixAndStripImportPrefixWorksWithRepositories() throws Exception { + FileSystemUtils.appendIsoLatin1( + scratch.resolve("WORKSPACE"), "local_repository(name = 'yolo_repo', path = '/yolo_repo')"); + invalidatePackages(); + useConfiguration("--incompatible_do_not_emit_buggy_external_repo_import"); + + scratch.file("/yolo_repo/WORKSPACE"); + scratch.file("/yolo_repo/yolo_pkg/yolo.proto"); + scratch.file( + "/yolo_repo/yolo_pkg/BUILD", + "proto_library(", + " name = 'yolo_proto',", + " srcs = ['yolo.proto'],", + " import_prefix = 'bazel.build/yolo',", + " strip_import_prefix = '/yolo_pkg'", + ")", + "cc_proto_library(", + " name = 'yolo_cc_proto',", + " deps = [':yolo_proto'],", + ")"); + getTarget("@yolo_repo//yolo_pkg:yolo_cc_proto"); + + assertThat(getTarget("@yolo_repo//yolo_pkg:yolo_cc_proto")).isNotNull(); + assertThat(getProtoHeaderExecPath()) + .endsWith("_virtual_includes/yolo_proto/bazel.build/yolo/yolo.pb.h"); + } + + private String getProtoHeaderExecPath() throws LabelSyntaxException { + ConfiguredTarget configuredTarget = getConfiguredTarget("@yolo_repo//yolo_pkg:yolo_cc_proto"); + CcInfo ccInfo = configuredTarget.get(CcInfo.PROVIDER); + ImmutableList headers = + ccInfo.getCcCompilationContext().getDeclaredIncludeSrcs().toList(); + return Iterables.getOnlyElement(headers).getExecPathString(); + } } 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 0994aa533fab75..225134e5187910 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.util.Pair; import com.google.devtools.build.lib.vfs.FileSystemUtils; import org.junit.Before; import org.junit.Test; @@ -433,6 +434,202 @@ public void testProtoSourceRootWithStripImportPrefix() throws Exception { assertContainsEvent("the 'proto_source_root' attribute is incompatible"); } + @Test + public void testImportPrefixInExternalRepoLegacyBehavior() throws Exception { + if (!isThisBazel()) { + return; + } + + FileSystemUtils.appendIsoLatin1( + scratch.resolve("WORKSPACE"), "local_repository(name = 'yolo_repo', path = '/yolo_repo')"); + invalidatePackages(); + useConfiguration("noincompatible_do_not_emit_buggy_external_repo_import"); + + scratch.file("/yolo_repo/WORKSPACE"); + scratch.file("/yolo_repo/yolo_pkg/yolo.proto"); + scratch.file( + "/yolo_repo/yolo_pkg/BUILD", + "proto_library(", + " name = 'yolo_proto',", + " srcs = ['yolo.proto'],", + " import_prefix = 'bazel.build/yolo',", + " visibility = ['//visibility:public'],", + ")"); + + scratch.file( + "main.proto", "syntax = 'proto3'';", "import 'bazel.build/yolo/yolo_pkg/yolo.proto';"); + scratch.file( + "BUILD", + "proto_library(", + " name = 'main_proto',", + " srcs = ['main.proto'],", + " deps = ['@yolo_repo//yolo_pkg:yolo_proto'],", + ")"); + + ConfiguredTarget main = getConfiguredTarget("//:main_proto"); + ProtoInfo protoInfo = main.get(ProtoInfo.PROVIDER); + ImmutableList> importPaths = + protoInfo.getStrictImportableProtoSourcesImportPaths().toList(); + assertThat(importPaths).isNotEmpty(); + assertThat(importPaths.get(1).second) + .isEqualTo("bazel.build/yolo/external/yolo_repo/yolo_pkg/yolo.proto"); + } + + @Test + public void testImportPrefixInExternalRepo() throws Exception { + if (!isThisBazel()) { + return; + } + + FileSystemUtils.appendIsoLatin1( + scratch.resolve("WORKSPACE"), "local_repository(name = 'yolo_repo', path = '/yolo_repo')"); + invalidatePackages(); + + scratch.file("/yolo_repo/WORKSPACE"); + scratch.file("/yolo_repo/yolo_pkg/yolo.proto"); + scratch.file( + "/yolo_repo/yolo_pkg/BUILD", + "proto_library(", + " name = 'yolo_proto',", + " srcs = ['yolo.proto'],", + " import_prefix = 'bazel.build/yolo',", + " visibility = ['//visibility:public'],", + ")"); + + scratch.file( + "main.proto", "syntax = 'proto3'';", "import 'bazel.build/yolo/yolo_pkg/yolo.proto';"); + scratch.file( + "BUILD", + "proto_library(", + " name = 'main_proto',", + " srcs = ['main.proto'],", + " deps = ['@yolo_repo//yolo_pkg:yolo_proto'],", + ")"); + + ConfiguredTarget main = getConfiguredTarget("//:main_proto"); + ProtoInfo protoInfo = main.get(ProtoInfo.PROVIDER); + ImmutableList> importPaths = + protoInfo.getStrictImportableProtoSourcesImportPaths().toList(); + assertThat(importPaths).isNotEmpty(); + assertThat(importPaths.get(0).second).isEqualTo("bazel.build/yolo/yolo_pkg/yolo.proto"); + } + + @Test + public void testImportPrefixAndStripInExternalRepo() throws Exception { + if (!isThisBazel()) { + return; + } + + FileSystemUtils.appendIsoLatin1( + scratch.resolve("WORKSPACE"), "local_repository(name = 'yolo_repo', path = '/yolo_repo')"); + invalidatePackages(); + + scratch.file("/yolo_repo/WORKSPACE"); + scratch.file("/yolo_repo/yolo_pkg_to_be_stripped/yolo_pkg/yolo.proto"); + scratch.file( + "/yolo_repo/yolo_pkg_to_be_stripped/yolo_pkg/BUILD", + "proto_library(", + " name = 'yolo_proto',", + " srcs = ['yolo.proto'],", + " import_prefix = 'bazel.build/yolo',", + " strip_import_prefix = '/yolo_pkg_to_be_stripped',", + " visibility = ['//visibility:public'],", + ")"); + + scratch.file( + "main.proto", "syntax = 'proto3'';", "import 'bazel.build/yolo/yolo_pkg/yolo.proto';"); + scratch.file( + "BUILD", + "proto_library(", + " name = 'main_proto',", + " srcs = ['main.proto'],", + " deps = ['@yolo_repo//yolo_pkg_to_be_stripped/yolo_pkg:yolo_proto'],", + ")"); + + ConfiguredTarget main = getConfiguredTarget("//:main_proto"); + ProtoInfo protoInfo = main.get(ProtoInfo.PROVIDER); + ImmutableList> importPaths = + protoInfo.getStrictImportableProtoSourcesImportPaths().toList(); + assertThat(importPaths).isNotEmpty(); + assertThat(importPaths.get(0).second).isEqualTo("bazel.build/yolo/yolo_pkg/yolo.proto"); + } + + @Test + public void testStripImportPrefixInExternalRepo() throws Exception { + if (!isThisBazel()) { + return; + } + + FileSystemUtils.appendIsoLatin1( + scratch.resolve("WORKSPACE"), "local_repository(name = 'yolo_repo', path = '/yolo_repo')"); + invalidatePackages(); + + scratch.file("/yolo_repo/WORKSPACE"); + scratch.file("/yolo_repo/yolo_pkg_to_be_stripped/yolo_pkg/yolo.proto"); + scratch.file( + "/yolo_repo/yolo_pkg_to_be_stripped/yolo_pkg/BUILD", + "proto_library(", + " name = 'yolo_proto',", + " srcs = ['yolo.proto'],", + " strip_import_prefix = '/yolo_pkg_to_be_stripped',", + " visibility = ['//visibility:public'],", + ")"); + + scratch.file("main.proto", "syntax = 'proto3'';", "import 'yolo_pkg/yolo.proto';"); + scratch.file( + "BUILD", + "proto_library(", + " name = 'main_proto',", + " srcs = ['main.proto'],", + " deps = ['@yolo_repo//yolo_pkg_to_be_stripped/yolo_pkg:yolo_proto'],", + ")"); + + ConfiguredTarget main = getConfiguredTarget("//:main_proto"); + ProtoInfo protoInfo = main.get(ProtoInfo.PROVIDER); + ImmutableList> importPaths = + protoInfo.getStrictImportableProtoSourcesImportPaths().toList(); + assertThat(importPaths).isNotEmpty(); + assertThat(importPaths.get(0).second).isEqualTo("yolo_pkg/yolo.proto"); + } + + @Test + public void testRelativeStripImportPrefixInExternalRepo() throws Exception { + if (!isThisBazel()) { + return; + } + + FileSystemUtils.appendIsoLatin1( + scratch.resolve("WORKSPACE"), "local_repository(name = 'yolo_repo', path = '/yolo_repo')"); + invalidatePackages(); + + scratch.file("/yolo_repo/WORKSPACE"); + scratch.file("/yolo_repo/yolo_pkg_to_be_stripped/yolo_pkg/yolo.proto"); + scratch.file( + "/yolo_repo/BUILD", + "proto_library(", + " name = 'yolo_proto',", + " srcs = ['yolo_pkg_to_be_stripped/yolo_pkg/yolo.proto'],", + " strip_import_prefix = 'yolo_pkg_to_be_stripped',", + " visibility = ['//visibility:public'],", + ")"); + + scratch.file("main.proto", "syntax = 'proto3'';", "import 'yolo_pkg/yolo.proto';"); + scratch.file( + "BUILD", + "proto_library(", + " name = 'main_proto',", + " srcs = ['main.proto'],", + " deps = ['@yolo_repo//:yolo_proto'],", + ")"); + + ConfiguredTarget main = getConfiguredTarget("//:main_proto"); + ProtoInfo protoInfo = main.get(ProtoInfo.PROVIDER); + ImmutableList> importPaths = + protoInfo.getStrictImportableProtoSourcesImportPaths().toList(); + assertThat(importPaths).isNotEmpty(); + assertThat(importPaths.get(0).second).isEqualTo("yolo_pkg/yolo.proto"); + } + @Test public void testIllegalStripImportPrefix() throws Exception { scratch.file(