From 4833822ced449008ffc57d217334edac3122ef7e Mon Sep 17 00:00:00 2001 From: Lukacs Berki Date: Fri, 12 Jun 2015 11:37:46 +0000 Subject: Remove Path from Location, ParserInputSource and bunch of other low-level classes. This makes the code cleaner because a lot of places never read the file and thus never needed a Path in the first place. I got to this change in a bit convoluted way: - I wanted the default tools in Android rules to point to //external: - I wanted to make sure that that doesn't cause an error is no Android rules are built, thus I had to add some binding for them in the default WORKSPACE file - I wanted the Android rules not to depend on Bazel core with an eye towards eventually moving them to a separate jar / Skylark code - The default WORKSPACE file is currently composed from files extracted by the Bazel launcher which would make the Android rules depend on a very core mechanism - I couldn't simply pass in jdk.WORKSPACE as a String because Location, ParserInputSource and a bunch of other things needed a Path, which a simple string doesn't have. Thus, this change. -- MOS_MIGRATED_REVID=95828839 --- .../google/devtools/build/lib/events/Location.java | 19 +++++++++++-------- .../build/lib/packages/PackageFactory.java | 4 ++-- .../devtools/build/lib/packages/Preprocessor.java | 6 +++--- .../build/lib/skyframe/PackageFunction.java | 2 +- .../devtools/build/lib/syntax/BuildFileAST.java | 6 ++++-- .../google/devtools/build/lib/syntax/Lexer.java | 15 +++++++-------- .../devtools/build/lib/syntax/LineNumberTable.java | 22 +++++++++++----------- .../google/devtools/build/lib/syntax/Parser.java | 7 ++++--- .../build/lib/syntax/ParserInputSource.java | 21 +++++++-------------- 9 files changed, 50 insertions(+), 52 deletions(-) (limited to 'src/main') diff --git a/src/main/java/com/google/devtools/build/lib/events/Location.java b/src/main/java/com/google/devtools/build/lib/events/Location.java index c16c244c5d..160d4a586d 100644 --- a/src/main/java/com/google/devtools/build/lib/events/Location.java +++ b/src/main/java/com/google/devtools/build/lib/events/Location.java @@ -23,11 +23,11 @@ import java.io.Serializable; /** * A Location is a range of characters within a file. * - * The start and end locations may be the same, in which case the Location + *

The start and end locations may be the same, in which case the Location * denotes a point in the file, not a range. The path may be null, indicating * an unknown file. * - * Implementations of Location should be optimised for speed of construction, + *

