diff options
Diffstat (limited to 'src')
11 files changed, 620 insertions, 115 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspect.java b/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspect.java index f6d4856374..472bef60f9 100644 --- a/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspect.java @@ -20,8 +20,10 @@ import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.BuildType.LABEL; import com.google.common.base.Function; +import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.io.ByteSource; import com.google.devtools.build.lib.Constants; @@ -56,6 +58,7 @@ import com.google.devtools.build.lib.ideinfo.androidstudio.AndroidStudioIdeInfo. import com.google.devtools.build.lib.packages.AspectDefinition; import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.packages.BuildType; +import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.rules.android.AndroidCommon; import com.google.devtools.build.lib.rules.android.AndroidIdeInfoProvider; @@ -303,6 +306,9 @@ public class AndroidStudioInfoAspect implements ConfiguredNativeAspectFactory { .getPath() .toString()); + outputBuilder.setBuildFileArtifactLocation( + makeArtifactLocation(ruleContext.getRule().getPackage())); + outputBuilder.setKind(ruleKind); if (ruleKind == Kind.JAVA_LIBRARY @@ -373,8 +379,7 @@ public class AndroidStudioInfoAspect implements ConfiguredNativeAspectFactory { .setExecutable(ruleContext.getExecutablePrerequisite("$packageParser", Mode.HOST)) .setCommandLine(CustomCommandLine.builder() .addExecPath("--output_manifest", packageManifest) - .addJoinStrings("--sources_absolute_paths", ":", Artifact.toAbsolutePaths(sourceFiles)) - .addJoinExecPaths("--sources_execution_paths", ":", sourceFiles) + .addJoinStrings("--sources", ":", toSerializedArtifactLocations(sourceFiles)) .build()) .useParameterFile(ParameterFileType.SHELL_QUOTED) .setProgressMessage("Parsing java package strings for " + ruleContext.getRule()) @@ -382,6 +387,25 @@ public class AndroidStudioInfoAspect implements ConfiguredNativeAspectFactory { .build(ruleContext); } + private static Iterable<String> toSerializedArtifactLocations(Iterable<Artifact> artifacts) { + return Iterables.transform( + Iterables.filter(artifacts, Artifact.MIDDLEMAN_FILTER), + PACKAGE_PARSER_SERIALIZER); + } + + private static final Function<Artifact, String> PACKAGE_PARSER_SERIALIZER = + new Function<Artifact, String>() { + @Override + public String apply(Artifact artifact) { + ArtifactLocation location = makeArtifactLocation(artifact); + return Joiner.on(",").join( + location.getRootExecutionPathFragment(), + location.getRelativePath(), + location.getRootPath() + ); + } + }; + private static Artifact derivedArtifact(ConfiguredTarget base, RuleContext ruleContext, String suffix) { BuildConfiguration configuration = ruleContext.getConfiguration(); @@ -471,16 +495,28 @@ public class AndroidStudioInfoAspect implements ConfiguredNativeAspectFactory { } private static ArtifactLocation makeArtifactLocation(Artifact artifact) { + return makeArtifactLocation(artifact.getRoot(), artifact.getRootRelativePath()); + } + + private static ArtifactLocation makeArtifactLocation(Package pkg) { + Root root = Root.asSourceRoot(pkg.getSourceRoot()); + PathFragment relativePath = pkg.getBuildFile().getPath().relativeTo(root.getPath()); + return makeArtifactLocation(root, relativePath); + } + + private static ArtifactLocation makeArtifactLocation(Root root, PathFragment relativePath) { return ArtifactLocation.newBuilder() - .setRootPath(artifact.getRoot().getPath().toString()) - .setRelativePath(artifact.getRootRelativePathString()) - .setIsSource(artifact.isSourceArtifact()) + .setRootPath(root.getPath().toString()) + .setRootExecutionPathFragment(root.getExecPath().toString()) + .setRelativePath(relativePath.toString()) + .setIsSource(root.isSourceRoot()) .build(); } private static ArtifactLocation makeArtifactLocation(SourceDirectory resourceDir) { return ArtifactLocation.newBuilder() .setRootPath(resourceDir.getRootPath().toString()) + .setRootExecutionPathFragment(resourceDir.getRootExecutionPathFragment().toString()) .setRelativePath(resourceDir.getRelativePath().toString()) .setIsSource(resourceDir.isSource()) .build(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidIdeInfoProvider.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidIdeInfoProvider.java index 7028a45eca..0cd44ad59d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidIdeInfoProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidIdeInfoProvider.java @@ -13,6 +13,7 @@ // limitations under the License. package com.google.devtools.build.lib.rules.android; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; @@ -39,11 +40,29 @@ public final class AndroidIdeInfoProvider implements TransitiveInfoProvider { @Immutable public static class SourceDirectory { final PathFragment relativePath; + final PathFragment rootExecutionPathFragment; final PathFragment rootPath; final boolean isSource; - public SourceDirectory(PathFragment rootPath, PathFragment relativePath, boolean isSource) { + @VisibleForTesting + public static SourceDirectory fromSourceRoot( + PathFragment rootPath, + PathFragment relativePath) { + return new SourceDirectory(rootPath, PathFragment.EMPTY_FRAGMENT, relativePath, true); + } + + public static SourceDirectory fromRoot(Root root, PathFragment relativePath) { + return new SourceDirectory( + root.getPath().asFragment(), root.getExecPath(), relativePath, root.isSourceRoot()); + } + + private SourceDirectory( + PathFragment rootPath, + PathFragment rootExecutionPathFragment, + PathFragment relativePath, + boolean isSource) { this.rootPath = rootPath; + this.rootExecutionPathFragment = rootExecutionPathFragment; this.relativePath = relativePath; this.isSource = isSource; } @@ -62,6 +81,14 @@ public final class AndroidIdeInfoProvider implements TransitiveInfoProvider { return rootPath; } + /** + * The path from the execution root to the actual root. For source roots, this returns + * the empty fragment, {@link Root#getExecPath()}. + */ + public PathFragment getRootExecutionPathFragment() { + return rootExecutionPathFragment; + } + /** Indicates if the directory is in the gen files tree. */ public boolean isSource() { return isSource; @@ -69,7 +96,7 @@ public final class AndroidIdeInfoProvider implements TransitiveInfoProvider { @Override public int hashCode() { - return Objects.hash(relativePath, rootPath, isSource); + return Objects.hash(relativePath, rootPath, rootExecutionPathFragment, isSource); } @Override @@ -77,6 +104,7 @@ public final class AndroidIdeInfoProvider implements TransitiveInfoProvider { if (other instanceof SourceDirectory) { SourceDirectory otherDir = (SourceDirectory) other; return Objects.equals(rootPath, otherDir.rootPath) + && Objects.equals(rootExecutionPathFragment, otherDir.rootExecutionPathFragment) && Objects.equals(relativePath, otherDir.relativePath) && Objects.equals(isSource, otherDir.isSource); } @@ -86,7 +114,7 @@ public final class AndroidIdeInfoProvider implements TransitiveInfoProvider { @Override public String toString() { return "SourceDirectory [relativePath=" + relativePath + ", rootPath=" + rootPath - + ", isSource=" + isSource + "]"; + + ", executionRootPrefix=" + rootExecutionPathFragment + ", isSource=" + isSource + "]"; } } @@ -179,10 +207,9 @@ public final class AndroidIdeInfoProvider implements TransitiveInfoProvider { private void addIdlDirs(Collection<Artifact> idlArtifacts) { for (Artifact idl : idlArtifacts) { this.idlDirs.add( - new SourceDirectory( - idl.getRoot().getPath().asFragment(), - idl.getRootRelativePath().getParentDirectory(), - idl.isSourceArtifact())); + SourceDirectory.fromRoot( + idl.getRoot(), + idl.getRootRelativePath().getParentDirectory())); } } @@ -199,10 +226,9 @@ public final class AndroidIdeInfoProvider implements TransitiveInfoProvider { public Builder addResourceSource(Artifact resource) { PathFragment resourceDir = LocalResourceContainer.Builder.findResourceDir(resource); resourceDirs.add( - new SourceDirectory( - resource.getRoot().getPath().asFragment(), - trimTo(resource.getRootRelativePath(), resourceDir), - resource.isSourceArtifact())); + SourceDirectory.fromRoot( + resource.getRoot(), + trimTo(resource.getRootRelativePath(), resourceDir))); return this; } @@ -222,10 +248,9 @@ public final class AndroidIdeInfoProvider implements TransitiveInfoProvider { public Builder addAssetSource(Artifact asset, PathFragment assetDir) { assetDirs.add( - new SourceDirectory( - asset.getRoot().getPath().asFragment(), - trimTo(asset.getRootRelativePath(), assetDir), - asset.isSourceArtifact())); + SourceDirectory.fromRoot( + asset.getRoot(), + trimTo(asset.getRootRelativePath(), assetDir))); return this; } diff --git a/src/main/protobuf/android_studio_ide_info.proto b/src/main/protobuf/android_studio_ide_info.proto index 0be192ebcb..0b099c6969 100644 --- a/src/main/protobuf/android_studio_ide_info.proto +++ b/src/main/protobuf/android_studio_ide_info.proto @@ -22,9 +22,14 @@ option java_package = "com.google.devtools.build.lib.ideinfo.androidstudio"; option java_generate_equals_and_hash = true; message ArtifactLocation { - string root_path = 1; + // deprecated -- this is not machine agnostic and will be removed in a future release + string root_path = 1 [deprecated=true]; + string relative_path = 2; bool is_source = 3; + + // set for derived artifacts (isSource = false) + string root_execution_path_fragment = 4; } message LibraryArtifact { @@ -73,7 +78,8 @@ message RuleIdeInfo { string label = 1; Kind kind = 2; - string build_file = 3; + // deprecated -- this is not machine agnostic and will be removed in a future release + string build_file = 3 [deprecated=true]; repeated string dependencies = 4; // kind is ANDROID_SDK @@ -85,4 +91,6 @@ message RuleIdeInfo { repeated string tags = 9; repeated string runtime_deps = 10; + + ArtifactLocation build_file_artifact_location = 11; } diff --git a/src/main/protobuf/package_manifest.proto b/src/main/protobuf/package_manifest.proto index dd5f4fc35a..b714f8ad26 100644 --- a/src/main/protobuf/package_manifest.proto +++ b/src/main/protobuf/package_manifest.proto @@ -21,9 +21,23 @@ option java_package = "com.google.devtools.build.lib.ideinfo.androidstudio"; option java_generate_equals_and_hash = true; +// TODO(bazel-team): support proto => proto dependencies and remove this duplication +message ArtifactLocation { + // deprecated -- this is not machine agnostic and will be removed in a future release + string root_path = 1 [deprecated=true]; + + string relative_path = 2; + bool is_source = 3; + + // set for derived artifacts (isSource = false) + string root_execution_path_fragment = 4; +} + message JavaSourcePackage { - string absolute_path = 1; + // deprecated -- this is not machine agnostic and will be removed in a future release + string absolute_path = 1 [deprecated=true]; string package_string = 2; + ArtifactLocation artifact_location = 3; } message PackageManifest { diff --git a/src/test/java/com/google/devtools/build/android/ideinfo/ArtifactLocationConverterTest.java b/src/test/java/com/google/devtools/build/android/ideinfo/ArtifactLocationConverterTest.java new file mode 100644 index 0000000000..0ac6311857 --- /dev/null +++ b/src/test/java/com/google/devtools/build/android/ideinfo/ArtifactLocationConverterTest.java @@ -0,0 +1,96 @@ +// Copyright 2016 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.android.ideinfo; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.fail; + +import com.google.common.base.Joiner; +import com.google.devtools.build.lib.ideinfo.androidstudio.PackageManifestOuterClass.ArtifactLocation; +import com.google.devtools.common.options.OptionsParsingException; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** + * Tests {@link ArtifactLocationConverter}. + */ +@RunWith(JUnit4.class) +public class ArtifactLocationConverterTest { + + private ArtifactLocationConverter converter; + + @Before + public final void init() throws Exception { + converter = new ArtifactLocationConverter(); + } + + @Test + public void testConverterSourceArtifact() throws Exception { + ArtifactLocation parsed = converter.convert( + Joiner.on(",").join("", "test.java", "/usr/local/code") + ); + assertThat(parsed).isEqualTo( + ArtifactLocation.newBuilder() + .setRootPath("/usr/local/code") + .setRelativePath("test.java") + .setIsSource(true) + .build()); + } + + @Test + public void testConverterDerivedArtifact() throws Exception { + ArtifactLocation parsed = converter.convert( + Joiner.on(",").join("bin", "java/com/test.java", "/usr/local/_tmp/code/bin") + ); + assertThat(parsed).isEqualTo( + ArtifactLocation.newBuilder() + .setRootPath("/usr/local/_tmp/code/bin") + .setRootExecutionPathFragment("bin") + .setRelativePath("java/com/test.java") + .setIsSource(false) + .build()); + } + + @Test + public void testInvalidFormatFails() throws Exception { + assertFails("/root", "Expected either 2 or 3 comma-separated paths"); + assertFails("/root,exec,rel,extra", "Expected either 2 or 3 comma-separated paths"); + } + + @Test + public void testFutureFormat() throws Exception { + ArtifactLocation parsed = converter.convert("bin/out,java/com/test.java"); + assertThat(parsed).isEqualTo( + ArtifactLocation.newBuilder() + .setRootExecutionPathFragment("bin/out") + .setRelativePath("java/com/test.java") + .setIsSource(false) + .build()); + } + + + private void assertFails(String input, String expectedError) { + try { + new ArtifactLocationConverter().convert(input); + fail(); + } catch (OptionsParsingException e) { + assertThat(e).hasMessage(expectedError); + } + } +} + diff --git a/src/test/java/com/google/devtools/build/android/ideinfo/BUILD b/src/test/java/com/google/devtools/build/android/ideinfo/BUILD index 79f75dd8a3..aa917ed80a 100644 --- a/src/test/java/com/google/devtools/build/android/ideinfo/BUILD +++ b/src/test/java/com/google/devtools/build/android/ideinfo/BUILD @@ -1,8 +1,9 @@ java_test( name = "PackageParserTest", size = "small", - srcs = glob(["*.java"]), + srcs = ["com.google.devtools.build.android.ideinfo.PackageParserTest"], deps = [ + "//src/main/java/com/google/devtools/common/options", "//src/main/protobuf:package_manifest_proto", "//src/tools/android/java/com/google/devtools/build/android/ideinfo:package_parser_lib", "//third_party:guava", @@ -12,3 +13,18 @@ java_test( "//third_party:truth", ], ) + +java_test( + name = "ArtifactLocationConverterTest", + size = "small", + srcs = ["com.google.devtools.build.android.ideinfo.ArtifactLocationConverterTest"], + deps = [ + "//src/main/java/com/google/devtools/common/options", + "//src/main/protobuf:package_manifest_proto", + "//src/tools/android/java/com/google/devtools/build/android/ideinfo:package_parser_lib", + "//third_party:guava", + "//third_party:junit4", + "//third_party:protobuf", + "//third_party:truth", + ], +) diff --git a/src/test/java/com/google/devtools/build/android/ideinfo/PackageParserTest.java b/src/test/java/com/google/devtools/build/android/ideinfo/PackageParserTest.java index a02ac30baf..3f466d6d79 100644 --- a/src/test/java/com/google/devtools/build/android/ideinfo/PackageParserTest.java +++ b/src/test/java/com/google/devtools/build/android/ideinfo/PackageParserTest.java @@ -17,9 +17,11 @@ package com.google.devtools.build.android.ideinfo; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.fail; +import com.google.common.base.Joiner; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; +import com.google.devtools.build.lib.ideinfo.androidstudio.PackageManifestOuterClass.ArtifactLocation; import com.google.protobuf.MessageLite; import org.junit.Before; @@ -35,6 +37,7 @@ import java.io.InputStreamReader; import java.io.StringWriter; import java.io.UnsupportedEncodingException; import java.nio.charset.StandardCharsets; +import java.nio.file.InvalidPathException; import java.nio.file.Path; import java.nio.file.Paths; import java.util.List; @@ -50,12 +53,16 @@ public class PackageParserTest { private static class MockPackageParserIoProvider extends PackageParserIoProvider { private final Map<Path, InputStream> sources = Maps.newHashMap(); + private final List<ArtifactLocation> sourceLocations = Lists.newArrayList(); private StringWriter writer = new StringWriter(); - public MockPackageParserIoProvider addSource(String filePath, String javaSrc) { + public MockPackageParserIoProvider addSource(ArtifactLocation source, String javaSrc) { try { - sources.put(Paths.get(filePath), new ByteArrayInputStream(javaSrc.getBytes("UTF-8"))); - } catch (UnsupportedEncodingException e) { + Path path = Paths.get(source.getRootExecutionPathFragment(), source.getRelativePath()); + sources.put(path, new ByteArrayInputStream(javaSrc.getBytes("UTF-8"))); + sourceLocations.add(source); + + } catch (UnsupportedEncodingException | InvalidPathException e) { fail(e.getMessage()); } return this; @@ -63,11 +70,12 @@ public class PackageParserTest { public void reset() { sources.clear(); + sourceLocations.clear(); writer = new StringWriter(); } - public List<Path> getPaths() { - return Lists.newArrayList(sources.keySet()); + public List<ArtifactLocation> getSourceLocations() { + return Lists.newArrayList(sourceLocations); } @Nonnull @@ -83,6 +91,21 @@ public class PackageParserTest { } } + private static final ArtifactLocation DUMMY_SOURCE_ARTIFACT = + ArtifactLocation.newBuilder() + .setRootPath("/usr/local/google/code") + .setRelativePath("java/com/google/Foo.java") + .setIsSource(true) + .build(); + + private static final ArtifactLocation DUMMY_DERIVED_ARTIFACT = + ArtifactLocation.newBuilder() + .setRootPath("/usr/local/_tmp/code/bin") + .setRootExecutionPathFragment("bin") + .setRelativePath("java/com/google/Bla.java") + .setIsSource(false) + .build(); + private MockPackageParserIoProvider mockIoProvider; private PackageParser parser; @@ -92,9 +115,9 @@ public class PackageParserTest { parser = new PackageParser(mockIoProvider); } - private Map<Path, String> parsePackageStrings() throws Exception { - List<Path> paths = mockIoProvider.getPaths(); - return parser.parsePackageStrings(paths, paths); + private Map<ArtifactLocation, String> parsePackageStrings() throws Exception { + List<ArtifactLocation> sources = mockIoProvider.getSourceLocations(); + return parser.parsePackageStrings(sources); } @Test @@ -102,91 +125,205 @@ public class PackageParserTest { String[] args = new String[] { "--output_manifest", "/tmp/out.manifest", - "--sources_absolute_paths", - "/path/test1.java:/path/test2.java", - "--sources_execution_paths", - "/path/test1.java:/path/test2.java" - }; + "--sources", + Joiner.on(":").join( + ",java/com/google/Foo.java,/usr/local/google/code", + "bin,java/com/google/Bla.java,/usr/local/_tmp/code/bin" + )}; PackageParser.PackageParserOptions options = PackageParser.parseArgs(args); assertThat(options.outputManifest.toString()).isEqualTo("/tmp/out.manifest"); - assertThat(options.sourcesAbsolutePaths).hasSize(2); - assertThat(options.sourcesExecutionPaths).hasSize(2); - assertThat(options.sourcesAbsolutePaths.get(0).toString()).isEqualTo("/path/test1.java"); - assertThat(options.sourcesAbsolutePaths.get(1).toString()).isEqualTo("/path/test2.java"); + assertThat(options.sources).hasSize(2); + assertThat(options.sources.get(0)).isEqualTo( + ArtifactLocation.newBuilder() + .setRootPath("/usr/local/google/code") + .setRelativePath("java/com/google/Foo.java") + .setIsSource(true) + .build()); + assertThat(options.sources.get(1)).isEqualTo( + ArtifactLocation.newBuilder() + .setRootPath("/usr/local/_tmp/code/bin") + .setRootExecutionPathFragment("bin") + .setRelativePath("java/com/google/Bla.java") + .setIsSource(false) + .build()); } @Test public void testReadNoSources() throws Exception { - Map<Path, String> map = parsePackageStrings(); + Map<ArtifactLocation, String> map = parsePackageStrings(); assertThat(map).isEmpty(); } @Test public void testSingleRead() throws Exception { mockIoProvider - .addSource("java/com/google/Bla.java", - "package com.test;\n public class Bla {}\""); - Map<Path, String> map = parsePackageStrings(); + .addSource( + DUMMY_SOURCE_ARTIFACT, + "package com.google;\n public class Bla {}\""); + Map<ArtifactLocation, String> map = parsePackageStrings(); assertThat(map).hasSize(1); - assertThat(map).containsEntry(Paths.get("java/com/google/Bla.java"), "com.test"); + assertThat(map).containsEntry( + DUMMY_SOURCE_ARTIFACT, + "com.google"); } @Test public void testMultiRead() throws Exception { mockIoProvider - .addSource("java/com/google/Bla.java", + .addSource( + DUMMY_SOURCE_ARTIFACT, "package com.test;\n public class Bla {}\"") - .addSource("java/com/other/Foo.java", + .addSource( + DUMMY_DERIVED_ARTIFACT, "package com.other;\n public class Foo {}\""); - Map<Path, String> map = parsePackageStrings(); + Map<ArtifactLocation, String> map = parsePackageStrings(); assertThat(map).hasSize(2); - assertThat(map).containsEntry(Paths.get("java/com/google/Bla.java"), "com.test"); - assertThat(map).containsEntry(Paths.get("java/com/other/Foo.java"), "com.other"); + assertThat(map).containsEntry( + DUMMY_SOURCE_ARTIFACT, + "com.test"); + assertThat(map).containsEntry( + DUMMY_DERIVED_ARTIFACT, + "com.other"); } @Test public void testReadSomeInvalid() throws Exception { mockIoProvider - .addSource("java/com/google/Bla.java", + .addSource( + DUMMY_SOURCE_ARTIFACT, "package %com.test;\n public class Bla {}\"") - .addSource("java/com/other/Foo.java", + .addSource( + DUMMY_DERIVED_ARTIFACT, "package com.other;\n public class Foo {}\""); - Map<Path, String> map = parsePackageStrings(); + Map<ArtifactLocation, String> map = parsePackageStrings(); assertThat(map).hasSize(1); - assertThat(map).containsEntry(Paths.get("java/com/other/Foo.java"), "com.other"); + assertThat(map).containsEntry( + DUMMY_DERIVED_ARTIFACT, + "com.other"); } @Test public void testReadAllInvalid() throws Exception { mockIoProvider - .addSource("java/com/google/Bla.java", + .addSource( + DUMMY_SOURCE_ARTIFACT, "#package com.test;\n public class Bla {}\"") - .addSource("java/com/other/Foo.java", + .addSource( + DUMMY_DERIVED_ARTIFACT, "package com.other\n public class Foo {}\""); - Map<Path, String> map = parsePackageStrings(); + Map<ArtifactLocation, String> map = parsePackageStrings(); assertThat(map).isEmpty(); } @Test public void testWriteEmptyMap() throws Exception { parser.writeManifest( - Maps.<Path, String> newHashMap(), Paths.get("/java/com/google/test.manifest")); + Maps.<ArtifactLocation, String> newHashMap(), + Paths.get("/java/com/google/test.manifest"), + false); assertThat(mockIoProvider.writer.toString()).isEmpty(); } @Test public void testWriteMap() throws Exception { - Map<Path, String> map = ImmutableMap.of( - Paths.get("/java/com/google/Bla.java"), "com.google", - Paths.get("/java/com/other/Foo.java"), "com.other" + Map<ArtifactLocation, String> map = ImmutableMap.of( + DUMMY_SOURCE_ARTIFACT, + "com.google", + DUMMY_DERIVED_ARTIFACT, + "com.other" ); - parser.writeManifest(map, Paths.get("/java/com/google/test.manifest")); + parser.writeManifest(map, Paths.get("/java/com/google/test.manifest"), false); String writtenString = mockIoProvider.writer.toString(); - assertThat(writtenString).contains("absolute_path: \"/java/com/google/Bla.java\""); + assertThat(writtenString).contains(String.format( + "root_path: \"%s\"", + DUMMY_SOURCE_ARTIFACT.getRootPath())); + assertThat(writtenString).contains(String.format( + "relative_path: \"%s\"", + DUMMY_SOURCE_ARTIFACT.getRelativePath())); assertThat(writtenString).contains("package_string: \"com.google\""); - assertThat(writtenString).contains("absolute_path: \"/java/com/other/Foo.java\""); + + assertThat(writtenString).contains(String.format( + "root_path: \"%s\"", + DUMMY_DERIVED_ARTIFACT.getRootPath())); + assertThat(writtenString).contains(String.format( + "root_execution_path_fragment: \"%s\"", + DUMMY_DERIVED_ARTIFACT.getRootExecutionPathFragment())); + assertThat(writtenString).contains(String.format( + "relative_path: \"%s\"", + DUMMY_DERIVED_ARTIFACT.getRelativePath())); assertThat(writtenString).contains("package_string: \"com.other\""); } + @Test + public void testHandlesOldFormat() throws Exception { + String[] args = new String[] { + "--output_manifest", + "/tmp/out.manifest", + "--sources_absolute_paths", + "/usr/local/code/java/com/google/Foo.java:/usr/local/code/java/com/google/Bla.java", + "--sources_execution_paths", + "java/com/google/Foo.java:java/com/google/Bla.java" + }; + PackageParser.PackageParserOptions options = PackageParser.parseArgs(args); + assertThat(options.outputManifest.toString()).isEqualTo("/tmp/out.manifest"); + assertThat(options.sourcesAbsolutePaths.get(0).toString()) + .isEqualTo("/usr/local/code/java/com/google/Foo.java"); + assertThat(options.sourcesAbsolutePaths.get(1).toString()) + .isEqualTo("/usr/local/code/java/com/google/Bla.java"); + + PackageParser.convertFromOldFormat(options); + assertThat(options.sources).hasSize(2); + assertThat(options.sources.get(0)).isEqualTo( + ArtifactLocation.newBuilder() + .setRootPath("/usr/local/code/") + .setRelativePath("java/com/google/Foo.java") + .build()); + + mockIoProvider + .addSource(options.sources.get(0), + "package com.test;\n public class Bla {}\"") + .addSource(options.sources.get(1), + "package com.other;\n public class Foo {}\""); + + Map<ArtifactLocation, String> map = parser.parsePackageStrings(options.sources); + assertThat(map).hasSize(2); + assertThat(map).containsEntry(options.sources.get(0), "com.test"); + assertThat(map).containsEntry(options.sources.get(1), "com.other"); + + parser.writeManifest(map, options.outputManifest, true); + + String writtenString = mockIoProvider.writer.toString(); + assertThat(writtenString).doesNotContain("location"); + assertThat(writtenString).contains( + "absolute_path: \"/usr/local/code/java/com/google/Foo.java\""); + assertThat(writtenString).contains( + "absolute_path: \"/usr/local/code/java/com/google/Bla.java\""); + } + + @Test + public void testHandlesFutureFormat() throws Exception { + String[] args = new String[] { + "--output_manifest", + "/tmp/out.manifest", + "--sources", + ",java/com/google/Foo.java:" + + "bin/out,java/com/google/Bla.java", + }; + PackageParser.PackageParserOptions options = PackageParser.parseArgs(args); + assertThat(options.outputManifest.toString()).isEqualTo("/tmp/out.manifest"); + assertThat(options.sources).hasSize(2); + assertThat(options.sources.get(0)).isEqualTo( + ArtifactLocation.newBuilder() + .setRelativePath("java/com/google/Foo.java") + .setIsSource(true) + .build()); + assertThat(options.sources.get(1)).isEqualTo( + ArtifactLocation.newBuilder() + .setRootExecutionPathFragment("bin/out") + .setRelativePath("java/com/google/Bla.java") + .setIsSource(false) + .build()); + } + } diff --git a/src/test/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspectTest.java b/src/test/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspectTest.java index 6810d5d50d..22416fc896 100644 --- a/src/test/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspectTest.java +++ b/src/test/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspectTest.java @@ -43,6 +43,7 @@ import java.io.BufferedReader; import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; @@ -69,6 +70,10 @@ public class AndroidStudioInfoAspectTest extends AndroidStudioInfoAspectTestBase "//com/google/example:simple", ruleIdeInfos); if (isNativeTest()) { assertThat(ruleIdeInfo.getBuildFile()).isEqualTo(buildFilePath.toString()); + ArtifactLocation location = ruleIdeInfo.getBuildFileArtifactLocation(); + assertThat(Paths.get(location.getRootPath(), location.getRelativePath()).toString()) + .isEqualTo(buildFilePath.toString()); + assertThat(location.getRelativePath()).isEqualTo("com/google/example/BUILD"); } assertThat(ruleIdeInfo.getKind()).isEqualTo(Kind.JAVA_LIBRARY); assertThat(ruleIdeInfo.getDependenciesCount()).isEqualTo(0); @@ -907,26 +912,21 @@ public class AndroidStudioInfoAspectTest extends AndroidStudioInfoAspectTestBase ")"); Map<String, RuleIdeInfo> ruleIdeInfos = buildRuleIdeInfo("//com/google/example:lib"); RuleIdeInfo ruleIdeInfo = getRuleInfoAndVerifyLabel("//com/google/example:lib", ruleIdeInfos); - assertThat(ruleIdeInfo.getJavaRuleIdeInfo().getSourcesList()) - .containsExactly( - expectedArtifactLocationWithRootPath( - targetConfig.getGenfilesDirectory().getPath().getPathString()) - .setRelativePath("com/google/example/gen.java") - .setIsSource(false) - .build(), - expectedArtifactLocationWithRootPath(directories.getWorkspace().getPathString()) - .setRelativePath("com/google/example/Test.java") - .setIsSource(true) - .build()); - } - - private ArtifactLocation.Builder expectedArtifactLocationWithRootPath(String pathString) { - if (isNativeTest()) { - return ArtifactLocation.newBuilder().setRootPath(pathString); - } else { - // todo(dslomov): Skylark aspect implementation does not yet return a correct root path. - return ArtifactLocation.newBuilder(); - } + // todo(dslomov): Skylark aspect implementation does not yet return a correct root path. + assertThat(ruleIdeInfo.getJavaRuleIdeInfo().getSourcesList()).containsExactly( + ArtifactLocation.newBuilder() + .setRootPath( + isNativeTest() ? targetConfig.getGenfilesDirectory().getPath().getPathString() : "") + .setRootExecutionPathFragment( + isNativeTest() ? targetConfig.getGenfilesDirectory().getExecPathString() : "") + .setRelativePath("com/google/example/gen.java") + .setIsSource(false) + .build(), + ArtifactLocation.newBuilder() + .setRootPath(isNativeTest() ? directories.getWorkspace().getPathString() : "") + .setRelativePath("com/google/example/Test.java") + .setIsSource(true) + .build()); } @Test diff --git a/src/tools/android/java/com/google/devtools/build/android/ideinfo/ArtifactLocationConverter.java b/src/tools/android/java/com/google/devtools/build/android/ideinfo/ArtifactLocationConverter.java new file mode 100644 index 0000000000..318807a1b8 --- /dev/null +++ b/src/tools/android/java/com/google/devtools/build/android/ideinfo/ArtifactLocationConverter.java @@ -0,0 +1,68 @@ +// Copyright 2016 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.android.ideinfo; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Splitter; +import com.google.devtools.build.android.Converters.PathConverter; +import com.google.devtools.build.lib.ideinfo.androidstudio.PackageManifestOuterClass.ArtifactLocation; +import com.google.devtools.common.options.Converter; +import com.google.devtools.common.options.OptionsParsingException; + +import java.nio.file.Path; +import java.util.Iterator; +import java.util.NoSuchElementException; + +/** + * Parses artifact location from comma-separate paths + */ +@VisibleForTesting +public class ArtifactLocationConverter implements Converter<ArtifactLocation> { + private static final Splitter SPLITTER = Splitter.on(','); + private static final PathConverter pathConverter = new PathConverter(); + + @Override + public ArtifactLocation convert(String input) throws OptionsParsingException { + Iterator<String> values = SPLITTER.split(input).iterator(); + try { + Path rootExecutionPathFragment = pathConverter.convert(values.next()); + Path relPath = pathConverter.convert(values.next()); + + // will be removed in a future release -- make it forward-compatible + String root = values.hasNext() ? pathConverter.convert(values.next()).toString() : ""; + + if (values.hasNext()) { + throw new OptionsParsingException("Expected either 2 or 3 comma-separated paths"); + } + + boolean isSource = rootExecutionPathFragment.toString().isEmpty(); + return ArtifactLocation.newBuilder() + .setRootPath(root) + .setRootExecutionPathFragment(rootExecutionPathFragment.toString()) + .setRelativePath(relPath.toString()) + .setIsSource(isSource) + .build(); + + } catch (OptionsParsingException | NoSuchElementException e) { + throw new OptionsParsingException("Expected either 2 or 3 comma-separated paths", e); + } + } + + @Override + public String getTypeDescription() { + return "Artifact location parser"; + } + +} diff --git a/src/tools/android/java/com/google/devtools/build/android/ideinfo/ArtifactLocationListConverter.java b/src/tools/android/java/com/google/devtools/build/android/ideinfo/ArtifactLocationListConverter.java new file mode 100644 index 0000000000..a2e162a706 --- /dev/null +++ b/src/tools/android/java/com/google/devtools/build/android/ideinfo/ArtifactLocationListConverter.java @@ -0,0 +1,46 @@ +// Copyright 2016 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.android.ideinfo; + +import com.google.common.collect.Lists; +import com.google.devtools.build.lib.ideinfo.androidstudio.PackageManifestOuterClass.ArtifactLocation; +import com.google.devtools.common.options.Converter; +import com.google.devtools.common.options.OptionsParsingException; + +import java.util.Collections; +import java.util.List; + +/** + * Parses list of colon-separated artifact locations + */ +public class ArtifactLocationListConverter implements Converter<List<ArtifactLocation>> { + final ArtifactLocationConverter baseConverter = new ArtifactLocationConverter(); + + @Override + public List<ArtifactLocation> convert(String input) throws OptionsParsingException { + List<ArtifactLocation> list = Lists.newArrayList(); + for (String piece : input.split(":")) { + if (!piece.isEmpty()) { + list.add(baseConverter.convert(piece)); + } + } + return Collections.unmodifiableList(list); + } + + @Override + public String getTypeDescription() { + return "a colon-separated list of artifact locations"; + } +} diff --git a/src/tools/android/java/com/google/devtools/build/android/ideinfo/PackageParser.java b/src/tools/android/java/com/google/devtools/build/android/ideinfo/PackageParser.java index 62511d0b4f..f982a3104d 100644 --- a/src/tools/android/java/com/google/devtools/build/android/ideinfo/PackageParser.java +++ b/src/tools/android/java/com/google/devtools/build/android/ideinfo/PackageParser.java @@ -16,6 +16,7 @@ package com.google.devtools.build.android.ideinfo; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; +import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.io.Files; import com.google.common.util.concurrent.ListenableFuture; @@ -23,6 +24,7 @@ import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; import com.google.devtools.build.android.Converters.PathConverter; import com.google.devtools.build.android.Converters.PathListConverter; +import com.google.devtools.build.lib.ideinfo.androidstudio.PackageManifestOuterClass.ArtifactLocation; import com.google.devtools.build.lib.ideinfo.androidstudio.PackageManifestOuterClass.JavaSourcePackage; import com.google.devtools.build.lib.ideinfo.androidstudio.PackageManifestOuterClass.PackageManifest; import com.google.devtools.common.options.Option; @@ -34,6 +36,7 @@ import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -54,12 +57,27 @@ public class PackageParser { /** The options for a {@PackageParser} action. */ public static final class PackageParserOptions extends OptionsBase { + @Option(name = "sources", + defaultValue = "null", + converter = ArtifactLocationListConverter.class, + category = "input", + help = "The locations of the java source files. The expected format is a " + + "colon-separated list.") + public List<ArtifactLocation> sources; + + @Option(name = "output_manifest", + defaultValue = "null", + converter = PathConverter.class, + category = "output", + help = "The path to the manifest file this parser writes to.") + public Path outputManifest; + @Option(name = "sources_absolute_paths", defaultValue = "null", converter = PathListConverter.class, category = "input", help = "The absolute paths of the java source files. The expected format is a " - + "colon-separated list.") + + "colon-separated list.") public List<Path> sourcesAbsolutePaths; @Option(name = "sources_execution_paths", @@ -69,13 +87,6 @@ public class PackageParser { help = "The execution paths of the java source files. The expected format is a " + "colon-separated list.") public List<Path> sourcesExecutionPaths; - - @Option(name = "output_manifest", - defaultValue = "null", - converter = PathConverter.class, - category = "output", - help = "The path to the manifest file this parser writes to.") - public Path outputManifest; } private static final Logger logger = Logger.getLogger(PackageParser.class.getName()); @@ -85,17 +96,22 @@ public class PackageParser { public static void main(String[] args) throws Exception { PackageParserOptions options = parseArgs(args); - Preconditions.checkNotNull(options.sourcesAbsolutePaths); - Preconditions.checkNotNull(options.sourcesExecutionPaths); - Preconditions.checkState( - options.sourcesAbsolutePaths.size() == options.sourcesExecutionPaths.size()); Preconditions.checkNotNull(options.outputManifest); + // temporary code to handle output from older builds + boolean oldFormat = options.sources == null; + if (oldFormat) { + Preconditions.checkNotNull(options.sourcesAbsolutePaths); + Preconditions.checkNotNull(options.sourcesExecutionPaths); + Preconditions.checkState( + options.sourcesAbsolutePaths.size() == options.sourcesExecutionPaths.size()); + convertFromOldFormat(options); + } + try { PackageParser parser = new PackageParser(PackageParserIoProvider.INSTANCE); - Map<Path, String> outputMap = parser.parsePackageStrings(options.sourcesAbsolutePaths, - options.sourcesExecutionPaths); - parser.writeManifest(outputMap, options.outputManifest); + Map<ArtifactLocation, String> outputMap = parser.parsePackageStrings(options.sources); + parser.writeManifest(outputMap, options.outputManifest, oldFormat); } catch (Throwable e) { logger.log(Level.SEVERE, "Error parsing package strings", e); System.exit(1); @@ -104,6 +120,43 @@ public class PackageParser { } @VisibleForTesting + @Deprecated + protected static void convertFromOldFormat(PackageParserOptions options) { + options.sources = Lists.newArrayList(); + for (int i = 0; i < options.sourcesAbsolutePaths.size(); i++) { + options.sources.add(dummySourceFromOldFormat( + options.sourcesAbsolutePaths.get(i).toString(), + options.sourcesExecutionPaths.get(i).toString())); + } + } + + @Deprecated + private static ArtifactLocation dummySourceFromOldFormat( + String absolutePath, + String executionPath) { + if (!absolutePath.endsWith(executionPath)) { + throw new IllegalArgumentException( + String.format("Cannot parse root path from absolute path %s and execution path %s", + absolutePath, executionPath)); + } + String rootPath = absolutePath.substring(0, absolutePath.lastIndexOf(executionPath)); + return ArtifactLocation.newBuilder() + .setRelativePath(executionPath) + .setRootPath(rootPath) + .build(); + } + + @Nonnull + private static Path getExecutionPath(@Nonnull ArtifactLocation location) { + return Paths.get(location.getRootExecutionPathFragment(), location.getRelativePath()); + } + + @Nonnull + private static Path getAbsolutePath(@Nonnull ArtifactLocation location) { + return Paths.get(location.getRootPath(), location.getRelativePath()); + } + + @VisibleForTesting public static PackageParserOptions parseArgs(String[] args) { args = parseParamFileIfUsed(args); OptionsParser optionsParser = OptionsParser.newOptionsParser(PackageParserOptions.class); @@ -131,13 +184,20 @@ public class PackageParser { } @VisibleForTesting - public void writeManifest(@Nonnull Map<Path, String> sourceToPackageMap, Path outputFile) + public void writeManifest( + @Nonnull Map<ArtifactLocation, String> sourceToPackageMap, + Path outputFile, + boolean oldFormat) throws IOException { PackageManifest.Builder builder = PackageManifest.newBuilder(); - for (Entry<Path, String> entry : sourceToPackageMap.entrySet()) { - builder.addSources(JavaSourcePackage.newBuilder() - .setAbsolutePath(entry.getKey().toAbsolutePath().toString()) - .setPackageString(entry.getValue())); + for (Entry<ArtifactLocation, String> entry : sourceToPackageMap.entrySet()) { + JavaSourcePackage.Builder srcBuilder = JavaSourcePackage.newBuilder() + .setAbsolutePath(getAbsolutePath(entry.getKey()).toString()) + .setPackageString(entry.getValue()); + if (!oldFormat) { + srcBuilder.setArtifactLocation(entry.getKey()); + } + builder.addSources(srcBuilder.build()); } try { @@ -150,24 +210,23 @@ public class PackageParser { @Nonnull @VisibleForTesting - public Map<Path, String> parsePackageStrings(@Nonnull List<Path> absolutePaths, - @Nonnull List<Path> executionPaths) throws Exception { + public Map<ArtifactLocation, String> parsePackageStrings(@Nonnull List<ArtifactLocation> sources) + throws Exception { ListeningExecutorService executorService = MoreExecutors.listeningDecorator( Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors())); - Map<Path, ListenableFuture<String>> futures = Maps.newHashMap(); - for (int i = 0; i < absolutePaths.size(); i++) { - final Path source = executionPaths.get(i); - futures.put(absolutePaths.get(i), executorService.submit(new Callable<String>() { + Map<ArtifactLocation, ListenableFuture<String>> futures = Maps.newHashMap(); + for (final ArtifactLocation source : sources) { + futures.put(source, executorService.submit(new Callable<String>() { @Override public String call() throws Exception { return getDeclaredPackageOfJavaFile(source); } })); } - Map<Path, String> map = Maps.newHashMap(); - for (Entry<Path, ListenableFuture<String>> entry : futures.entrySet()) { + Map<ArtifactLocation, String> map = Maps.newHashMap(); + for (Entry<ArtifactLocation, ListenableFuture<String>> entry : futures.entrySet()) { String value = entry.getValue().get(); if (value != null) { map.put(entry.getKey(), value); @@ -177,8 +236,8 @@ public class PackageParser { } @Nullable - private String getDeclaredPackageOfJavaFile(@Nonnull Path source) { - try (BufferedReader reader = ioProvider.getReader(source)) { + private String getDeclaredPackageOfJavaFile(@Nonnull ArtifactLocation source) { + try (BufferedReader reader = ioProvider.getReader(getExecutionPath(source))) { return parseDeclaredPackage(reader); } catch (IOException e) { |