From 5d46f327041ade0a6314ba0859baef919730c796 Mon Sep 17 00:00:00 2001 From: Googler Date: Mon, 13 Aug 2018 04:00:32 -0700 Subject: bazel: handle proto_src_root when dealing with proto includes, generated files and C++ headers This change completes the handling of proto_src_root when it comes to inclusion of protos, generating the proto files in the right place and adding the generated headers to the include paths. WANT_LGTM=elenairina RELNOTES: None. PiperOrigin-RevId: 208457740 --- .../build/lib/rules/proto/BazelProtoLibrary.java | 1 + .../lib/rules/proto/ProtoCompileActionBuilder.java | 74 ++++++++++++++-------- .../build/lib/rules/proto/SupportData.java | 5 ++ 3 files changed, 54 insertions(+), 26 deletions(-) (limited to 'src/main/java/com/google/devtools/build/lib/rules/proto') 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 1c8a7d52cb..000aa5ec53 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 @@ -60,6 +60,7 @@ public class BazelProtoLibrary implements RuleConfiguredTargetFactory { protosInDirectDeps, transitiveImports, protoPathFlags, + protoSourceRoot, directProtoSourceRoots, !protoSources.isEmpty()); 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 ed138d0381..da43b6a2a4 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 @@ -47,6 +47,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.util.LazyString; +import com.google.devtools.build.lib.vfs.PathFragment; import java.util.Collection; import java.util.HashSet; import java.util.List; @@ -570,7 +571,11 @@ public class ProtoCompileActionBuilder { @Nullable NestedSet protosInDirectDependencies, NestedSet directProtoSourceRoots, NestedSet transitiveImports) { - commandLine.addAll(VectorArg.of(transitiveImports).mapped(EXPAND_TRANSITIVE_IMPORT_ARG)); + // For each import, include both the import as well as the import relativized against its + // 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))); if (protosInDirectDependencies != null) { if (!protosInDirectDependencies.isEmpty()) { commandLine.addAll( @@ -590,11 +595,31 @@ public class ProtoCompileActionBuilder { static final CommandLineItem.MapFn EXPAND_TRANSITIVE_PROTO_PATH_FLAGS = (flag, args) -> args.accept("--proto_path=" + flag); - @AutoCodec @AutoCodec.VisibleForSerialization - static final CommandLineItem.MapFn EXPAND_TRANSITIVE_IMPORT_ARG = - (artifact, args) -> - args.accept( - "-I" + getPathIgnoringRepository(artifact) + "=" + artifact.getExecPathString()); + @AutoCodec + @AutoCodec.VisibleForSerialization + static final class ExpandImportArgsFn implements CapturingMapFn { + private final NestedSet directProtoSourceRoots; + + public ExpandImportArgsFn(NestedSet directProtoSourceRoots) { + this.directProtoSourceRoots = directProtoSourceRoots; + } + + /** + * Generates up to two import flags for each artifact: one for full path (only relative to the + * repository root) and one for the path relative to the proto source root (if one exists + * corresponding to the artifact). + */ + @Override + public void expandToCommandLine(Artifact proto, Consumer args) { + for (String directProtoSourceRoot : directProtoSourceRoots) { + String path = getPathIgnoringSourceRoot(proto, directProtoSourceRoot); + if (path != null) { + args.accept("-I" + path + "=" + proto.getExecPathString()); + } + } + args.accept("-I" + getPathIgnoringRepository(proto) + "=" + proto.getExecPathString()); + } + } @AutoCodec @AutoCodec.VisibleForSerialization @@ -608,16 +633,15 @@ public class ProtoCompileActionBuilder { @Override public void expandToCommandLine(Artifact proto, Consumer args) { for (String directProtoSourceRoot : directProtoSourceRoots) { - expandToPathIgnoringSourceRoot(proto, directProtoSourceRoot, args); + String path = getPathIgnoringSourceRoot(proto, directProtoSourceRoot); + if (path != null) { + args.accept(path); + } } - EXPAND_TO_PATH_IGNORING_REPOSITORY.expandToCommandLine(proto, args); + args.accept(getPathIgnoringRepository(proto)); } } - @AutoCodec @AutoCodec.VisibleForSerialization - static final CommandLineItem.MapFn EXPAND_TO_PATH_IGNORING_REPOSITORY = - (artifact, args) -> args.accept(getPathIgnoringRepository(artifact)); - /** * Gets the artifact's path relative to the root, ignoring the external repository the artifact is * at. For example, @@ -633,27 +657,25 @@ public class ProtoCompileActionBuilder { .toString(); } - private static void expandToPathIgnoringSourceRoot( - Artifact artifact, String directProtoSourceRoot, Consumer args) { + /** + * Gets the artifact's path relative to the proto source root, ignoring the external repository + * the artifact is at. For example, + * //a/b/c:d.proto with proto source root a/b --> c/d.proto + * {@literal @}foo//a/b/c:d.proto with proto source root a/b --> c/d.proto + * + */ + private static String getPathIgnoringSourceRoot(Artifact artifact, String directProtoSourceRoot) { // TODO(bazel-team): IAE is caught here because every artifact is relativized against every // directProtoSourceRoot. Instead of catching the exception, a check should be performed // to see if the artifact has the root as a substring before relativizing. try { - String relativePath = - artifact - .getRootRelativePath() - .relativeTo( - artifact - .getOwnerLabel() - .getPackageIdentifier() - .getRepository() - .getPathUnderExecRoot()) - .relativeTo(directProtoSourceRoot) - .toString(); - args.accept(relativePath); + return PathFragment.createAlreadyNormalized(getPathIgnoringRepository(artifact)) + .relativeTo(directProtoSourceRoot) + .toString(); } catch (IllegalArgumentException exception) { // do nothing } + return null; } /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/SupportData.java b/src/main/java/com/google/devtools/build/lib/rules/proto/SupportData.java index c3bc3aa553..b3644d4c17 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/SupportData.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/SupportData.java @@ -35,6 +35,7 @@ public abstract class SupportData { NestedSet protosInDirectDeps, NestedSet transitiveImports, NestedSet transitiveProtoPathFlags, + String protoSourceRoot, NestedSet directProtoSourceRoots, boolean hasProtoSources) { return new AutoValue_SupportData( @@ -43,6 +44,7 @@ public abstract class SupportData { transitiveImports, protosInDirectDeps, transitiveProtoPathFlags, + protoSourceRoot, directProtoSourceRoots, hasProtoSources); } @@ -64,6 +66,9 @@ public abstract class SupportData { */ public abstract NestedSet getTransitiveProtoPathFlags(); + /** The {@code proto_source_root} of the current library. */ + public abstract String getProtoSourceRoot(); + /** * The {@code proto_source_root}'s collected from the current library and the direct dependencies. */ -- cgit v1.2.3