Implementations of Location should be optimised for speed of construction, * not speed of attribute access, as far more Locations are created during * parsing than are ever used to display error messages. */ @@ -38,10 +38,10 @@ public abstract class Location implements Serializable { private final PathFragment path; private final LineAndColumn startLineAndColumn; - private LocationWithPathAndStartColumn(Path path, int startOffSet, int endOffSet, + private LocationWithPathAndStartColumn(PathFragment path, int startOffSet, int endOffSet, LineAndColumn startLineAndColumn) { super(startOffSet, endOffSet); - this.path = path != null ? path.asFragment() : null; + this.path = path; this.startLineAndColumn = startLineAndColumn; } @@ -60,7 +60,7 @@ public abstract class Location implements Serializable { /** * Returns a Location with a given Path, start and end offset and start line and column info. */ - public static Location fromPathAndStartColumn(Path path, int startOffSet, int endOffSet, + public static Location fromPathAndStartColumn(PathFragment path, int startOffSet, int endOffSet, LineAndColumn startLineAndColumn) { return new LocationWithPathAndStartColumn(path, startOffSet, endOffSet, startLineAndColumn); } @@ -70,14 +70,17 @@ public abstract class Location implements Serializable { * region within the file. Try to use a more specific location if possible. */ public static Location fromFile(Path path) { - return fromFileAndOffsets(path, 0, 0); + return fromFileAndOffsets(path.asFragment(), 0, 0); } + public static Location fromPathFragment(PathFragment path) { + return fromFileAndOffsets(path, 0, 0); + } /** * Returns a Location relating to the subset of file 'path', starting at * 'startOffset' and ending at 'endOffset'. */ - public static Location fromFileAndOffsets(final Path path, + public static Location fromFileAndOffsets(final PathFragment path, int startOffset, int endOffset) { return new LocationWithPathAndStartColumn(path, startOffset, endOffset, null); @@ -113,7 +116,7 @@ public abstract class Location implements Serializable { * Returns the path of the file to which the start/end offsets refer. May be * null if the file name information is not available. * - * This method is intentionally abstract, as a space optimisation. Some + *

This method is intentionally abstract, as a space optimisation. Some * subclass instances implement sharing of common data (e.g. tables for * convering offsets into line numbers) and this enables them to share the * Path value in the same way. diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java index 02c7f03d1f..871b3d7997 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java @@ -1097,7 +1097,7 @@ public final class PackageFactory { EventHandler eventHandler) throws InterruptedException { ParserInputSource inputSource = maybeGetParserInputSource(buildFile, eventHandler); if (inputSource == null) { - return Preprocessor.Result.transientError(buildFile); + return Preprocessor.Result.transientError(buildFile.asFragment()); } Globber globber = createLegacyGlobber(buildFile.getParentDirectory(), packageId, locator); try { @@ -1128,7 +1128,7 @@ public final class PackageFactory { } catch (IOException e) { eventHandler.handle(Event.error(Location.fromFile(buildFile), "preprocessing failed: " + e.getMessage())); - return Preprocessor.Result.transientError(buildFile); + return Preprocessor.Result.transientError(buildFile.asFragment()); } catch (InterruptedException e) { globber.onInterrupt(); throw e; diff --git a/src/main/java/com/google/devtools/build/lib/packages/Preprocessor.java b/src/main/java/com/google/devtools/build/lib/packages/Preprocessor.java index 8eda1e9dec..834fcaef36 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Preprocessor.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Preprocessor.java @@ -17,7 +17,7 @@ import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.packages.PackageFactory.Globber; import com.google.devtools.build.lib.syntax.Environment; import com.google.devtools.build.lib.syntax.ParserInputSource; -import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; import java.util.Set; @@ -119,12 +119,12 @@ public interface Preprocessor { /*containsTransientErrors=*/false); } - public static Result invalidSyntax(Path buildFile) { + public static Result invalidSyntax(PathFragment buildFile) { return new Result(ParserInputSource.create("", buildFile), /*preprocessed=*/true, /*containsPersistentErrors=*/true, /*containsTransientErrors=*/false); } - public static Result transientError(Path buildFile) { + public static Result transientError(PathFragment buildFile) { return new Result(ParserInputSource.create("", buildFile), /*preprocessed=*/false, /*containsPersistentErrors=*/false, /*containsTransientErrors=*/true); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java index 862e87b4fb..8aac499d6e 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java @@ -712,7 +712,7 @@ public class PackageFunction implements SkyFunction { List preludeStatements, SkylarkImportResult importResult) throws InterruptedException { ParserInputSource replacementSource = replacementContents == null ? null - : ParserInputSource.create(replacementContents, buildFilePath); + : ParserInputSource.create(replacementContents, buildFilePath.asFragment()); Package.LegacyBuilder pkgBuilder = packageFunctionCache.getIfPresent(packageId); if (pkgBuilder == null) { profiler.startTask(ProfilerTask.CREATE_PACKAGE, packageId.toString()); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java index 0645cb3983..800aa65535 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.syntax; import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.hash.HashCode; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.Location; @@ -67,7 +68,7 @@ public class BuildFileAST extends ASTNode { result.statements.get(0).getLocation().getStartOffset(), result.statements.get(result.statements.size() - 1).getLocation().getEndOffset())); } else { - setLocation(Location.fromFile(lexer.getFilename())); + setLocation(Location.fromPathFragment(lexer.getFilename())); } } @@ -222,7 +223,8 @@ public class BuildFileAST extends ASTNode { Lexer lexer = new Lexer(input, eventHandler, false); Parser.ParseResult result = Parser.parseFileForSkylark(lexer, eventHandler, locator, validationEnvironment); - return new BuildFileAST(lexer, ImmutableList.of(), result, input.contentHashCode()); + return new BuildFileAST(lexer, ImmutableList.of(), result, + HashCode.fromBytes(file.getMD5Digest()).toString()); } /** diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java b/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java index 77cdf3f5ee..5f6c769ebb 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java @@ -20,7 +20,6 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.util.Pair; -import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; @@ -62,8 +61,8 @@ public final class Lexer { */ private static class LocationInfo { final LineNumberTable lineNumberTable; - final Path filename; - LocationInfo(Path filename, LineNumberTable lineNumberTable) { + final PathFragment filename; + LocationInfo(PathFragment filename, LineNumberTable lineNumberTable) { this.filename = filename; this.lineNumberTable = lineNumberTable; } @@ -94,8 +93,8 @@ public final class Lexer { this.pos = 0; this.parsePython = parsePython; this.eventHandler = eventHandler; - this.locationInfo = new LocationInfo(input.getPath(), - LineNumberTable.create(buffer, input.getPath())); + this.locationInfo = + new LocationInfo(input.getPath(), LineNumberTable.create(buffer, input.getPath())); indentStack.push(0); tokenize(); @@ -109,7 +108,7 @@ public final class Lexer { * Returns the filename from which the lexer's input came. Returns a dummy * value if the input came from a string. */ - public Path getFilename() { + public PathFragment getFilename() { return locationInfo.filename; } @@ -164,8 +163,8 @@ public final class Lexer { @Override public PathFragment getPath() { - Path path = lineNumberTable.getPath(getStartOffset()); - return path != null ? path.asFragment() : null; + PathFragment path = lineNumberTable.getPath(getStartOffset()); + return path; } @Override diff --git a/src/main/java/com/google/devtools/build/lib/syntax/LineNumberTable.java b/src/main/java/com/google/devtools/build/lib/syntax/LineNumberTable.java index c006bf9e5e..653abee1f6 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/LineNumberTable.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/LineNumberTable.java @@ -20,7 +20,7 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.events.Location.LineAndColumn; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.StringUtilities; -import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; import java.io.Serializable; import java.util.ArrayList; @@ -50,9 +50,9 @@ abstract class LineNumberTable implements Serializable { /** * Returns the path corresponding to the given offset. */ - abstract Path getPath(int offset); + abstract PathFragment getPath(int offset); - static LineNumberTable create(char[] buffer, Path path) { + static LineNumberTable create(char[] buffer, PathFragment path) { // If #line appears within a BUILD file, we assume it has been preprocessed // by gconfig2blaze. We ignore all actual newlines and compute the logical // LNT based only on the presence of #line markers. @@ -72,10 +72,10 @@ abstract class LineNumberTable implements Serializable { * A mapping from line number (line >= 1) to character offset into the file. */ private final int[] linestart; - private final Path path; + private final PathFragment path; private final int bufferLength; - private Regular(char[] buffer, Path path) { + private Regular(char[] buffer, PathFragment path) { // Compute the size. int size = 2; for (int i = 0; i < buffer.length; i++) { @@ -132,7 +132,7 @@ abstract class LineNumberTable implements Serializable { } @Override - Path getPath(int offset) { + PathFragment getPath(int offset) { return path; } @@ -161,9 +161,9 @@ abstract class LineNumberTable implements Serializable { private static class SingleHashLine implements Serializable { private final int offset; private final int line; - private final Path path; + private final PathFragment path; - SingleHashLine(int offset, int line, Path path) { + SingleHashLine(int offset, int line, PathFragment path) { this.offset = offset; this.line = line; this.path = path; @@ -181,10 +181,10 @@ abstract class LineNumberTable implements Serializable { private static final Pattern pattern = Pattern.compile("\n#line ([0-9]+) \"([^\"\\n]+)\""); private final List table; - private final Path defaultPath; + private final PathFragment defaultPath; private final int bufferLength; - private HashLine(char[] buffer, Path defaultPath) { + private HashLine(char[] buffer, PathFragment defaultPath) { // Not especially efficient, but that's fine: we just exec'd Python. String bufString = new String(buffer); Matcher m = pattern.matcher(bufString); @@ -223,7 +223,7 @@ abstract class LineNumberTable implements Serializable { } @Override - Path getPath(int offset) { + PathFragment getPath(int offset) { SingleHashLine hashLine = getHashLine(offset); return hashLine == null ? defaultPath : hashLine.path; } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java index 71277a0b4c..8ea20eb7d3 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java @@ -31,6 +31,7 @@ import com.google.devtools.build.lib.packages.CachingPackageLocator; import com.google.devtools.build.lib.syntax.DictionaryLiteral.DictionaryEntryLiteral; import com.google.devtools.build.lib.syntax.IfStatement.ConditionalStatements; import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; import java.util.ArrayList; @@ -159,7 +160,7 @@ class Parser { private CachingPackageLocator locator; - private List includedFiles; + private List includedFiles; private Parser( Lexer lexer, @@ -252,7 +253,7 @@ class Parser { return result; } - private void addIncludedFiles(List files) { + private void addIncludedFiles(List files) { this.includedFiles.addAll(files); } @@ -622,7 +623,7 @@ class Parser { Path path = packagePath.getParentDirectory(); Path file = path.getRelative(label.getName()); - if (this.includedFiles.contains(file)) { + if (this.includedFiles.contains(file.asFragment())) { reportError(location, "Recursive inclusion of file '" + path + "'"); return; } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ParserInputSource.java b/src/main/java/com/google/devtools/build/lib/syntax/ParserInputSource.java index e83cec639f..6cfdc94b00 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/ParserInputSource.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/ParserInputSource.java @@ -13,9 +13,9 @@ // limitations under the License. package com.google.devtools.build.lib.syntax; -import com.google.common.hash.HashCode; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; import java.io.InputStream; @@ -37,7 +37,7 @@ public abstract class ParserInputSource { * Returns the path of the input source. Note: Once constructed, this object * will never re-read the content from path. */ - public abstract Path getPath(); + public abstract PathFragment getPath(); /** * Create an input source instance by (eagerly) reading from the file at @@ -53,7 +53,7 @@ public abstract class ParserInputSource { throw new IOException("Unexpected short read from file '" + path + "' (expected " + path.getFileSize() + ", got " + content.length + " bytes)"); } - return create(content, path); + return create(content, path.asFragment()); } /** @@ -61,7 +61,7 @@ public abstract class ParserInputSource { * this source. Path will be used in error messages etc. but we will *never* * attempt to read the content from path. */ - public static ParserInputSource create(String content, Path path) { + public static ParserInputSource create(String content, PathFragment path) { return create(content.toCharArray(), path); } @@ -70,7 +70,7 @@ public abstract class ParserInputSource { * this source. Path will be used in error messages etc. but we will *never* * attempt to read the content from path. */ - public static ParserInputSource create(final char[] content, final Path path) { + public static ParserInputSource create(final char[] content, final PathFragment path) { return new ParserInputSource() { @Override @@ -79,7 +79,7 @@ public abstract class ParserInputSource { } @Override - public Path getPath() { + public PathFragment getPath() { return path; } }; @@ -97,16 +97,9 @@ public abstract class ParserInputSource { */ public static ParserInputSource create(InputStream in, Path path) throws IOException { try { - return create(new String(FileSystemUtils.readContentAsLatin1(in)), path); + return create(new String(FileSystemUtils.readContentAsLatin1(in)), path.asFragment()); } finally { in.close(); } } - - /** - * Returns a hash code calculated from the string content of this file. - */ - public String contentHashCode() throws IOException { - return HashCode.fromBytes(getPath().getMD5Digest()).toString(); - } } -- cgit v1.2.3