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=114682419
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 472bef6..f6d4856 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,10 +20,8 @@
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;
@@ -58,7 +56,6 @@
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;
@@ -306,9 +303,6 @@
.getPath()
.toString());
- outputBuilder.setBuildFileArtifactLocation(
- makeArtifactLocation(ruleContext.getRule().getPackage()));
-
outputBuilder.setKind(ruleKind);
if (ruleKind == Kind.JAVA_LIBRARY
@@ -379,7 +373,8 @@
.setExecutable(ruleContext.getExecutablePrerequisite("$packageParser", Mode.HOST))
.setCommandLine(CustomCommandLine.builder()
.addExecPath("--output_manifest", packageManifest)
- .addJoinStrings("--sources", ":", toSerializedArtifactLocations(sourceFiles))
+ .addJoinStrings("--sources_absolute_paths", ":", Artifact.toAbsolutePaths(sourceFiles))
+ .addJoinExecPaths("--sources_execution_paths", ":", sourceFiles)
.build())
.useParameterFile(ParameterFileType.SHELL_QUOTED)
.setProgressMessage("Parsing java package strings for " + ruleContext.getRule())
@@ -387,25 +382,6 @@
.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();
@@ -495,28 +471,16 @@
}
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(root.getPath().toString())
- .setRootExecutionPathFragment(root.getExecPath().toString())
- .setRelativePath(relativePath.toString())
- .setIsSource(root.isSourceRoot())
+ .setRootPath(artifact.getRoot().getPath().toString())
+ .setRelativePath(artifact.getRootRelativePathString())
+ .setIsSource(artifact.isSourceArtifact())
.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 0cd44ad..7028a45 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,7 +13,6 @@
// 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;
@@ -40,29 +39,11 @@
@Immutable
public static class SourceDirectory {
final PathFragment relativePath;
- final PathFragment rootExecutionPathFragment;
final PathFragment rootPath;
final 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) {
+ public SourceDirectory(PathFragment rootPath, PathFragment relativePath, boolean isSource) {
this.rootPath = rootPath;
- this.rootExecutionPathFragment = rootExecutionPathFragment;
this.relativePath = relativePath;
this.isSource = isSource;
}
@@ -81,14 +62,6 @@
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;
@@ -96,7 +69,7 @@
@Override
public int hashCode() {
- return Objects.hash(relativePath, rootPath, rootExecutionPathFragment, isSource);
+ return Objects.hash(relativePath, rootPath, isSource);
}
@Override
@@ -104,7 +77,6 @@
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);
}
@@ -114,7 +86,7 @@
@Override
public String toString() {
return "SourceDirectory [relativePath=" + relativePath + ", rootPath=" + rootPath
- + ", executionRootPrefix=" + rootExecutionPathFragment + ", isSource=" + isSource + "]";
+ + ", isSource=" + isSource + "]";
}
}
@@ -207,9 +179,10 @@
private void addIdlDirs(Collection<Artifact> idlArtifacts) {
for (Artifact idl : idlArtifacts) {
this.idlDirs.add(
- SourceDirectory.fromRoot(
- idl.getRoot(),
- idl.getRootRelativePath().getParentDirectory()));
+ new SourceDirectory(
+ idl.getRoot().getPath().asFragment(),
+ idl.getRootRelativePath().getParentDirectory(),
+ idl.isSourceArtifact()));
}
}
@@ -226,9 +199,10 @@
public Builder addResourceSource(Artifact resource) {
PathFragment resourceDir = LocalResourceContainer.Builder.findResourceDir(resource);
resourceDirs.add(
- SourceDirectory.fromRoot(
- resource.getRoot(),
- trimTo(resource.getRootRelativePath(), resourceDir)));
+ new SourceDirectory(
+ resource.getRoot().getPath().asFragment(),
+ trimTo(resource.getRootRelativePath(), resourceDir),
+ resource.isSourceArtifact()));
return this;
}
@@ -248,9 +222,10 @@
public Builder addAssetSource(Artifact asset, PathFragment assetDir) {
assetDirs.add(
- SourceDirectory.fromRoot(
- asset.getRoot(),
- trimTo(asset.getRootRelativePath(), assetDir)));
+ new SourceDirectory(
+ asset.getRoot().getPath().asFragment(),
+ trimTo(asset.getRootRelativePath(), assetDir),
+ asset.isSourceArtifact()));
return this;
}
diff --git a/src/main/protobuf/android_studio_ide_info.proto b/src/main/protobuf/android_studio_ide_info.proto
index 0b099c6..0be192e 100644
--- a/src/main/protobuf/android_studio_ide_info.proto
+++ b/src/main/protobuf/android_studio_ide_info.proto
@@ -22,14 +22,9 @@
option java_generate_equals_and_hash = true;
message ArtifactLocation {
- // deprecated -- this is not machine agnostic and will be removed in a future release
- string root_path = 1 [deprecated=true];
-
+ string root_path = 1;
string relative_path = 2;
bool is_source = 3;
-
- // set for derived artifacts (isSource = false)
- string root_execution_path_fragment = 4;
}
message LibraryArtifact {
@@ -78,8 +73,7 @@
string label = 1;
Kind kind = 2;
- // deprecated -- this is not machine agnostic and will be removed in a future release
- string build_file = 3 [deprecated=true];
+ string build_file = 3;
repeated string dependencies = 4;
// kind is ANDROID_SDK
@@ -91,6 +85,4 @@
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 b714f8a..dd5f4fc 100644
--- a/src/main/protobuf/package_manifest.proto
+++ b/src/main/protobuf/package_manifest.proto
@@ -21,23 +21,9 @@
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 {
- // deprecated -- this is not machine agnostic and will be removed in a future release
- string absolute_path = 1 [deprecated=true];
+ string absolute_path = 1;
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
deleted file mode 100644
index 0ac6311..0000000
--- a/src/test/java/com/google/devtools/build/android/ideinfo/ArtifactLocationConverterTest.java
+++ /dev/null
@@ -1,96 +0,0 @@
-// 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 aa917ed..79f75dd 100644
--- a/src/test/java/com/google/devtools/build/android/ideinfo/BUILD
+++ b/src/test/java/com/google/devtools/build/android/ideinfo/BUILD
@@ -1,9 +1,8 @@
java_test(
name = "PackageParserTest",
size = "small",
- srcs = ["com.google.devtools.build.android.ideinfo.PackageParserTest"],
+ srcs = glob(["*.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",
@@ -13,18 +12,3 @@
"//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 3f466d6..a02ac30 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,11 +17,9 @@
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;
@@ -37,7 +35,6 @@
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;
@@ -53,16 +50,12 @@
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(ArtifactLocation source, String javaSrc) {
+ public MockPackageParserIoProvider addSource(String filePath, String javaSrc) {
try {
- Path path = Paths.get(source.getRootExecutionPathFragment(), source.getRelativePath());
- sources.put(path, new ByteArrayInputStream(javaSrc.getBytes("UTF-8")));
- sourceLocations.add(source);
-
- } catch (UnsupportedEncodingException | InvalidPathException e) {
+ sources.put(Paths.get(filePath), new ByteArrayInputStream(javaSrc.getBytes("UTF-8")));
+ } catch (UnsupportedEncodingException e) {
fail(e.getMessage());
}
return this;
@@ -70,12 +63,11 @@
public void reset() {
sources.clear();
- sourceLocations.clear();
writer = new StringWriter();
}
- public List<ArtifactLocation> getSourceLocations() {
- return Lists.newArrayList(sourceLocations);
+ public List<Path> getPaths() {
+ return Lists.newArrayList(sources.keySet());
}
@Nonnull
@@ -91,21 +83,6 @@
}
}
- 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;
@@ -115,9 +92,9 @@
parser = new PackageParser(mockIoProvider);
}
- private Map<ArtifactLocation, String> parsePackageStrings() throws Exception {
- List<ArtifactLocation> sources = mockIoProvider.getSourceLocations();
- return parser.parsePackageStrings(sources);
+ private Map<Path, String> parsePackageStrings() throws Exception {
+ List<Path> paths = mockIoProvider.getPaths();
+ return parser.parsePackageStrings(paths, paths);
}
@Test
@@ -125,205 +102,91 @@
String[] args = new String[] {
"--output_manifest",
"/tmp/out.manifest",
- "--sources",
- Joiner.on(":").join(
- ",java/com/google/Foo.java,/usr/local/google/code",
- "bin,java/com/google/Bla.java,/usr/local/_tmp/code/bin"
- )};
+ "--sources_absolute_paths",
+ "/path/test1.java:/path/test2.java",
+ "--sources_execution_paths",
+ "/path/test1.java:/path/test2.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()
- .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());
+ 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");
}
@Test
public void testReadNoSources() throws Exception {
- Map<ArtifactLocation, String> map = parsePackageStrings();
+ Map<Path, String> map = parsePackageStrings();
assertThat(map).isEmpty();
}
@Test
public void testSingleRead() throws Exception {
mockIoProvider
- .addSource(
- DUMMY_SOURCE_ARTIFACT,
- "package com.google;\n public class Bla {}\"");
- Map<ArtifactLocation, String> map = parsePackageStrings();
+ .addSource("java/com/google/Bla.java",
+ "package com.test;\n public class Bla {}\"");
+ Map<Path, String> map = parsePackageStrings();
assertThat(map).hasSize(1);
- assertThat(map).containsEntry(
- DUMMY_SOURCE_ARTIFACT,
- "com.google");
+ assertThat(map).containsEntry(Paths.get("java/com/google/Bla.java"), "com.test");
}
@Test
public void testMultiRead() throws Exception {
mockIoProvider
- .addSource(
- DUMMY_SOURCE_ARTIFACT,
+ .addSource("java/com/google/Bla.java",
"package com.test;\n public class Bla {}\"")
- .addSource(
- DUMMY_DERIVED_ARTIFACT,
+ .addSource("java/com/other/Foo.java",
"package com.other;\n public class Foo {}\"");
- Map<ArtifactLocation, String> map = parsePackageStrings();
+ Map<Path, String> map = parsePackageStrings();
assertThat(map).hasSize(2);
- assertThat(map).containsEntry(
- DUMMY_SOURCE_ARTIFACT,
- "com.test");
- assertThat(map).containsEntry(
- DUMMY_DERIVED_ARTIFACT,
- "com.other");
+ assertThat(map).containsEntry(Paths.get("java/com/google/Bla.java"), "com.test");
+ assertThat(map).containsEntry(Paths.get("java/com/other/Foo.java"), "com.other");
}
@Test
public void testReadSomeInvalid() throws Exception {
mockIoProvider
- .addSource(
- DUMMY_SOURCE_ARTIFACT,
+ .addSource("java/com/google/Bla.java",
"package %com.test;\n public class Bla {}\"")
- .addSource(
- DUMMY_DERIVED_ARTIFACT,
+ .addSource("java/com/other/Foo.java",
"package com.other;\n public class Foo {}\"");
- Map<ArtifactLocation, String> map = parsePackageStrings();
+ Map<Path, String> map = parsePackageStrings();
assertThat(map).hasSize(1);
- assertThat(map).containsEntry(
- DUMMY_DERIVED_ARTIFACT,
- "com.other");
+ assertThat(map).containsEntry(Paths.get("java/com/other/Foo.java"), "com.other");
}
@Test
public void testReadAllInvalid() throws Exception {
mockIoProvider
- .addSource(
- DUMMY_SOURCE_ARTIFACT,
+ .addSource("java/com/google/Bla.java",
"#package com.test;\n public class Bla {}\"")
- .addSource(
- DUMMY_DERIVED_ARTIFACT,
+ .addSource("java/com/other/Foo.java",
"package com.other\n public class Foo {}\"");
- Map<ArtifactLocation, String> map = parsePackageStrings();
+ Map<Path, String> map = parsePackageStrings();
assertThat(map).isEmpty();
}
@Test
public void testWriteEmptyMap() throws Exception {
parser.writeManifest(
- Maps.<ArtifactLocation, String> newHashMap(),
- Paths.get("/java/com/google/test.manifest"),
- false);
+ Maps.<Path, String> newHashMap(), Paths.get("/java/com/google/test.manifest"));
assertThat(mockIoProvider.writer.toString()).isEmpty();
}
@Test
public void testWriteMap() throws Exception {
- Map<ArtifactLocation, String> map = ImmutableMap.of(
- DUMMY_SOURCE_ARTIFACT,
- "com.google",
- DUMMY_DERIVED_ARTIFACT,
- "com.other"
+ Map<Path, String> map = ImmutableMap.of(
+ Paths.get("/java/com/google/Bla.java"), "com.google",
+ Paths.get("/java/com/other/Foo.java"), "com.other"
);
- parser.writeManifest(map, Paths.get("/java/com/google/test.manifest"), false);
+ parser.writeManifest(map, Paths.get("/java/com/google/test.manifest"));
String writtenString = mockIoProvider.writer.toString();
- 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("absolute_path: \"/java/com/google/Bla.java\"");
assertThat(writtenString).contains("package_string: \"com.google\"");
-
- 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("absolute_path: \"/java/com/other/Foo.java\"");
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 22416fc..6810d5d 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,7 +43,6 @@
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;
@@ -70,10 +69,6 @@
"//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);
@@ -912,21 +907,26 @@
")");
Map<String, RuleIdeInfo> ruleIdeInfos = buildRuleIdeInfo("//com/google/example:lib");
RuleIdeInfo ruleIdeInfo = getRuleInfoAndVerifyLabel("//com/google/example:lib", ruleIdeInfos);
- // 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());
+ 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();
+ }
}
@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
deleted file mode 100644
index 318807a..0000000
--- a/src/tools/android/java/com/google/devtools/build/android/ideinfo/ArtifactLocationConverter.java
+++ /dev/null
@@ -1,68 +0,0 @@
-// 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
deleted file mode 100644
index a2e162a..0000000
--- a/src/tools/android/java/com/google/devtools/build/android/ideinfo/ArtifactLocationListConverter.java
+++ /dev/null
@@ -1,46 +0,0 @@
-// 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 f982a31..62511d0 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,7 +16,6 @@
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;
@@ -24,7 +23,6 @@
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;
@@ -36,7 +34,6 @@
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;
@@ -57,27 +54,12 @@
/** 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",
@@ -87,6 +69,13 @@
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());
@@ -96,22 +85,17 @@
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<ArtifactLocation, String> outputMap = parser.parsePackageStrings(options.sources);
- parser.writeManifest(outputMap, options.outputManifest, oldFormat);
+ Map<Path, String> outputMap = parser.parsePackageStrings(options.sourcesAbsolutePaths,
+ options.sourcesExecutionPaths);
+ parser.writeManifest(outputMap, options.outputManifest);
} catch (Throwable e) {
logger.log(Level.SEVERE, "Error parsing package strings", e);
System.exit(1);
@@ -120,43 +104,6 @@
}
@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);
@@ -184,20 +131,13 @@
}
@VisibleForTesting
- public void writeManifest(
- @Nonnull Map<ArtifactLocation, String> sourceToPackageMap,
- Path outputFile,
- boolean oldFormat)
+ public void writeManifest(@Nonnull Map<Path, String> sourceToPackageMap, Path outputFile)
throws IOException {
PackageManifest.Builder builder = PackageManifest.newBuilder();
- 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());
+ for (Entry<Path, String> entry : sourceToPackageMap.entrySet()) {
+ builder.addSources(JavaSourcePackage.newBuilder()
+ .setAbsolutePath(entry.getKey().toAbsolutePath().toString())
+ .setPackageString(entry.getValue()));
}
try {
@@ -210,23 +150,24 @@
@Nonnull
@VisibleForTesting
- public Map<ArtifactLocation, String> parsePackageStrings(@Nonnull List<ArtifactLocation> sources)
- throws Exception {
+ public Map<Path, String> parsePackageStrings(@Nonnull List<Path> absolutePaths,
+ @Nonnull List<Path> executionPaths) throws Exception {
ListeningExecutorService executorService = MoreExecutors.listeningDecorator(
Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors()));
- Map<ArtifactLocation, ListenableFuture<String>> futures = Maps.newHashMap();
- for (final ArtifactLocation source : sources) {
- futures.put(source, executorService.submit(new Callable<String>() {
+ 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>() {
@Override
public String call() throws Exception {
return getDeclaredPackageOfJavaFile(source);
}
}));
}
- Map<ArtifactLocation, String> map = Maps.newHashMap();
- for (Entry<ArtifactLocation, ListenableFuture<String>> entry : futures.entrySet()) {
+ Map<Path, String> map = Maps.newHashMap();
+ for (Entry<Path, ListenableFuture<String>> entry : futures.entrySet()) {
String value = entry.getValue().get();
if (value != null) {
map.put(entry.getKey(), value);
@@ -236,8 +177,8 @@
}
@Nullable
- private String getDeclaredPackageOfJavaFile(@Nonnull ArtifactLocation source) {
- try (BufferedReader reader = ioProvider.getReader(getExecutionPath(source))) {
+ private String getDeclaredPackageOfJavaFile(@Nonnull Path source) {
+ try (BufferedReader reader = ioProvider.getReader(source)) {
return parseDeclaredPackage(reader);
} catch (IOException e) {
