Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

Commit

Permalink
Introduce a level of indirection between files and paths to files.
Browse files Browse the repository at this point in the history
Summary: This allows rules to depend on a target where they could also expect a path.

Test Plan: buck test --all
  • Loading branch information
shs96c authored and bolinfest committed Jun 21, 2013
1 parent 9eaa707 commit 2aa814b
Show file tree
Hide file tree
Showing 27 changed files with 610 additions and 53 deletions.
3 changes: 2 additions & 1 deletion src/com/facebook/buck/android/AndroidLibraryRule.java
Expand Up @@ -21,6 +21,7 @@
import com.facebook.buck.java.JavacOptions;
import com.facebook.buck.model.BuildTarget;
import com.facebook.buck.model.BuildTargetPattern;
import com.facebook.buck.rules.SourcePath;
import com.facebook.buck.rules.BuildContext;
import com.facebook.buck.rules.BuildRuleParams;
import com.facebook.buck.rules.BuildRuleResolver;
Expand All @@ -45,7 +46,7 @@ public class AndroidLibraryRule extends DefaultJavaLibraryRule {
@VisibleForTesting
public AndroidLibraryRule(BuildRuleParams buildRuleParams,
Set<String> srcs,
Set<String> resources,
Set<SourcePath> resources,
Optional<String> proguardConfig,
JavacOptions javacOptions,
Optional<String> manifestFile) {
Expand Down
3 changes: 2 additions & 1 deletion src/com/facebook/buck/android/RobolectricTestRule.java
Expand Up @@ -21,6 +21,7 @@
import com.facebook.buck.java.JavaTestRule;
import com.facebook.buck.java.JavacOptions;
import com.facebook.buck.model.BuildTarget;
import com.facebook.buck.rules.SourcePath;
import com.facebook.buck.rules.BuildContext;
import com.facebook.buck.rules.BuildRuleResolver;
import com.facebook.buck.rules.BuildRuleParams;
Expand All @@ -36,7 +37,7 @@ public class RobolectricTestRule extends JavaTestRule {

protected RobolectricTestRule(BuildRuleParams buildRuleParams,
Set<String> srcs,
Set<String> resources,
Set<SourcePath> resources,
Set<String> labels,
Optional<String> proguardConfig,
JavacOptions javacOptions,
Expand Down
1 change: 1 addition & 0 deletions src/com/facebook/buck/cli/BuckConfig.java
Expand Up @@ -16,6 +16,7 @@

package com.facebook.buck.cli;

import com.facebook.buck.java.DefaultJavaPackageFinder;
import com.facebook.buck.model.BuildTarget;
import com.facebook.buck.parser.BuildTargetParser;
import com.facebook.buck.parser.NoSuchBuildTargetException;
Expand Down
1 change: 1 addition & 0 deletions src/com/facebook/buck/cli/TestCommand.java
Expand Up @@ -18,6 +18,7 @@

import com.facebook.buck.command.Build;
import com.facebook.buck.graph.AbstractBottomUpTraversal;
import com.facebook.buck.java.DefaultJavaPackageFinder;
import com.facebook.buck.java.GenerateCodeCoverageReportStep;
import com.facebook.buck.java.InstrumentStep;
import com.facebook.buck.java.JUnitStep;
Expand Down
1 change: 1 addition & 0 deletions src/com/facebook/buck/cli/TestCommandOptions.java
Expand Up @@ -16,6 +16,7 @@

package com.facebook.buck.cli;

import com.facebook.buck.java.DefaultJavaPackageFinder;
import com.google.common.base.Optional;
import com.google.common.base.Supplier;
import com.google.common.base.Suppliers;
Expand Down
1 change: 1 addition & 0 deletions src/com/facebook/buck/java/BUCK
@@ -1,5 +1,6 @@
SUPPORT_SRCS = [
'AnnotationProcessingParams.java',
'DefaultJavaPackageFinder.java',
'HasClasspathEntries.java',
'JavacOptions.java',
]
Expand Down
48 changes: 34 additions & 14 deletions src/com/facebook/buck/java/DefaultJavaLibraryRule.java
Expand Up @@ -33,6 +33,7 @@
import com.facebook.buck.rules.JavaPackageFinder;
import com.facebook.buck.rules.ResourcesAttributeBuilder;
import com.facebook.buck.rules.RuleKey;
import com.facebook.buck.rules.SourcePath;
import com.facebook.buck.rules.SrcsAttributeBuilder;
import com.facebook.buck.step.Step;
import com.facebook.buck.step.fs.MakeCleanDirectoryStep;
Expand Down Expand Up @@ -85,7 +86,7 @@ public class DefaultJavaLibraryRule extends AbstractCachingBuildRule

private final ImmutableSortedSet<String> srcs;

private final ImmutableSortedSet<String> resources;
private final ImmutableSortedSet<SourcePath> resources;

private final Optional<String> outputJar;

Expand Down Expand Up @@ -147,7 +148,7 @@ public ImmutableSet<String> apply(String classPath) {

protected DefaultJavaLibraryRule(BuildRuleParams buildRuleParams,
Set<String> srcs,
Set<String> resources,
Set<? extends SourcePath> resources,
Optional<String> proguardConfig,
boolean exportDeps,
JavacOptions javacOptions) {
Expand All @@ -167,10 +168,13 @@ protected DefaultJavaLibraryRule(BuildRuleParams buildRuleParams,
// Note that both srcs and resources are sorted so that the list order is consistent even if
// the iteration order of the sets passed to the constructor changes. See
// AbstractBuildRule.getInputsToCompareToOutput() for details.
inputsToConsiderForCachingPurposes = ImmutableList.<String>builder()
.addAll(this.srcs)
.addAll(this.resources)
.build();
ImmutableList.Builder<String> builder = ImmutableList.<String>builder().addAll(this.srcs);
for (SourcePath resource : resources) {
builder.add(resource.asReference());
}
inputsToConsiderForCachingPurposes = builder.build();



outputClasspathEntriesSupplier =
Suppliers.memoize(new Supplier<ImmutableSet<String>>() {
Expand Down Expand Up @@ -316,7 +320,7 @@ public boolean isLibrary() {
protected RuleKey.Builder appendToRuleKey(RuleKey.Builder builder) {
super.appendToRuleKey(builder)
.set("srcs", srcs)
.set("resources", resources)
.setSourcePaths("resources", resources)
.set("proguard", proguardConfig)
.set("exportDeps", exportDeps);
javacOptions.appendToRuleKey(builder);
Expand Down Expand Up @@ -454,7 +458,7 @@ protected final List<Step> buildInternal(BuildContext context) throws IOExceptio


// If there are resources, then link them to the appropriate place in the classes directory.
addResourceCommands(commands, outputDirectory, context.getJavaPackageFinder());
addResourceCommands(context, commands, outputDirectory, context.getJavaPackageFinder());

if (outputJar.isPresent()) {
commands.add(new MakeCleanDirectoryStep(getOutputJarDirPath(getBuildTarget())));
Expand Down Expand Up @@ -557,11 +561,16 @@ public ImmutableSet<String> apply(ImmutableSet<String> failedImports) {


@VisibleForTesting
void addResourceCommands(ImmutableList.Builder<Step> commands,
void addResourceCommands(BuildContext context,
ImmutableList.Builder<Step> commands,
String outputDirectory,
JavaPackageFinder javaPackageFinder) {
if (!resources.isEmpty()) {
for (String resource : resources) {
String targetPackageDir = javaPackageFinder.findJavaPackageForPath(
getBuildTarget().getBasePathWithSlash())
.replace('.', File.separatorChar);

for (SourcePath rawResource : resources) {
// If the path to the file defining this rule were:
// "first-party/orca/lib-http/tests/com/facebook/orca/BUILD"
//
Expand All @@ -575,9 +584,21 @@ void addResourceCommands(ImmutableList.Builder<Step> commands,
// "com/facebook/orca/protocol/base/batch_exception1.txt"
//
// Therefore, some path-wrangling is required to produce the correct string.

String resource = rawResource.resolve(context).toString();
String javaPackageAsPath = javaPackageFinder.findJavaPackageFolderForPath(resource);
String relativeSymlinkPath;
if ("".equals(javaPackageAsPath)) {


if (resource.startsWith(BuckConstant.BUCK_OUTPUT_DIRECTORY) ||
resource.startsWith(BuckConstant.GEN_DIR) ||
resource.startsWith(BuckConstant.BIN_DIR) ||
resource.startsWith(BuckConstant.ANNOTATION_DIR)) {
// Handle the case where we depend on the output of another BuildRule. In that case, just
// grab the output and put in the same package as this target would be in.
relativeSymlinkPath = String.format(
"%s/%s", targetPackageDir, rawResource.resolve(context).getFileName());
} else if ("".equals(javaPackageAsPath)) {
// In this case, the project root is acting as the default package, so the resource path
// works fine.
relativeSymlinkPath = resource;
Expand All @@ -591,7 +612,6 @@ void addResourceCommands(ImmutableList.Builder<Step> commands,
relativeSymlinkPath = resource.substring(lastIndex);
}
String target = outputDirectory + '/' + relativeSymlinkPath;

MkdirAndSymlinkFileStep link = new MkdirAndSymlinkFileStep(resource, target);
commands.add(link);
}
Expand All @@ -611,7 +631,7 @@ public static class Builder extends AbstractBuildRuleBuilder<DefaultJavaLibraryR
SrcsAttributeBuilder, ResourcesAttributeBuilder {

protected Set<String> srcs = Sets.newHashSet();
protected Set<String> resources = Sets.newHashSet();
protected Set<SourcePath> resources = Sets.newHashSet();
protected final AnnotationProcessingParams.Builder annotationProcessingBuilder =
new AnnotationProcessingParams.Builder();
protected boolean exportDeps = false;
Expand Down Expand Up @@ -660,7 +680,7 @@ public Builder addSrc(String src) {
}

@Override
public Builder addResource(String relativePathToResource) {
public Builder addResource(SourcePath relativePathToResource) {
resources.add(relativePathToResource);
return this;
}
Expand Down
Expand Up @@ -14,7 +14,7 @@
* under the License.
*/

package com.facebook.buck.cli;
package com.facebook.buck.java;

import com.facebook.buck.rules.JavaPackageFinder;
import com.facebook.buck.util.HumanReadableException;
Expand All @@ -27,7 +27,7 @@
import java.io.File;
import java.util.Deque;

class DefaultJavaPackageFinder implements JavaPackageFinder {
public class DefaultJavaPackageFinder implements JavaPackageFinder {

/**
* Each element in this set is a path prefix from the root of the repository.
Expand Down
3 changes: 2 additions & 1 deletion src/com/facebook/buck/java/JavaTestRule.java
Expand Up @@ -18,6 +18,7 @@

import com.facebook.buck.android.UberRDotJavaUtil;
import com.facebook.buck.model.BuildTarget;
import com.facebook.buck.rules.SourcePath;
import com.facebook.buck.rules.BuildContext;
import com.facebook.buck.rules.BuildRule;
import com.facebook.buck.rules.BuildRuleParams;
Expand Down Expand Up @@ -71,7 +72,7 @@ public class JavaTestRule extends DefaultJavaLibraryRule implements TestRule {

protected JavaTestRule(BuildRuleParams buildRuleParams,
Set<String> srcs,
Set<String> resources,
Set<SourcePath> resources,
Set<String> labels,
Optional<String> proguardConfig,
JavacOptions javacOptions,
Expand Down
15 changes: 13 additions & 2 deletions src/com/facebook/buck/parser/AbstractBuildRuleFactory.java
Expand Up @@ -18,6 +18,8 @@

import com.facebook.buck.model.BuildTarget;
import com.facebook.buck.model.BuildTargetPattern;
import com.facebook.buck.rules.BuildTargetSourcePath;
import com.facebook.buck.rules.SourcePath;
import com.facebook.buck.rules.AbstractBuildRuleBuilder;
import com.facebook.buck.rules.ResourcesAttributeBuilder;
import com.facebook.buck.rules.SrcsAttributeBuilder;
Expand Down Expand Up @@ -71,9 +73,18 @@ public T newInstance(BuildRuleFactoryParams params)

// resources
if (builder instanceof ResourcesAttributeBuilder) {
ResourcesAttributeBuilder attributeBuilder = (ResourcesAttributeBuilder) builder;

for (String resource : params.getOptionalListAttribute("resources")) {
String relativePath = params.resolveFilePathRelativeToBuildFileDirectory(resource);
((ResourcesAttributeBuilder)builder).addResource(relativePath);
SourcePath path = params.asSourcePath(resource);
attributeBuilder.addResource(path);

// Remember to add the resource as a dependency. On the down side, this means that the file
// will appear on the classpath, but there's no elegant way (yet) to avoid this.
// TODO(simons): A smarter, more elegant way of adding additional deps not on the classpath.
if (path instanceof BuildTargetSourcePath) {
builder.addDep(((BuildTargetSourcePath) path).getTarget());
}
}
}

Expand Down
23 changes: 22 additions & 1 deletion src/com/facebook/buck/parser/BuildRuleFactoryParams.java
Expand Up @@ -18,6 +18,9 @@

import com.facebook.buck.model.BuildFileTree;
import com.facebook.buck.model.BuildTarget;
import com.facebook.buck.rules.BuildTargetSourcePath;
import com.facebook.buck.rules.FileSourcePath;
import com.facebook.buck.rules.SourcePath;
import com.facebook.buck.util.BuckConstant;
import com.facebook.buck.util.Console;
import com.facebook.buck.util.HumanReadableException;
Expand Down Expand Up @@ -47,6 +50,7 @@ public final class BuildRuleFactoryParams {
public final BuildTargetParser buildTargetParser;
public final BuildTargetPatternParser buildTargetPatternParser;
public final BuildTarget target;
private final ParseContext sourcePathContext;
private final ParseContext visibilityParseContext;
private final boolean ignoreFileExistenceChecks;

Expand Down Expand Up @@ -83,8 +87,9 @@ public BuildRuleFactoryParams(
this.buildFiles = buildFiles;
this.buildTargetParser = buildTargetParser;
this.buildTargetPatternParser = new BuildTargetPatternParser(filesystem);
this.target = target;
this.target = Preconditions.checkNotNull(target);
this.visibilityParseContext = ParseContext.forVisibilityArgument();
this.sourcePathContext = ParseContext.forBaseName(target.getBaseName());
this.ignoreFileExistenceChecks = ignoreFileExistenceChecks;

this.resolveFilePathRelativeToBuildFileDirectoryTransform = new Function<String, String>() {
Expand Down Expand Up @@ -200,6 +205,22 @@ private String resolvePathAgainstBuildTargetBase(String path) {
}
}

public SourcePath asSourcePath(String resource) {
// TODO(simons): Don't hard code this check for built-target-ism
if (resource.startsWith(BuildTarget.BUILD_TARGET_PREFIX) || resource.charAt(0) == ':') {
try {
BuildTarget buildTarget = buildTargetParser.parse(resource, sourcePathContext);
return new BuildTargetSourcePath(buildTarget);
} catch (NoSuchBuildTargetException e) {
throw new HumanReadableException(
"Unable to find build target '%s' while parsing definition of %s", resource, target);
}
} else {
String relativePath = resolveFilePathRelativeToBuildFileDirectory(resource);
return new FileSourcePath(relativePath);
}
}

public String getRequiredStringAttribute(String attributeName) {
Optional<String> value = getOptionalStringAttribute(attributeName);
if (value.isPresent()) {
Expand Down
50 changes: 50 additions & 0 deletions src/com/facebook/buck/rules/AbstractSourcePath.java
@@ -0,0 +1,50 @@
/*
* 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.google.common.base.Preconditions;

/**
* Abstract base class for implementations of {@link SourcePath}.
*/
abstract class AbstractSourcePath implements SourcePath {
@Override
public int compareTo(SourcePath o) {
Preconditions.checkNotNull(o);

return asReference().compareTo(o.asReference());
}

@Override
public int hashCode() {
return asReference().hashCode();
}

@Override
public boolean equals(Object that) {
if (that == null || !(that instanceof SourcePath)) {
return false;
}

return asReference().equals(((SourcePath) that).asReference());
}

@Override
public String toString() {
return asReference();
}
}

0 comments on commit 2aa814b

Please sign in to comment.