Replace InputRule with Path. Summary: Updated `ConcurrentMapFileHashCache` to use a `LoadingCache` to maintain the mapping from `Path` to `HashCode`. Previously, `InputRule` contained four fields (`File`, `BuildTarget`, `ListenableFuture`, `RuleKey`). We had one of these for potentially every file in a project. That is a large potential for wasted memory. The existence of `InputRule` also required us to persist some kludginess in `BuildTarget`. This diff removes the hacks from `BuildTarget` and reclaims potentially a ton of memory. By using a `LoadingCache`, we do not have to worry about the same input file being sha1'd multiple times. We can also stop passing around references to a pathResolver in many places and rely on `Path` being a path relative to the project root. Test Plan: Sandcastle builds.
diff --git a/src/com/facebook/buck/cli/AuditInputCommand.java b/src/com/facebook/buck/cli/AuditInputCommand.java index 48298b8..95174b1 100644 --- a/src/com/facebook/buck/cli/AuditInputCommand.java +++ b/src/com/facebook/buck/cli/AuditInputCommand.java
@@ -24,11 +24,11 @@ import com.facebook.buck.parser.RawRulePredicate; import com.facebook.buck.rules.BuildRule; import com.facebook.buck.rules.BuildRuleType; -import com.facebook.buck.rules.InputRule; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import java.io.IOException; +import java.nio.file.Path; import java.util.Map; import java.util.Set; @@ -85,14 +85,14 @@ // than once. new AbstractBottomUpTraversal<BuildRule, Void>(partialGraph.getDependencyGraph()) { - final Set<InputRule> inputs = Sets.newHashSet(); + final Set<Path> inputs = Sets.newHashSet(); @Override public void visit(BuildRule rule) { - for (InputRule input : rule.getInputs()) { + for (Path input : rule.getInputs()) { boolean isNewInput = inputs.add(input); if (isNewInput) { - getStdOut().println(input.getFullyQualifiedName()); + getStdOut().println(input); } } }
diff --git a/src/com/facebook/buck/cli/AuditOwnerCommand.java b/src/com/facebook/buck/cli/AuditOwnerCommand.java index 0f44e1d..2fc63c5 100644 --- a/src/com/facebook/buck/cli/AuditOwnerCommand.java +++ b/src/com/facebook/buck/cli/AuditOwnerCommand.java
@@ -22,12 +22,10 @@ import com.facebook.buck.rules.BuildRule; import com.facebook.buck.rules.BuildRuleType; import com.facebook.buck.rules.DependencyGraph; -import com.facebook.buck.rules.InputRule; import com.facebook.buck.util.Ansi; import com.facebook.buck.util.BuckConstant; import com.facebook.buck.util.ProjectFilesystem; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Function; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSetMultimap; @@ -39,7 +37,7 @@ import java.io.IOException; import java.io.PrintStream; import java.nio.file.Path; -import java.util.Comparator; +import java.nio.file.Paths; import java.util.Set; /** @@ -55,13 +53,13 @@ @VisibleForTesting static final class OwnersReport { - final ImmutableSetMultimap<BuildRule, InputRule> owners; - final ImmutableSet<InputRule> inputsWithNoOwners; + final ImmutableSetMultimap<BuildRule, Path> owners; + final ImmutableSet<Path> inputsWithNoOwners; final ImmutableSet<String> nonExistentInputs; final ImmutableSet<String> nonFileInputs; - public OwnersReport(SetMultimap<BuildRule, InputRule> owners, - Set<InputRule> inputsWithNoOwners, + public OwnersReport(SetMultimap<BuildRule, Path> owners, + Set<Path> inputsWithNoOwners, Set<String> nonExistentInputs, Set<String> nonFileInputs) { this.owners = ImmutableSetMultimap.copyOf(owners); @@ -101,12 +99,11 @@ OwnersReport generateOwnersReport(DependencyGraph graph, AuditOwnerOptions options) { // Process arguments assuming they are all relative file paths. - Set<InputRule> inputs = Sets.newHashSet(); + Set<Path> inputs = Sets.newHashSet(); Set<String> nonExistentInputs = Sets.newHashSet(); Set<String> nonFileInputs = Sets.newHashSet(); ProjectFilesystem projectFilesystem = getProjectFilesystem(); - Function<String, Path> pathRelativizer = projectFilesystem.getPathRelativizer(); for (String filePath : options.getArguments()) { File file = projectFilesystem.getFileForRelativePath(filePath); if (!file.exists()) { @@ -114,15 +111,15 @@ } else if (!file.isFile()) { nonFileInputs.add(filePath); } else { - inputs.add(InputRule.inputPathAsInputRule(filePath, pathRelativizer)); + inputs.add(Paths.get(filePath)); } } // Try to find owners for each valid and existing file. - Set<InputRule> inputsWithNoOwners = Sets.newHashSet(inputs); - SetMultimap<BuildRule, InputRule> owners = createOwnersMap(); + Set<Path> inputsWithNoOwners = Sets.newHashSet(inputs); + SetMultimap<BuildRule, Path> owners = TreeMultimap.create(); for (BuildRule rule : graph.getNodes()) { - for (InputRule ruleInput : rule.getInputs()) { + for (Path ruleInput : rule.getInputs()) { if (inputs.contains(ruleInput)) { inputsWithNoOwners.remove(ruleInput); owners.put(rule, ruleInput); @@ -144,10 +141,9 @@ * missing file as input. */ private void guessOwnersForNonExistentFiles(DependencyGraph graph, - SetMultimap<BuildRule, InputRule> owners, Set<String> nonExistentFiles) { + SetMultimap<BuildRule, Path> owners, Set<String> nonExistentFiles) { ProjectFilesystem projectFilesystem = getProjectFilesystem(); - Function<String, Path> pathRelativizer = projectFilesystem.getPathRelativizer(); for (String nonExistentFile : nonExistentFiles) { File file = projectFilesystem.getFileForRelativePath(nonExistentFile); File buck = findBuckFileFor(file); @@ -158,7 +154,7 @@ try { File ruleBuck = rule.getBuildTarget().getBuildFile(projectFilesystem); if (buck.getCanonicalFile().equals(ruleBuck.getCanonicalFile())) { - owners.put(rule, InputRule.inputPathAsInputRule(nonExistentFile, pathRelativizer)); + owners.put(rule, Paths.get(nonExistentFile)); } } catch (IOException | BuildTargetException e) { throw Throwables.propagate(e); @@ -215,8 +211,8 @@ out.println(ansi.asSuccessText("Owners:")); for (BuildRule rule : report.owners.keySet()) { out.println(rule.getFullyQualifiedName()); - Set<InputRule> files = report.owners.get(rule); - for (InputRule input : files) { + Set<Path> files = report.owners.get(rule); + for (Path input : files) { out.println(FILE_INDENT + input); } } @@ -225,7 +221,7 @@ if (!report.inputsWithNoOwners.isEmpty()) { out.println(); out.println(ansi.asErrorText("Files without owners:")); - for (InputRule input : report.inputsWithNoOwners) { + for (Path input : report.inputsWithNoOwners) { out.println(FILE_INDENT + input); } } @@ -247,24 +243,6 @@ } } - private SetMultimap<BuildRule, InputRule> createOwnersMap() { - Comparator<BuildRule> keyComparator = new Comparator<BuildRule>() { - @Override - public int compare(BuildRule o1, BuildRule o2) { - return o1.getFullyQualifiedName().compareTo(o2.getFullyQualifiedName()); - } - }; - - Comparator<InputRule> valueComparator = new Comparator<InputRule>() { - @Override - public int compare(InputRule o1, InputRule o2) { - return o1.compareTo(o2); - } - }; - - return TreeMultimap.create(keyComparator, valueComparator); - } - @Override String getUsageIntro() { return "prints targets that own specified files";
diff --git a/src/com/facebook/buck/cli/Main.java b/src/com/facebook/buck/cli/Main.java index 28676a2..3744688 100644 --- a/src/com/facebook/buck/cli/Main.java +++ b/src/com/facebook/buck/cli/Main.java
@@ -37,7 +37,7 @@ import com.facebook.buck.timing.DefaultClock; import com.facebook.buck.util.Ansi; import com.facebook.buck.util.Console; -import com.facebook.buck.util.ConcurrentMapFileHashCache; +import com.facebook.buck.util.DefaultFileHashCache; import com.facebook.buck.util.FileHashCache; import com.facebook.buck.util.HumanReadableException; import com.facebook.buck.util.MoreStrings; @@ -121,7 +121,7 @@ Console console) throws IOException { this.config = Preconditions.checkNotNull(config); this.console = Preconditions.checkNotNull(console); - ConcurrentMapFileHashCache hashCache = new ConcurrentMapFileHashCache(projectFilesystem, console); + DefaultFileHashCache hashCache = new DefaultFileHashCache(projectFilesystem, console); this.parser = new Parser(projectFilesystem, new KnownBuildRuleTypes(), console, @@ -346,7 +346,7 @@ config.getPythonInterpreter(), config.getTempFilePatterns(), createRuleKeyBuilderFactory(config, - new ConcurrentMapFileHashCache(projectFilesystem, console))); + new DefaultFileHashCache(projectFilesystem, console))); } Clock clock = new DefaultClock();
diff --git a/src/com/facebook/buck/cli/TargetsCommand.java b/src/com/facebook/buck/cli/TargetsCommand.java index 3e69339..5f95da2 100644 --- a/src/com/facebook/buck/cli/TargetsCommand.java +++ b/src/com/facebook/buck/cli/TargetsCommand.java
@@ -28,11 +28,10 @@ import com.facebook.buck.rules.BuildRuleType; import com.facebook.buck.rules.Buildable; import com.facebook.buck.rules.DependencyGraph; -import com.facebook.buck.rules.InputRule; import com.facebook.buck.util.HumanReadableException; +import com.facebook.buck.util.MorePaths; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Function; import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.google.common.base.Throwables; @@ -111,7 +110,6 @@ graph.getDependencyGraph(), new TargetsCommandPredicate( graph, - getProjectFilesystem().getPathRelativizer(), buildRuleTypesBuilder.build(), options.getReferencedFiles(getProjectFilesystem().getProjectRoot()), matchingBuildTargets)); @@ -335,30 +333,31 @@ private DependencyGraph graph; private ImmutableSet<BuildRuleType> buildRuleTypes; - private ImmutableSet<InputRule> referencedInputs; + private ImmutableSet<Path> referencedInputs; private Set<String> basePathOfTargets; private Set<BuildRule> dependentTargets; private Set<BuildTarget> matchingBuildRules; + /** + * @param referencedPaths All of these paths must be relative to the project root. + */ public TargetsCommandPredicate( PartialGraph partialGraph, - Function<String, Path> pathRelativizer, ImmutableSet<BuildRuleType> buildRuleTypes, - ImmutableSet<String> referencedFiles, + ImmutableSet<String> referencedPaths, ImmutableSet<BuildTarget> matchingBuildRules) { this.graph = partialGraph.getDependencyGraph(); this.buildRuleTypes = Preconditions.checkNotNull(buildRuleTypes); this.matchingBuildRules = Preconditions.checkNotNull(matchingBuildRules); - Preconditions.checkNotNull(referencedFiles); - if (!referencedFiles.isEmpty()) { - this.referencedInputs = InputRule.inputPathsAsInputRules(referencedFiles, pathRelativizer); + Preconditions.checkNotNull(referencedPaths); + if (!referencedPaths.isEmpty()) { + this.referencedInputs = MorePaths.asPaths(referencedPaths); BuildFileTree tree = new BuildFileTree(partialGraph.getTargets()); basePathOfTargets = Sets.newHashSet(); dependentTargets = Sets.newHashSet(); - for (InputRule input : referencedInputs) { - basePathOfTargets.add(tree.getBasePathOfAncestorTarget( - input.getBuildTarget().getBasePath())); + for (Path input : referencedInputs) { + basePathOfTargets.add(tree.getBasePathOfAncestorTarget(input.toString())); } } else { basePathOfTargets = ImmutableSet.of(); @@ -376,7 +375,7 @@ // Any referenced file, only those with the nearest BuildTarget can // directly depend on that file. if (!isDependent && basePathOfTargets.contains(rule.getBuildTarget().getBasePath())) { - for (InputRule input : rule.getInputs()) { + for (Path input : rule.getInputs()) { if (referencedInputs.contains(input)) { isDependent = true; break;
diff --git a/src/com/facebook/buck/model/BuildTarget.java b/src/com/facebook/buck/model/BuildTarget.java index 8a4632c..d7d4b58 100644 --- a/src/com/facebook/buck/model/BuildTarget.java +++ b/src/com/facebook/buck/model/BuildTarget.java
@@ -53,26 +53,6 @@ } /** - * For use by InputRule, which is synthetic, and therefore has no buildFile. - * @param inputFile Input file. - * @param relativePath - */ - private BuildTarget(File inputFile, String relativePath) { - Preconditions.checkNotNull(inputFile); - Preconditions.checkNotNull(relativePath); - this.baseName = String.format("//%s", relativePath); - this.shortName = inputFile.getName(); - this.fullyQualifiedName = String.format("%s:%s", baseName, shortName); - } - - /** - * For exclusive use by {@link com.facebook.buck.rules.InputRule#InputRule(File, String)}. - */ - public static BuildTarget createBuildTargetForInputFile(File inputFile, String relativePath) { - return new BuildTarget(inputFile, relativePath); - } - - /** * The build file in which this rule was defined. * @throws MissingBuildFileException if the build file for the target does not exist. */
diff --git a/src/com/facebook/buck/rules/AbstractCachingBuildRule.java b/src/com/facebook/buck/rules/AbstractCachingBuildRule.java index ee147f5..774dfec 100644 --- a/src/com/facebook/buck/rules/AbstractCachingBuildRule.java +++ b/src/com/facebook/buck/rules/AbstractCachingBuildRule.java
@@ -22,10 +22,10 @@ import com.facebook.buck.step.StepFailedException; import com.facebook.buck.step.StepRunner; import com.facebook.buck.util.BuckConstant; +import com.facebook.buck.util.MorePaths; import com.facebook.buck.util.ProcessExecutor; import com.facebook.buck.util.concurrent.MoreFutures; import com.google.common.annotations.Beta; -import com.google.common.base.Function; import com.google.common.base.Optional; import com.google.common.base.Preconditions; import com.google.common.base.Supplier; @@ -69,24 +69,20 @@ */ private final SettableFuture<BuildRuleSuccess> buildRuleResult; - private final Function<String, Path> pathRelativizer; - /** @see Buildable#getInputsToCompareToOutput() */ - private Iterable<InputRule> inputsToCompareToOutputs; + private Iterable<Path> inputsToCompareToOutputs; protected AbstractCachingBuildRule(Buildable buildable, BuildRuleParams params) { super(params); this.buildable = Preconditions.checkNotNull(buildable); this.hasBuildStarted = new AtomicBoolean(false); this.buildRuleResult = SettableFuture.create(); - this.pathRelativizer = params.getPathRelativizer(); } protected AbstractCachingBuildRule(BuildRuleParams buildRuleParams) { super(buildRuleParams); this.hasBuildStarted = new AtomicBoolean(false); this.buildRuleResult = SettableFuture.create(); - this.pathRelativizer = buildRuleParams.getPathRelativizer(); this.buildable = Preconditions.checkNotNull(getBuildable()); } @@ -110,10 +106,10 @@ } @Override - public Iterable<InputRule> getInputs() { + public Iterable<Path> getInputs() { if (inputsToCompareToOutputs == null) { - inputsToCompareToOutputs = InputRule.inputPathsAsInputRules( - buildable.getInputsToCompareToOutput(), pathRelativizer); + inputsToCompareToOutputs = MorePaths.asPaths( + buildable.getInputsToCompareToOutput()); } return inputsToCompareToOutputs; } @@ -125,7 +121,7 @@ // files will be hashed. In the case of .set("srcs", srcs), the list of strings itself will be // hashed. It turns out that we need both of these in order to construct a RuleKey correctly. builder = super.appendToRuleKey(builder) - .setInputs("buck.inputs", getInputs()); + .setInputs("buck.inputs", getInputs().iterator()); // TODO(simons): Rename this when no Buildables extend this class. return buildable.appendDetailsToRuleKey(builder); }
diff --git a/src/com/facebook/buck/rules/BUCK b/src/com/facebook/buck/rules/BUCK index a68b728..a92ba22 100644 --- a/src/com/facebook/buck/rules/BUCK +++ b/src/com/facebook/buck/rules/BUCK
@@ -55,7 +55,6 @@ 'DefaultBuildRuleBuilderParams.java', 'DependencyGraph.java', 'FileSourcePath.java', - 'InputRule.java', 'JavaPackageFinder.java', 'OnDiskBuildInfo.java', 'RuleKey.java',
diff --git a/src/com/facebook/buck/rules/BuildRule.java b/src/com/facebook/buck/rules/BuildRule.java index 57e0c69..ab745ff 100644 --- a/src/com/facebook/buck/rules/BuildRule.java +++ b/src/com/facebook/buck/rules/BuildRule.java
@@ -25,6 +25,7 @@ import com.google.common.util.concurrent.ListenableFuture; import java.io.IOException; +import java.nio.file.Path; import javax.annotation.Nullable; @@ -71,7 +72,7 @@ /** * @return the inputs needed to build this build rule */ - public Iterable<InputRule> getInputs(); + public Iterable<Path> getInputs(); /** * This method must be idempotent.
diff --git a/src/com/facebook/buck/rules/BuildRuleSuccess.java b/src/com/facebook/buck/rules/BuildRuleSuccess.java index 385ada0..7035888 100644 --- a/src/com/facebook/buck/rules/BuildRuleSuccess.java +++ b/src/com/facebook/buck/rules/BuildRuleSuccess.java
@@ -63,7 +63,7 @@ Property.SHOULD_WRITE_RECORDED_METADATA_TO_DISK ), - /** Created trivially, such as an {@link InputRule} or {@link ProjectConfigRule}. */ + /** Created trivially, such as a {@link ProjectConfigRule}. */ BY_DEFINITION, ;
diff --git a/src/com/facebook/buck/rules/BuildRuleType.java b/src/com/facebook/buck/rules/BuildRuleType.java index 2d6a4bf..e85c016 100644 --- a/src/com/facebook/buck/rules/BuildRuleType.java +++ b/src/com/facebook/buck/rules/BuildRuleType.java
@@ -30,7 +30,6 @@ public static BuildRuleType GEN_AIDL = new BuildRuleType("gen_aidl"); public static BuildRuleType GEN_PARCELABLE = new BuildRuleType("gen_parcelable"); public static BuildRuleType GENRULE = new BuildRuleType("genrule"); - public static BuildRuleType INPUT = new BuildRuleType("input"); public static BuildRuleType JAVA_BINARY = new BuildRuleType("java_binary"); public static BuildRuleType JAVA_LIBRARY = new BuildRuleType("java_library"); public static BuildRuleType JAVA_TEST = new BuildRuleType("java_test");
diff --git a/src/com/facebook/buck/rules/Buildable.java b/src/com/facebook/buck/rules/Buildable.java index 0ef11cf..49b8042 100644 --- a/src/com/facebook/buck/rules/Buildable.java +++ b/src/com/facebook/buck/rules/Buildable.java
@@ -35,7 +35,7 @@ * Get the set of input files whose contents should be hashed for the purpose of determining * whether this rule is cached. * <p> - * Note that inputs are iterable and should generally be alphatized so that lists with the same + * Note that inputs are iterable and should generally be alphabetized so that lists with the same * elements will be {@code .equals()} to one another. However, for some build rules (such as * {@link com.facebook.buck.shell.Genrule}), the order of the inputs is significant and in these * cases the inputs may be ordered in any way the rule feels most appropriate.
diff --git a/src/com/facebook/buck/rules/InputRule.java b/src/com/facebook/buck/rules/InputRule.java deleted file mode 100644 index 510522a..0000000 --- a/src/com/facebook/buck/rules/InputRule.java +++ /dev/null
@@ -1,190 +0,0 @@ -/* - * Copyright 2012-present Facebook, Inc. - * - * 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.facebook.buck.rules; - -import com.facebook.buck.model.BuildTarget; -import com.facebook.buck.model.BuildTargetPattern; -import com.facebook.buck.util.FileHashCache; -import com.facebook.buck.util.NullFileHashCache; -import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Function; -import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ImmutableSortedSet; -import com.google.common.collect.Iterables; -import com.google.common.util.concurrent.Futures; -import com.google.common.util.concurrent.ListenableFuture; - -import java.io.File; -import java.io.IOException; -import java.nio.file.Path; - -import javax.annotation.Nullable; - -/** - * InputRule is a no-op BuildRule that encapsulates an input file. This encapsulation allows the - * entire dependency graph to deal with BuildRules, rather than having leaves of a different type. - */ -public class InputRule implements BuildRule { - - private static final BuildRuleSuccess.Type SUCCESS_TYPE = BuildRuleSuccess.Type.BY_DEFINITION; - private static final FileHashCache nullFileHashCache = new NullFileHashCache(); - - private final File inputFile; - private final BuildTarget buildTarget; - private final ListenableFuture<BuildRuleSuccess> buildOutput; - @Nullable private RuleKey ruleKey; - - /** - * It is imperative that {@code inputFile} be properly relativized, so use - * {@link #inputPathAsInputRule(String, Function)} to create an {@link InputRule}. The only reason - * this method is {@code protected} rather than {@code private} is so that fakes can be created - * for testing. - */ - @VisibleForTesting - protected InputRule(File inputFile, String relativePath) { - this.inputFile = Preconditions.checkNotNull(inputFile); - this.buildTarget = BuildTarget.createBuildTargetForInputFile(inputFile, relativePath); - this.buildOutput = Futures.immediateFuture(new BuildRuleSuccess(this, SUCCESS_TYPE)); - } - - public static InputRule inputPathAsInputRule(String relativePath, - Function<String, Path> pathRelativizer) { - return Iterables.getOnlyElement( - inputPathsAsInputRules(ImmutableList.of(relativePath), pathRelativizer)); - } - - /** - * Convert a set of input file paths to InputRules. - */ - public static ImmutableSortedSet<InputRule> inputPathsAsInputRules(Iterable<String> paths, - Function<String, Path> pathRelativizer) { - ImmutableSortedSet.Builder<InputRule> builder = ImmutableSortedSet.naturalOrder(); - for (String path : paths) { - builder.add(new InputRule(pathRelativizer.apply(path).toFile(), path)); - } - return builder.build(); - } - - @Override - public BuildRuleSuccess.Type getBuildResultType() { - return SUCCESS_TYPE; - } - - @Nullable - @Override - public Buildable getBuildable() { - return null; - } - - @Override - public BuildTarget getBuildTarget() { - return buildTarget; - } - - @Override - public String getFullyQualifiedName() { - return buildTarget.getFullyQualifiedName(); - } - - @Override - public BuildRuleType getType() { - return BuildRuleType.INPUT; - } - - @Override - public BuildableProperties getProperties() { - return BuildableProperties.NONE; - } - - @Override - public ImmutableSortedSet<BuildRule> getDeps() { - return ImmutableSortedSet.of(); - } - - @Override - public ImmutableSet<BuildTargetPattern> getVisibilityPatterns() { - return ImmutableSet.of(BuildTargetPattern.MATCH_ALL); - } - - @Override - public boolean isVisibleTo(BuildTarget target) { - return true; - } - - @Override - public Iterable<InputRule> getInputs() { - return ImmutableList.of(); - } - - @Override - public ListenableFuture<BuildRuleSuccess> build(BuildContext context) { - return buildOutput; - } - - @Override - public boolean getExportDeps() { - return false; - } - - @Override - public RuleKey getRuleKey() throws IOException { - return getRuleKey(nullFileHashCache); - } - - public RuleKey getRuleKey(FileHashCache hashCache) throws IOException { - if (this.ruleKey == null) { - - // TODO(user, mbolin): avoid RuleKey.Builder/InputRule mutual dependence. - ruleKey = RuleKey.builder(this, hashCache) - .set("inputFile", inputFile).build().getTotalRuleKey(); - } - return ruleKey; - } - - @Override - public int hashCode() { - return getFullyQualifiedName().hashCode(); - } - - @Override - public boolean equals(Object other) { - if (this == other) { - return true; - } - if (other == null || getClass() != other.getClass()) { - return false; - } - return (compareTo((InputRule) other) == 0); - } - - @Override - public int compareTo(BuildRule buildRule) { - return getFullyQualifiedName().compareTo(buildRule.getFullyQualifiedName()); - } - - @Override - public String toString() { - return getFullyQualifiedName(); - } - - @Override - public RuleKey getRuleKeyWithoutDeps() throws IOException { - throw new UnsupportedOperationException(); - } -}
diff --git a/src/com/facebook/buck/rules/ProjectConfigRule.java b/src/com/facebook/buck/rules/ProjectConfigRule.java index 5973a1f..bc4cf11 100644 --- a/src/com/facebook/buck/rules/ProjectConfigRule.java +++ b/src/com/facebook/buck/rules/ProjectConfigRule.java
@@ -28,6 +28,7 @@ import com.google.common.util.concurrent.ListenableFuture; import java.io.IOException; +import java.nio.file.Path; import java.util.List; import java.util.Set; @@ -161,7 +162,7 @@ } @Override - public final Iterable<InputRule> getInputs() { + public final Iterable<Path> getInputs() { return ImmutableList.of(); }
diff --git a/src/com/facebook/buck/rules/RuleKey.java b/src/com/facebook/buck/rules/RuleKey.java index 71eb0d4..b14012d 100644 --- a/src/com/facebook/buck/rules/RuleKey.java +++ b/src/com/facebook/buck/rules/RuleKey.java
@@ -32,14 +32,10 @@ import com.google.common.hash.HashCode; import com.google.common.hash.Hasher; import com.google.common.hash.Hashing; -import com.google.common.io.ByteStreams; -import com.google.common.io.Files; -import com.google.common.io.InputSupplier; -import java.io.File; import java.io.IOException; -import java.io.InputStream; import java.nio.file.Path; +import java.util.Iterator; import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; @@ -166,36 +162,6 @@ return separate().feed(sectionLabel.getBytes()).separate(); } - private Builder setVal(@Nullable File file) throws IOException { - - if (file != null) { - - Path path = file.toPath(); - HashCode fileSha1 = null; - - if (hashCache.contains(path)) { - - // Use cached hash if possible, to avoid reading all input files on each build. - fileSha1 = hashCache.get(path); - } else { - - // Compute a separate SHA-1 for the file contents and feed that into messageDigest rather - // than the file contents, in order to avoid the overhead of escaping SEPARATOR in the file - // content. - InputSupplier<? extends InputStream> inputSupplier = Files.newInputStreamSupplier(file); - fileSha1 = ByteStreams.hash(inputSupplier, Hashing.sha1()); - hashCache.put(path, fileSha1); - } - - if (logElms != null) { - logElms.add(String.format("file(path=\"%s\", sha1=%s):", file.getPath(), - fileSha1.toString())); - } - feed(fileSha1.asBytes()); - } - return separate(); - } - private Builder setVal(@Nullable String s) { if (s != null) { if (logElms != null) { @@ -232,14 +198,6 @@ return separate(); } - public Builder set(String key, @Nullable File val) throws IOException { - return setKey(key).setVal(val); - } - - public Builder set(String key, @Nullable Path path) throws IOException { - return set(key, path == null ? null : path.toFile()); - } - public Builder set(String key, @Nullable String val) { return setKey(key).setVal(val); } @@ -284,12 +242,22 @@ return separate(); } - public Builder setInputs(String key, @Nullable Iterable<InputRule> val) throws IOException { + /** + * @param inputs is an {@link Iterator} rather than an {@link Iterable} because {@link Path} + * implements {@link Iterable} and we want to protect against passing a single {@link Path} + * instead of multiple {@link Path}s. + */ + public Builder setInputs(String key, Iterator<Path> inputs) throws IOException { + Preconditions.checkNotNull(key); + Preconditions.checkNotNull(inputs); setKey(key); - if (val != null) { - for (InputRule inputRule : val) { - setVal(inputRule.getRuleKey(hashCache)); + while (inputs.hasNext()) { + Path input = inputs.next(); + HashCode sha1 = hashCache.get(input); + if (sha1 == null) { + throw new RuntimeException("No SHA for " + input); } + setVal(sha1.toString()); } return separate(); }
diff --git a/src/com/facebook/buck/util/ConcurrentMapFileHashCache.java b/src/com/facebook/buck/util/ConcurrentMapFileHashCache.java deleted file mode 100644 index e9ff13d..0000000 --- a/src/com/facebook/buck/util/ConcurrentMapFileHashCache.java +++ /dev/null
@@ -1,86 +0,0 @@ -/* - * Copyright 2012-present Facebook, Inc. - * - * 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.facebook.buck.util; - -import com.google.common.base.Preconditions; -import com.google.common.collect.Maps; -import com.google.common.eventbus.Subscribe; -import com.google.common.hash.HashCode; - -import java.io.IOException; -import java.nio.file.WatchEvent; -import java.nio.file.Path; -import java.util.concurrent.ConcurrentMap; - -public class ConcurrentMapFileHashCache implements FileHashCache { - - private final ConcurrentMap<Path, HashCode> hashCache; - private final ProjectFilesystem projectFilesystem; - private final Console console; - - public ConcurrentMapFileHashCache(ProjectFilesystem projectFilesystem, Console console) { - this.projectFilesystem = projectFilesystem; - this.console = console; - hashCache = Maps.newConcurrentMap(); - } - - @Override - public boolean contains(Path path) { - return hashCache.containsKey(path); - } - - /** - * @param path {@link #contains(java.nio.file.Path)} must be true for path. - * @return The {@link com.google.common.hash.HashCode} of the contents of path. - */ - @Override - public HashCode get(Path path) { - - // checkNotNull on result rather than checkState(this.contains(Path)) to avoid 2 lookups. - return Preconditions.checkNotNull(hashCache.get(path)); - } - - @Override - public void put(Path path, HashCode fileSha1) { - hashCache.put(path, fileSha1); - } - - /** - * Called when file change events are posted to the file change EventBus to invalidate cached - * build rules if required. - */ - @Subscribe - public void onFileSystemChange(WatchEvent<?> event) throws IOException { - - if (console.getVerbosity() == Verbosity.ALL) { - console.getStdErr().printf("ConcurrentMapFileHashCache watched event %s %s\n", event.kind(), - projectFilesystem.createContextString(event)); - } - - if (projectFilesystem.isPathChangeEvent(event)) { - - // Path event, remove the path from the cache as it has been changed, added or deleted. - Path path = (Path) event.context(); - hashCache.remove(path); - - } else { - - // Non-path change event, likely an overflow due to many change events: invalidate everything. - hashCache.clear(); - } - } -}
diff --git a/src/com/facebook/buck/util/DefaultFileHashCache.java b/src/com/facebook/buck/util/DefaultFileHashCache.java new file mode 100644 index 0000000..9032b84 --- /dev/null +++ b/src/com/facebook/buck/util/DefaultFileHashCache.java
@@ -0,0 +1,101 @@ +/* + * Copyright 2012-present Facebook, Inc. + * + * 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.facebook.buck.util; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; +import com.google.common.eventbus.Subscribe; +import com.google.common.hash.HashCode; +import com.google.common.hash.Hashing; +import com.google.common.io.ByteStreams; +import com.google.common.io.Files; +import com.google.common.io.InputSupplier; + +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Path; +import java.nio.file.WatchEvent; +import java.util.concurrent.ExecutionException; + +public class DefaultFileHashCache implements FileHashCache { + + private final ProjectFilesystem projectFilesystem; + private final Console console; + + @VisibleForTesting + final LoadingCache<Path, HashCode> loadingCache; + + public DefaultFileHashCache(ProjectFilesystem projectFilesystem, Console console) { + this.projectFilesystem = Preconditions.checkNotNull(projectFilesystem); + this.console = Preconditions.checkNotNull(console); + + this.loadingCache = CacheBuilder.newBuilder() + .build(new CacheLoader<Path, HashCode>() { + @Override + public HashCode load(Path path) throws Exception { + File file = DefaultFileHashCache.this.projectFilesystem.resolve(path).toFile(); + InputSupplier<? extends InputStream> inputSupplier = Files.newInputStreamSupplier(file); + return ByteStreams.hash(inputSupplier, Hashing.sha1()); + } + }); + } + + @Override + public boolean contains(Path path) { + return loadingCache.getIfPresent(path) != null; + } + + /** + * @param path {@link #contains(java.nio.file.Path)} must be true for path. + * @return The {@link com.google.common.hash.HashCode} of the contents of path. + */ + @Override + public HashCode get(Path path) { + HashCode sha1; + try { + sha1 = loadingCache.get(path); + } catch (ExecutionException e) { + throw new RuntimeException(e); + } + return Preconditions.checkNotNull(sha1, "Failed to find a HashCode for %s.", path); + } + + /** + * Called when file change events are posted to the file change EventBus to invalidate cached + * build rules if required. + */ + @Subscribe + public void onFileSystemChange(WatchEvent<?> event) throws IOException { + if (console.getVerbosity() == Verbosity.ALL) { + console.getStdErr().printf("ConcurrentMapFileHashCache watched event %s %s\n", event.kind(), + projectFilesystem.createContextString(event)); + } + + if (projectFilesystem.isPathChangeEvent(event)) { + // Path event, remove the path from the cache as it has been changed, added or deleted. + Path path = (Path) event.context(); + loadingCache.invalidate(path); + } else { + // Non-path change event, likely an overflow due to many change events: invalidate everything. + loadingCache.invalidateAll(); + } + } +}
diff --git a/src/com/facebook/buck/util/FileHashCache.java b/src/com/facebook/buck/util/FileHashCache.java index 51a5dc5..3d48a32 100644 --- a/src/com/facebook/buck/util/FileHashCache.java +++ b/src/com/facebook/buck/util/FileHashCache.java
@@ -17,9 +17,10 @@ package com.facebook.buck.util; import com.google.common.hash.HashCode; + import java.nio.file.Path; -/* +/** * A cache which maps Paths to cached hashes of their contents, * based on a simplified subset of the java.util.Map<Path, HashCode> interface. */ @@ -28,6 +29,4 @@ HashCode get(Path path); - void put(Path path, HashCode fileSha1); - }
diff --git a/src/com/facebook/buck/util/MorePaths.java b/src/com/facebook/buck/util/MorePaths.java index dff2c96..fcaca0a 100644 --- a/src/com/facebook/buck/util/MorePaths.java +++ b/src/com/facebook/buck/util/MorePaths.java
@@ -18,6 +18,7 @@ import com.google.common.base.Preconditions; import com.google.common.base.Strings; +import com.google.common.collect.ImmutableSortedSet; import java.io.File; import java.io.IOException; @@ -108,4 +109,15 @@ Files.createSymbolicLink(pathToProjectRoot.resolve(pathToDesiredLinkUnderProjectRoot), target); return target; } + + /** + * Convert a set of input file paths as strings to {@link Path} objects. + */ + public static ImmutableSortedSet<Path> asPaths(Iterable<String> paths) { + ImmutableSortedSet.Builder<Path> builder = ImmutableSortedSet.naturalOrder(); + for (String path : paths) { + builder.add(Paths.get(path)); + } + return builder.build(); + } }
diff --git a/src/com/facebook/buck/util/NullFileHashCache.java b/src/com/facebook/buck/util/NullFileHashCache.java index 50b5ccd..de78b52 100644 --- a/src/com/facebook/buck/util/NullFileHashCache.java +++ b/src/com/facebook/buck/util/NullFileHashCache.java
@@ -20,6 +20,7 @@ import java.nio.file.Path; +// TODO: This should be renamed to DummyFileHashCache and moved under the test/ directory. public class NullFileHashCache implements FileHashCache { @Override @@ -31,8 +32,4 @@ public HashCode get(Path path) { return null; } - - @Override - public void put(Path path, HashCode fileSha1) { - } }
diff --git a/test/com/facebook/buck/cli/AuditOwnerCommandTest.java b/test/com/facebook/buck/cli/AuditOwnerCommandTest.java index 4ed4b6f..044e41e 100644 --- a/test/com/facebook/buck/cli/AuditOwnerCommandTest.java +++ b/test/com/facebook/buck/cli/AuditOwnerCommandTest.java
@@ -33,12 +33,12 @@ import com.facebook.buck.rules.Buildable; import com.facebook.buck.rules.BuildableProperties; import com.facebook.buck.rules.DependencyGraph; -import com.facebook.buck.rules.InputRule; import com.facebook.buck.rules.KnownBuildRuleTypes; import com.facebook.buck.rules.NoopArtifactCache; import com.facebook.buck.rules.RuleKey; import com.facebook.buck.testutil.TestConsole; import com.facebook.buck.util.Console; +import com.facebook.buck.util.MorePaths; import com.facebook.buck.util.ProjectFilesystem; import com.facebook.buck.util.environment.Platform; import com.google.common.collect.ImmutableSet; @@ -51,6 +51,8 @@ import org.kohsuke.args4j.CmdLineParser; import java.io.File; +import java.nio.file.Path; +import java.util.Arrays; import javax.annotation.Nullable; @@ -61,16 +63,16 @@ private static class StubBuildRule implements BuildRule { - private final ImmutableSet<InputRule> inputs; + private final ImmutableSet<Path> inputs; private final BuildTarget target; - public StubBuildRule(BuildTarget target, Iterable<InputRule> inputs) { + public StubBuildRule(BuildTarget target, Iterable<Path> inputs) { this.target = target; this.inputs = ImmutableSet.copyOf(inputs); } @Override - public Iterable<InputRule> getInputs() { + public Iterable<Path> getInputs() { return inputs; } @@ -142,7 +144,7 @@ @Override public int compareTo(BuildRule buildRule) { - throw new UnsupportedOperationException(); + return this.getFullyQualifiedName().compareTo(buildRule.getFullyQualifiedName()); } } @@ -338,9 +340,7 @@ "java/somefolder/perfect.java", "com/test/subtest/random.java" }; - ImmutableSortedSet<InputRule> inputs - = InputRule.inputPathsAsInputRules(ImmutableSortedSet.copyOf(args), - filesystem.getPathRelativizer()); + ImmutableSortedSet<Path> inputs = MorePaths.asPaths(Arrays.asList(args)); // Create options AuditOwnerOptions options = getOptions(args); @@ -376,9 +376,7 @@ "java/somefolder/perfect.java", "com/test/subtest/random.java" }; - ImmutableSortedSet<InputRule> inputs - = InputRule.inputPathsAsInputRules(ImmutableSortedSet.copyOf(args), - filesystem.getPathRelativizer()); + ImmutableSortedSet<Path> inputs = MorePaths.asPaths(ImmutableSortedSet.copyOf(args)); // Build rule that owns all inputs BuildTarget target = new BuildTarget("//base/name", "name"); @@ -427,9 +425,7 @@ "java/somefolder/perfect.java", "com/test/subtest/random.java" }; - ImmutableSortedSet<InputRule> inputs - = InputRule.inputPathsAsInputRules(ImmutableSortedSet.copyOf(args), - filesystem.getPathRelativizer()); + ImmutableSortedSet<Path> inputs = MorePaths.asPaths(ImmutableSortedSet.copyOf(args)); // Build rule that owns all inputs BuildTarget target1 = BuildTargetFactory.newInstance("//base/name1:name1");
diff --git a/test/com/facebook/buck/cli/TargetsCommandTest.java b/test/com/facebook/buck/cli/TargetsCommandTest.java index 0c6b38e..baaa93c 100644 --- a/test/com/facebook/buck/cli/TargetsCommandTest.java +++ b/test/com/facebook/buck/cli/TargetsCommandTest.java
@@ -47,7 +47,6 @@ import com.facebook.buck.rules.KnownBuildRuleTypes; import com.facebook.buck.rules.NoopArtifactCache; import com.facebook.buck.testutil.BuckTestConstant; -import com.facebook.buck.testutil.IdentityPathRelativizer; import com.facebook.buck.testutil.RuleMap; import com.facebook.buck.testutil.TestConsole; import com.facebook.buck.util.ProjectFilesystem; @@ -74,14 +73,11 @@ import java.io.IOException; import java.io.Reader; import java.io.StringReader; -import java.nio.file.Path; import java.util.List; import java.util.SortedMap; public class TargetsCommandTest { - private static final Function<String, Path> PATH_RELATIVIZER = - IdentityPathRelativizer.getIdentityRelativizer(); private final String projectRootPath = "."; private final File projectRoot = new File(projectRootPath); private TestConsole console; @@ -301,7 +297,7 @@ SortedMap<String, BuildRule> matchingBuildRules = targetsCommand.getMatchingBuildRules( graph.getDependencyGraph(), - new TargetsCommandPredicate(graph, PATH_RELATIVIZER, buildRuleTypes, referencedFiles, targetBuildRules)); + new TargetsCommandPredicate(graph, buildRuleTypes, referencedFiles, targetBuildRules)); assertTrue(matchingBuildRules.isEmpty()); // Only test-android-library target depends on the referenced file. @@ -309,18 +305,18 @@ matchingBuildRules = targetsCommand.getMatchingBuildRules( graph.getDependencyGraph(), - new TargetsCommandPredicate(graph, PATH_RELATIVIZER, buildRuleTypes, referencedFiles, targetBuildRules)); + new TargetsCommandPredicate(graph, buildRuleTypes, referencedFiles, targetBuildRules)); assertEquals( ImmutableSet.of("//javatest:test-java-library"), matchingBuildRules.keySet()); - // The test-android-library target indirectly depend on the referenced file, - // while test-java-library target directly depend on the referenced file. + // The test-android-library target indirectly depends on the referenced file, + // while test-java-library target directly depends on the referenced file. referencedFiles = ImmutableSet.of("javasrc/JavaLibrary.java"); matchingBuildRules = targetsCommand.getMatchingBuildRules( graph.getDependencyGraph(), - new TargetsCommandPredicate(graph, PATH_RELATIVIZER, buildRuleTypes, referencedFiles, targetBuildRules)); + new TargetsCommandPredicate(graph, buildRuleTypes, referencedFiles, targetBuildRules)); assertEquals( ImmutableSet.of("//javatest:test-java-library", "//javasrc:java-library"), matchingBuildRules.keySet()); @@ -331,7 +327,7 @@ matchingBuildRules = targetsCommand.getMatchingBuildRules( graph.getDependencyGraph(), - new TargetsCommandPredicate(graph, PATH_RELATIVIZER, buildRuleTypes, referencedFiles, targetBuildRules)); + new TargetsCommandPredicate(graph, buildRuleTypes, referencedFiles, targetBuildRules)); assertEquals( ImmutableSet.of("//javatest:test-java-library"), matchingBuildRules.keySet()); @@ -342,7 +338,6 @@ targetsCommand.getMatchingBuildRules( graph.getDependencyGraph(), new TargetsCommandPredicate(graph, - PATH_RELATIVIZER, buildRuleTypes, ImmutableSet.<String>of(), targetBuildRules)); @@ -359,7 +354,6 @@ graph.getDependencyGraph(), new TargetsCommandPredicate( graph, - PATH_RELATIVIZER, ImmutableSet.of(BuildRuleType.JAVA_TEST, BuildRuleType.JAVA_LIBRARY), ImmutableSet.<String>of(), targetBuildRules)); @@ -376,7 +370,6 @@ graph.getDependencyGraph(), new TargetsCommandPredicate( graph, - PATH_RELATIVIZER, ImmutableSet.of(BuildRuleType.JAVA_TEST, BuildRuleType.JAVA_LIBRARY), ImmutableSet.<String>of(), ImmutableSet.of(BuildTargetFactory.newInstance("//javasrc:java-library")))); @@ -389,7 +382,6 @@ graph.getDependencyGraph(), new TargetsCommandPredicate( graph, - PATH_RELATIVIZER, ImmutableSet.<BuildRuleType>of(), ImmutableSet.<String>of(), ImmutableSet.of(BuildTargetFactory.newInstance("//javasrc:java-library")))); @@ -403,7 +395,6 @@ graph.getDependencyGraph(), new TargetsCommandPredicate( graph, - PATH_RELATIVIZER, ImmutableSet.<BuildRuleType>of(), ImmutableSet.of("javatest/TestJavaLibrary.java"), ImmutableSet.of(BuildTargetFactory.newInstance("//javasrc:java-library"))));
diff --git a/test/com/facebook/buck/rules/AbstractBuildRuleTest.java b/test/com/facebook/buck/rules/AbstractBuildRuleTest.java index a27a3f0..361a929 100644 --- a/test/com/facebook/buck/rules/AbstractBuildRuleTest.java +++ b/test/com/facebook/buck/rules/AbstractBuildRuleTest.java
@@ -33,6 +33,7 @@ import org.junit.Test; +import java.nio.file.Path; import java.util.Comparator; import javax.annotation.Nullable; @@ -218,7 +219,7 @@ } @Override - public final Iterable<InputRule> getInputs() { + public final Iterable<Path> getInputs() { return ImmutableList.of(); }
diff --git a/test/com/facebook/buck/rules/AbstractCachingBuildRuleTest.java b/test/com/facebook/buck/rules/AbstractCachingBuildRuleTest.java index 8617c7b..57f7ce4 100644 --- a/test/com/facebook/buck/rules/AbstractCachingBuildRuleTest.java +++ b/test/com/facebook/buck/rules/AbstractCachingBuildRuleTest.java
@@ -33,9 +33,11 @@ import com.facebook.buck.step.Step; import com.facebook.buck.step.StepFailedException; import com.facebook.buck.step.StepRunner; +import com.facebook.buck.testutil.FakeFileHashCache; import com.facebook.buck.testutil.RuleMap; import com.facebook.buck.testutil.TestConsole; import com.facebook.buck.testutil.integration.DebuggableTemporaryFolder; +import com.facebook.buck.util.FileHashCache; import com.facebook.buck.util.ProjectFilesystem; import com.facebook.buck.util.concurrent.MoreFutures; import com.google.common.base.Optional; @@ -121,8 +123,7 @@ List<Step> buildSteps = Lists.newArrayList(); AbstractCachingBuildRule cachingRule = createRule( ImmutableSet.of(dep), - ImmutableList.<InputRule>of(FakeInputRule.createWithRuleKey("/dev/null", - new RuleKey("ae8c0f860a0ecad94ecede79b69460434eddbfbc"))), + ImmutableList.of(Paths.get("/dev/null")), buildSteps, pathToOutputFile); verifyAll(); @@ -310,7 +311,7 @@ }; BuildableAbstractCachingBuildRule cachingRule = createRule( /* deps */ ImmutableSet.<BuildRule>of(), - ImmutableList.<InputRule>of(), + ImmutableList.<Path>of(), ImmutableList.of(step), /* pathToOutputFile */ null); @@ -376,33 +377,47 @@ private static BuildableAbstractCachingBuildRule createRule( ImmutableSet<BuildRule> deps, - Iterable<InputRule> inputRules, + Iterable<Path> inputs, List<Step> buildSteps, @Nullable String pathToOutputFile) { Comparator<BuildRule> comparator = RetainOrderComparator.createComparator(deps); ImmutableSortedSet<BuildRule> sortedDeps = ImmutableSortedSet.copyOf(comparator, deps); - BuildRuleParams buildRuleParams = new FakeBuildRuleParams(buildTarget, sortedDeps); + final FileHashCache fileHashCache = FakeFileHashCache.createFromStrings(ImmutableMap.of( + "/dev/null", "ae8c0f860a0ecad94ecede79b69460434eddbfbc")); + final RuleKeyBuilderFactory ruleKeyBuilderFactory = new RuleKeyBuilderFactory() { + @Override + public RuleKey.Builder newInstance(BuildRule buildRule) { + return RuleKey.builder(buildRule, fileHashCache); + } + }; + BuildRuleParams buildRuleParams = new FakeBuildRuleParams(buildTarget, sortedDeps) { + @Override + public RuleKeyBuilderFactory getRuleKeyBuilderFactory() { + return ruleKeyBuilderFactory; + } + }; + return new BuildableAbstractCachingBuildRule(buildRuleParams, - inputRules, + inputs, pathToOutputFile, buildSteps); } private static class BuildableAbstractCachingBuildRule extends DoNotUseAbstractBuildable { - private final Iterable<InputRule> inputRules; + private final Iterable<Path> inputs; private final String pathToOutputFile; private final List<Step> buildSteps; private boolean isInitializedFromDisk = false; private BuildableAbstractCachingBuildRule(BuildRuleParams params, - Iterable<InputRule> inputRules, + Iterable<Path> inputs, @Nullable String pathToOutputFile, List<Step> buildSteps) { super(params); - this.inputRules = inputRules; + this.inputs = inputs; this.pathToOutputFile = pathToOutputFile; this.buildSteps = buildSteps; } @@ -413,8 +428,8 @@ } @Override - public Iterable<InputRule> getInputs() { - return inputRules; + public Iterable<Path> getInputs() { + return inputs; } @Override
diff --git a/test/com/facebook/buck/rules/BUCK b/test/com/facebook/buck/rules/BUCK index 527b418..7ee4b12 100644 --- a/test/com/facebook/buck/rules/BUCK +++ b/test/com/facebook/buck/rules/BUCK
@@ -6,7 +6,6 @@ 'FakeBuildableContext.java', 'FakeBuildRule.java', 'FakeBuildRuleParams.java', - 'FakeInputRule.java', 'FakeRuleKeyBuilderFactory.java', 'FakeTestRule.java', 'NonCheckingBuildRuleFactoryParams.java',
diff --git a/test/com/facebook/buck/rules/DirArtifactCacheTest.java b/test/com/facebook/buck/rules/DirArtifactCacheTest.java index 46d1ad3..5053ade 100644 --- a/test/com/facebook/buck/rules/DirArtifactCacheTest.java +++ b/test/com/facebook/buck/rules/DirArtifactCacheTest.java
@@ -20,9 +20,13 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import com.facebook.buck.model.BuildTarget; +import com.facebook.buck.util.FileHashCache; import com.facebook.buck.util.NullFileHashCache; import com.google.common.base.Charsets; import com.google.common.base.Optional; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.io.Files; @@ -32,10 +36,14 @@ import java.io.File; import java.io.IOException; +import java.nio.file.Path; import java.nio.file.attribute.FileTime; public class DirArtifactCacheTest { - @Rule public TemporaryFolder tmpDir = new TemporaryFolder(); + @Rule + public TemporaryFolder tmpDir = new TemporaryFolder(); + + private FileHashCache fileHashCache = new NullFileHashCache(); @Test public void testCacheCreation() throws IOException { @@ -53,9 +61,9 @@ /* maxCacheSizeBytes */ Optional.of(0L)); Files.write("x", fileX, Charsets.UTF_8); - InputRule inputRuleX = new InputRuleForTest(fileX); + BuildRule inputRuleX = new BuildRuleForTest(fileX); RuleKey ruleKeyX = RuleKey.builder(inputRuleX, - new NullFileHashCache()).build().getTotalRuleKey(); + fileHashCache).build().getTotalRuleKey(); assertEquals(CacheResult.MISS, dirArtifactCache.fetch(ruleKeyX, fileX)); } @@ -69,19 +77,19 @@ /* maxCacheSizeBytes */ Optional.<Long>absent()); Files.write("x", fileX, Charsets.UTF_8); - InputRule inputRuleX = new InputRuleForTest(fileX); - RuleKey ruleKeyX = RuleKey.builder(inputRuleX, new NullFileHashCache()).build().getTotalRuleKey(); + BuildRule inputRuleX = new BuildRuleForTest(fileX); + RuleKey ruleKeyX = RuleKey.builder(inputRuleX, fileHashCache).build().getTotalRuleKey(); dirArtifactCache.store(ruleKeyX, fileX); // Test that artifact overwrite works. assertEquals(CacheResult.DIR_HIT, dirArtifactCache.fetch(ruleKeyX, fileX)); - assertEquals(inputRuleX, new InputRuleForTest(fileX)); + assertEquals(inputRuleX, new BuildRuleForTest(fileX)); // Test that artifact creation works. assertTrue(fileX.delete()); assertEquals(CacheResult.DIR_HIT, dirArtifactCache.fetch(ruleKeyX, fileX)); - assertEquals(inputRuleX, new InputRuleForTest(fileX)); + assertEquals(inputRuleX, new BuildRuleForTest(fileX)); } @Test @@ -93,14 +101,14 @@ /* maxCacheSizeBytes */ Optional.of(0L)); Files.write("x", fileX, Charsets.UTF_8); - InputRule inputRuleX = new InputRuleForTest(fileX); - RuleKey ruleKeyX = RuleKey.builder(inputRuleX, new NullFileHashCache()).build().getTotalRuleKey(); + BuildRule inputRuleX = new BuildRuleForTest(fileX); + RuleKey ruleKeyX = RuleKey.builder(inputRuleX, fileHashCache).build().getTotalRuleKey(); dirArtifactCache.store(ruleKeyX, fileX); dirArtifactCache.store(ruleKeyX, fileX); // Overwrite. assertEquals(CacheResult.DIR_HIT, dirArtifactCache.fetch(ruleKeyX, fileX)); - assertEquals(inputRuleX, new InputRuleForTest(fileX)); + assertEquals(inputRuleX, new BuildRuleForTest(fileX)); } @Test @@ -117,17 +125,16 @@ Files.write("y", fileY, Charsets.UTF_8); Files.write("z", fileZ, Charsets.UTF_8); - InputRule inputRuleX = new InputRuleForTest(fileX); - InputRule inputRuleY = new InputRuleForTest(fileY); - InputRule inputRuleZ = new InputRuleForTest(fileZ); + BuildRule inputRuleX = new BuildRuleForTest(fileX); + BuildRule inputRuleY = new BuildRuleForTest(fileY); + BuildRule inputRuleZ = new BuildRuleForTest(fileZ); assertFalse(inputRuleX.equals(inputRuleY)); assertFalse(inputRuleX.equals(inputRuleZ)); assertFalse(inputRuleY.equals(inputRuleZ)); - NullFileHashCache hashCache = new NullFileHashCache(); - RuleKey ruleKeyX = RuleKey.builder(inputRuleX, hashCache).build().getTotalRuleKey(); - RuleKey ruleKeyY = RuleKey.builder(inputRuleY, hashCache).build().getTotalRuleKey(); - RuleKey ruleKeyZ = RuleKey.builder(inputRuleZ, hashCache).build().getTotalRuleKey(); + RuleKey ruleKeyX = RuleKey.builder(inputRuleX, fileHashCache).build().getTotalRuleKey(); + RuleKey ruleKeyY = RuleKey.builder(inputRuleY, fileHashCache).build().getTotalRuleKey(); + RuleKey ruleKeyZ = RuleKey.builder(inputRuleZ, fileHashCache).build().getTotalRuleKey(); assertEquals(CacheResult.MISS, dirArtifactCache.fetch(ruleKeyX, fileX)); assertEquals(CacheResult.MISS, dirArtifactCache.fetch(ruleKeyY, fileY)); @@ -145,9 +152,9 @@ assertEquals(CacheResult.DIR_HIT, dirArtifactCache.fetch(ruleKeyY, fileY)); assertEquals(CacheResult.DIR_HIT, dirArtifactCache.fetch(ruleKeyZ, fileZ)); - assertEquals(inputRuleX, new InputRuleForTest(fileX)); - assertEquals(inputRuleY, new InputRuleForTest(fileY)); - assertEquals(inputRuleZ, new InputRuleForTest(fileZ)); + assertEquals(inputRuleX, new BuildRuleForTest(fileX)); + assertEquals(inputRuleY, new BuildRuleForTest(fileY)); + assertEquals(inputRuleZ, new BuildRuleForTest(fileZ)); assertEquals(3, cacheDir.listFiles().length); @@ -226,9 +233,19 @@ assertEquals(ImmutableSet.of(fileZ, fileW), ImmutableSet.copyOf(cacheDir.listFiles())); } - private static class InputRuleForTest extends InputRule { - private InputRuleForTest(File file) { - super(file, file.getPath()); + private static class BuildRuleForTest extends FakeBuildRule { + private static final BuildRuleType TYPE = new BuildRuleType("fake"); + + private final File file; + + private BuildRuleForTest(File file) { + super(TYPE, new BuildTarget("//foo", file.getName())); + this.file = Preconditions.checkNotNull(file); + } + + @Override + public Iterable<Path> getInputs() { + return ImmutableList.of(file.toPath()); } } }
diff --git a/test/com/facebook/buck/rules/FakeBuildRule.java b/test/com/facebook/buck/rules/FakeBuildRule.java index 04e6706..a7250af 100644 --- a/test/com/facebook/buck/rules/FakeBuildRule.java +++ b/test/com/facebook/buck/rules/FakeBuildRule.java
@@ -26,6 +26,7 @@ import com.google.common.util.concurrent.ListenableFuture; import java.io.IOException; +import java.nio.file.Path; import java.util.List; import javax.annotation.Nullable; @@ -75,7 +76,7 @@ } @Override - public final Iterable<InputRule> getInputs() { + public Iterable<Path> getInputs() { return ImmutableList.of(); }
diff --git a/test/com/facebook/buck/rules/FakeInputRule.java b/test/com/facebook/buck/rules/FakeInputRule.java deleted file mode 100644 index f11027f..0000000 --- a/test/com/facebook/buck/rules/FakeInputRule.java +++ /dev/null
@@ -1,45 +0,0 @@ -/* - * Copyright 2013-present Facebook, Inc. - * - * 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.facebook.buck.rules; - -import com.facebook.buck.util.FileHashCache; -import com.google.common.base.Preconditions; - -import java.io.File; - -/** - * An {@link InputRule} that uses the {@link RuleKey} passed to its constructor rather than - * computing the value from the underlying file contents. - */ -public class FakeInputRule extends InputRule { - - private final RuleKey customRuleKey; - - private FakeInputRule(File inputFile, RuleKey customRuleKey) { - super(inputFile, inputFile.getPath()); - this.customRuleKey = Preconditions.checkNotNull(customRuleKey); - } - - @Override - public RuleKey getRuleKey(FileHashCache ignored) { - return customRuleKey; - } - - public static InputRule createWithRuleKey(String absolutePath, RuleKey customRuleKey) { - return new FakeInputRule(new File(absolutePath), Preconditions.checkNotNull(customRuleKey)); - } -}
diff --git a/test/com/facebook/buck/rules/FakeTestRule.java b/test/com/facebook/buck/rules/FakeTestRule.java index f88bca3..f3b6f26 100644 --- a/test/com/facebook/buck/rules/FakeTestRule.java +++ b/test/com/facebook/buck/rules/FakeTestRule.java
@@ -63,7 +63,7 @@ } @Override - public Iterable<InputRule> getInputs() { + public Iterable<Path> getInputs() { return ImmutableList.of(); }
diff --git a/test/com/facebook/buck/rules/InputRuleTest.java b/test/com/facebook/buck/rules/InputRuleTest.java deleted file mode 100644 index 9086901..0000000 --- a/test/com/facebook/buck/rules/InputRuleTest.java +++ /dev/null
@@ -1,62 +0,0 @@ -/* - * Copyright 2012-present Facebook, Inc. - * - * 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.facebook.buck.rules; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; - -import com.facebook.buck.util.ProjectFilesystem; -import com.google.common.collect.Iterables; - -import org.junit.Before; -import org.junit.Test; - -import java.io.File; - -public class InputRuleTest { - - private InputRule inputRule; - - @Before - public void setUp() { - ProjectFilesystem projectFilesystem = new ProjectFilesystem(new File(".")); - inputRule = InputRule.inputPathAsInputRule("src/com/facebook/buck/cli/Main.java", - projectFilesystem.getPathRelativizer()); - } - - @Test - public void testBuildTargetForInputRule() { - assertEquals("//src/com/facebook/buck/cli/Main.java:Main.java", - inputRule.getBuildTarget().getFullyQualifiedName()); - } - - @Test - public void testGetInputsIsEmpty() { - assertTrue("Even though inputRule is itself an InputRule, its getInputs() should be empty.", - Iterables.isEmpty(inputRule.getInputs())); - } - - @Test - public void testGetBuildResultTypeIsByDefinition() { - assertEquals(BuildRuleSuccess.Type.BY_DEFINITION, inputRule.getBuildResultType()); - } - - @Test - public void testToStringIsFullyQualifiedName() { - assertEquals(inputRule.getFullyQualifiedName(), inputRule.toString()); - } -}
diff --git a/test/com/facebook/buck/rules/RuleKeyTest.java b/test/com/facebook/buck/rules/RuleKeyTest.java index 12d09ca..727c6f1 100644 --- a/test/com/facebook/buck/rules/RuleKeyTest.java +++ b/test/com/facebook/buck/rules/RuleKeyTest.java
@@ -23,9 +23,15 @@ import com.facebook.buck.java.DefaultJavaLibraryRule; import com.facebook.buck.model.BuildTargetFactory; +import com.facebook.buck.rules.RuleKey.Builder; +import com.facebook.buck.testutil.TestConsole; +import com.facebook.buck.util.DefaultFileHashCache; +import com.facebook.buck.util.FileHashCache; +import com.facebook.buck.util.ProjectFilesystem; import org.junit.Test; +import java.io.File; import java.io.IOException; /** @@ -45,15 +51,26 @@ @Test public void testRuleKeyDependsOnDeps() throws IOException { BuildRuleResolver ruleResolver = new BuildRuleResolver(); + ProjectFilesystem projectFilesystem = new ProjectFilesystem(new File(".")); + final FileHashCache fileHashCache = new DefaultFileHashCache(projectFilesystem, + new TestConsole()); + AbstractBuildRuleBuilderParams builderParams = new DefaultBuildRuleBuilderParams( + projectFilesystem, + new RuleKeyBuilderFactory() { + @Override + public Builder newInstance(BuildRule buildRule) { + return RuleKey.builder(buildRule, fileHashCache); + } + }); // Create a dependent build rule, //src/com/facebook/buck/cli:common. ruleResolver.buildAndAddToIndex( - DefaultJavaLibraryRule.newJavaLibraryRuleBuilder(new FakeAbstractBuildRuleBuilderParams()) + DefaultJavaLibraryRule.newJavaLibraryRuleBuilder(builderParams) .setBuildTarget(BuildTargetFactory.newInstance("//src/com/facebook/buck/cli:common"))); // Create a java_library() rule with no deps. DefaultJavaLibraryRule.Builder javaLibraryBuilder = DefaultJavaLibraryRule - .newJavaLibraryRuleBuilder(new FakeAbstractBuildRuleBuilderParams()) + .newJavaLibraryRuleBuilder(builderParams) .setBuildTarget(BuildTargetFactory.newInstance("//src/com/facebook/buck/cli:cli")) // The source file must be an existing file or else RuleKey.Builder.setVal(File) will throw // an IOException, which is caught and then results in the rule being flagged as
diff --git a/test/com/facebook/buck/testutil/BUCK b/test/com/facebook/buck/testutil/BUCK index fa11daf..ab32c43 100644 --- a/test/com/facebook/buck/testutil/BUCK +++ b/test/com/facebook/buck/testutil/BUCK
@@ -15,6 +15,7 @@ '//src/com/facebook/buck/shell:steps', '//src/com/facebook/buck/step:step', '//src/com/facebook/buck/util:io', + '//src/com/facebook/buck/util:util', ], visibility = [ '//test/...',
diff --git a/test/com/facebook/buck/testutil/FakeFileHashCache.java b/test/com/facebook/buck/testutil/FakeFileHashCache.java new file mode 100644 index 0000000..f95a997 --- /dev/null +++ b/test/com/facebook/buck/testutil/FakeFileHashCache.java
@@ -0,0 +1,56 @@ +/* + * Copyright 2013-present Facebook, Inc. + * + * 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.facebook.buck.testutil; + +import com.facebook.buck.util.FileHashCache; +import com.google.common.collect.ImmutableMap; +import com.google.common.hash.HashCode; + +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Map; + +/** + * FileHashCache that is populated with a fixed set of entries. + */ +public class FakeFileHashCache implements FileHashCache { + + private final ImmutableMap<Path, HashCode> pathsToHashes; + + private FakeFileHashCache(Map<Path, HashCode> pathsToHashes) { + this.pathsToHashes = ImmutableMap.copyOf(pathsToHashes); + } + + public static FakeFileHashCache createFromStrings(Map<String, String> pathsToHashes) { + ImmutableMap.Builder<Path, HashCode> builder = ImmutableMap.builder(); + for (Map.Entry<String, String> entry : pathsToHashes.entrySet()) { + builder.put(Paths.get(entry.getKey()), HashCode.fromString(entry.getValue())); + } + return new FakeFileHashCache(builder.build()); + } + + @Override + public boolean contains(Path path) { + return pathsToHashes.containsKey(path); + } + + @Override + public HashCode get(Path path) { + return pathsToHashes.get(path); + } + +}
diff --git a/test/com/facebook/buck/util/ConcurrentMapFileHashCacheTest.java b/test/com/facebook/buck/util/ConcurrentMapFileHashCacheTest.java index 8c15eb5..d75340b 100644 --- a/test/com/facebook/buck/util/ConcurrentMapFileHashCacheTest.java +++ b/test/com/facebook/buck/util/ConcurrentMapFileHashCacheTest.java
@@ -16,8 +16,8 @@ package com.facebook.buck.util; -import static com.facebook.buck.testutil.WatchEvents.createPathEvent; import static com.facebook.buck.testutil.WatchEvents.createOverflowEvent; +import static com.facebook.buck.testutil.WatchEvents.createPathEvent; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -36,60 +36,60 @@ @Test public void whenPathIsPutCacheContainsPath() { - ConcurrentMapFileHashCache cache = new ConcurrentMapFileHashCache(new ProjectFilesystem(new File(".")), new TestConsole()); + DefaultFileHashCache cache = new DefaultFileHashCache(new ProjectFilesystem(new File(".")), new TestConsole()); Path path = new File("SomeClass.java").toPath(); - cache.put(path, HashCode.fromInt(42)); + cache.loadingCache.put(path, HashCode.fromInt(42)); assertTrue("Cache should contain path", cache.contains(path)); } @Test public void whenPathIsPutPathGetReturnsHash() { - ConcurrentMapFileHashCache cache = new ConcurrentMapFileHashCache(new ProjectFilesystem(new File(".")), new TestConsole()); + DefaultFileHashCache cache = new DefaultFileHashCache(new ProjectFilesystem(new File(".")), new TestConsole()); Path path = new File("SomeClass.java").toPath(); HashCode hash = HashCode.fromInt(42); - cache.put(path, hash); + cache.loadingCache.put(path, hash); assertEquals("Cache should contain hash", hash, cache.get(path)); } @Test public void whenNotifiedOfOverflowEventCacheIsCleared() throws IOException { - ConcurrentMapFileHashCache cache = new ConcurrentMapFileHashCache(new ProjectFilesystem(new File(".")), new TestConsole()); + DefaultFileHashCache cache = new DefaultFileHashCache(new ProjectFilesystem(new File(".")), new TestConsole()); Path path = new File("SomeClass.java").toPath(); HashCode hash = HashCode.fromInt(42); - cache.put(path, hash); + cache.loadingCache.put(path, hash); cache.onFileSystemChange(createOverflowEvent()); assertFalse("Cache should not contain path", cache.contains(path)); } @Test public void whenNotifiedOfCreateEventCacheEntryIsRemoved() throws IOException { - ConcurrentMapFileHashCache cache = new ConcurrentMapFileHashCache(new ProjectFilesystem(new File(".")), new TestConsole()); + DefaultFileHashCache cache = new DefaultFileHashCache(new ProjectFilesystem(new File(".")), new TestConsole()); File file = new File("SomeClass.java"); Path path = file.toPath(); HashCode hash = HashCode.fromInt(42); - cache.put(path, hash); + cache.loadingCache.put(path, hash); cache.onFileSystemChange(createPathEvent(file, StandardWatchEventKinds.ENTRY_CREATE)); assertFalse("Cache should not contain path", cache.contains(path)); } @Test public void whenNotifiedOfChangeEventCacheEntryIsRemoved() throws IOException { - ConcurrentMapFileHashCache cache = new ConcurrentMapFileHashCache(new ProjectFilesystem(new File(".")), new TestConsole()); + DefaultFileHashCache cache = new DefaultFileHashCache(new ProjectFilesystem(new File(".")), new TestConsole()); File file = new File("SomeClass.java"); Path path = file.toPath(); HashCode hash = HashCode.fromInt(42); - cache.put(path, hash); + cache.loadingCache.put(path, hash); cache.onFileSystemChange(createPathEvent(file, StandardWatchEventKinds.ENTRY_MODIFY)); assertFalse("Cache should not contain path", cache.contains(path)); } @Test public void whenNotifiedOfDeleteEventCacheEntryIsRemoved() throws IOException { - ConcurrentMapFileHashCache cache = new ConcurrentMapFileHashCache(new ProjectFilesystem(new File(".")), new TestConsole()); + DefaultFileHashCache cache = new DefaultFileHashCache(new ProjectFilesystem(new File(".")), new TestConsole()); File file = new File("SomeClass.java"); Path path = file.toPath(); HashCode hash = HashCode.fromInt(42); - cache.put(path, hash); + cache.loadingCache.put(path, hash); cache.onFileSystemChange(createPathEvent(file, StandardWatchEventKinds.ENTRY_DELETE)); assertFalse("Cache should not contain path", cache.contains(path)); }