Roll forward of commit 500175fcfb37953f50cf0869df164902755807f2: Don't include absolute paths in blaze IDE
artifacts
NEW: Fixed bazel java_test rules
Rollback of commit a4ba24e286b7e9a7a09e333d8eb64a8cf7e81895.
*** Reason for rollback ***
Rollforward, fixing bazel java_test rules
*** Original change description ***
Automated [] rollback of commit 500175fcfb37953f50cf0869df164902755807f2.
*** Reason for rollback ***
Breaks Bazel Build
http://ci.bazel.io/job/Bazel/JAVA_VERSION=1.7,PLATFORM_NAME=linux-x86_64/356/console
*** Original change description ***
Don't include absolute paths in blaze IDE artifacts
RELNOTES: Don't include absolute paths in blaze IDE artifacts
--
MOS_MIGRATED_REVID=114751236
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 f6d4856..472bef6 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.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.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 @@
.getPath()
.toString());
+ outputBuilder.setBuildFileArtifactLocation(
+ makeArtifactLocation(ruleContext.getRule().getPackage()));
+
outputBuilder.setKind(ruleKind);
if (ruleKind == Kind.JAVA_LIBRARY
@@ -373,8 +379,7 @@
.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 @@
.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 @@
}
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 7028a45..0cd44ad 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 @@
@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 @@
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 @@
@Override
public int hashCode() {
- return Objects.hash(relativePath, rootPath, isSource);
+ return Objects.hash(relativePath, rootPath, rootExecutionPathFragment, isSource);
}
@Override
@@ -77,6 +104,7 @@
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 @@
@Override
public String toString() {
return "SourceDirectory [relativePath=" + relativePath + ", rootPath=" + rootPath
- + ", isSource=" + isSource + "]";
+ + ", executionRootPrefix=" + rootExecutionPathFragment + ", isSource=" + isSource + "]";
}
}
@@ -179,10 +207,9 @@
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 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 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 0be192e..0b099c6 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_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 @@
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 @@
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 dd5f4fc..b714f8a 100644
--- a/src/main/protobuf/package_manifest.proto
+++ b/src/main/protobuf/package_manifest.proto
@@ -21,9 +21,23 @@
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 0000000..0ac6311
--- /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 79f75dd..f268b1e 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 = ["PackageParserTest.java"],
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 @@
"//third_party:truth",
],
)
+
+java_test(
+ name = "ArtifactLocationConverterTest",
+ size = "small",
+ srcs = ["ArtifactLocationConverterTest.java"],
+ 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 a02ac30..3f466d6 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 @@
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.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 @@
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 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 @@
}
}
+ 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 @@
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 @@
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 6810d5d..22416fc 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.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 @@
"//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 @@
")");
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 0000000..318807a
--- /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 0000000..a2e162a
--- /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 62511d0..f982a31 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 @@
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.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.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 @@
/** 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 @@
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 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 @@
}
@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 @@
}
@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 @@
@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 @@
}
@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) {
