Skip to content

Commit

Permalink
Move verifyMarkerData into RepositoryFunction
Browse files Browse the repository at this point in the history 
And for new_XX_repository rules, add BUILD and WORKSPACE file into
markerData.

In markerData, key starting with FILE: can be an absolute label or an
absoulte path, but the latter one will be depracated in future.

Fixed bazelbuild#3093

Change-Id: Ic3e16c123b3f1f781ab12c41d13f5e540b05686c
PiperOrigin-RevId: 160382024
  • Loading branch information
@meteorcloudy @hlopko
meteorcloudy authored and hlopko committed Jun 28, 2017
1 parent c2de565 commit ac51982
Show file tree
Hide file tree
Showing 9 changed files with 258 additions and 138 deletions.
2 changes: 1 addition & 1 deletion src/main/java/com/google/devtools/build/lib/bazel/repository/NewGitRepositoryFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public RepositoryDirectoryValue.Builder fetch(Rule rule, Path outputDirectory,

createDirectory(outputDirectory, rule);
GitCloner.clone(rule, outputDirectory, env.getListener(), clientEnvironment, downloader);
fileHandler.finishFile(outputDirectory);
fileHandler.finishFile(rule, outputDirectory, markerData);

return RepositoryDirectoryValue.builder().setPath(outputDirectory);
}
Expand Down
18 changes: 9 additions & 9 deletions src/main/java/com/google/devtools/build/lib/bazel/repository/NewHttpArchiveFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ public RepositoryDirectoryValue.Builder fetch(Rule rule, Path outputDirectory,
rule, outputDirectory, env.getListener(), clientEnvironment);

// Decompress.
Path decompressed;
WorkspaceAttributeMapper mapper = WorkspaceAttributeMapper.of(rule);
String prefix = null;
if (mapper.isAttributeValueExplicitlySpecified("strip_prefix")) {
Expand All @@ -72,16 +71,17 @@ public RepositoryDirectoryValue.Builder fetch(Rule rule, Path outputDirectory,
throw new RepositoryFunctionException(e, Transience.PERSISTENT);
}
}
decompressed = DecompressorValue.decompress(DecompressorDescriptor.builder()
.setTargetKind(rule.getTargetKind())
.setTargetName(rule.getName())
.setArchivePath(downloadedPath)
.setRepositoryPath(outputDirectory)
.setPrefix(prefix)
.build());
DecompressorValue.decompress(
DecompressorDescriptor.builder()
.setTargetKind(rule.getTargetKind())
.setTargetName(rule.getName())
.setArchivePath(downloadedPath)
.setRepositoryPath(outputDirectory)
.setPrefix(prefix)
.build());

// Finally, write WORKSPACE and BUILD files.
fileHandler.finishFile(outputDirectory);
fileHandler.finishFile(rule, outputDirectory, markerData);

return RepositoryDirectoryValue.builder().setPath(outputDirectory);
}
Expand Down
70 changes: 3 additions & 67 deletions ...java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.bazel.repository.DecompressorDescriptor;
Expand All @@ -25,18 +24,17 @@
import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader;
import com.google.devtools.build.lib.bazel.repository.downloader.HttpUtils;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.NativeClassObjectConstructor;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.SkylarkClassObject;
import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException;
import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper;
import com.google.devtools.build.lib.skyframe.FileSymlinkException;
import com.google.devtools.build.lib.skyframe.FileValue;
import com.google.devtools.build.lib.skyframe.InconsistentFilesystemException;
import com.google.devtools.build.lib.skyframe.PackageLookupValue;
import com.google.devtools.build.lib.skylarkinterface.Param;
import com.google.devtools.build.lib.skylarkinterface.ParamType;
import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
Expand Down Expand Up @@ -64,7 +62,6 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Objects;

/** Skylark API for the repository_rule's context. */
@SkylarkModule(
Expand Down Expand Up @@ -688,34 +685,9 @@ public String toString() {
return "repository_ctx[" + rule.getLabel() + "]";
}

private static RootedPath getRootedPathFromLabel(Label label, Environment env)
throws InterruptedException, EvalException {
// Look for package.
if (label.getPackageIdentifier().getRepository().isDefault()) {
try {
label = Label.create(label.getPackageIdentifier().makeAbsolute(), label.getName());
} catch (LabelSyntaxException e) {
throw new AssertionError(e); // Can't happen because the input label is valid
}
}
SkyKey pkgSkyKey = PackageLookupValue.key(label.getPackageIdentifier());
PackageLookupValue pkgLookupValue = (PackageLookupValue) env.getValue(pkgSkyKey);
if (pkgLookupValue == null) {
throw SkylarkRepositoryFunction.restart();
}
if (!pkgLookupValue.packageExists()) {
throw new EvalException(Location.BUILTIN,
"Unable to load package for " + label + ": not found.");
}

// And now for the file
Path packageRoot = pkgLookupValue.getRoot();
return RootedPath.toRootedPath(packageRoot, label.toPathFragment());
}

// Resolve the label given by value into a file path.
private SkylarkPath getPathFromLabel(Label label) throws EvalException, InterruptedException {
RootedPath rootedPath = getRootedPathFromLabel(label, env);
RootedPath rootedPath = RepositoryFunction.getRootedPathFromLabel(label, env);
SkyKey fileSkyKey = FileValue.key(rootedPath);
FileValue fileValue = null;
try {
Expand All @@ -726,7 +698,7 @@ private SkylarkPath getPathFromLabel(Label label) throws EvalException, Interrup
}

if (fileValue == null) {
throw SkylarkRepositoryFunction.restart();
throw RepositoryFunction.restart();
}
if (!fileValue.isFile()) {
throw new EvalException(Location.BUILTIN,
Expand All @@ -738,40 +710,4 @@ private SkylarkPath getPathFromLabel(Label label) throws EvalException, Interrup
return new SkylarkPath(rootedPath.asPath());
}

private static boolean verifyLabelMarkerData(String key, String value, Environment env)
throws InterruptedException {
Preconditions.checkArgument(key.startsWith("FILE:"));
try {
Label label = Label.parseAbsolute(key.substring(5));
RootedPath rootedPath = getRootedPathFromLabel(label, env);
SkyKey fileSkyKey = FileValue.key(rootedPath);
FileValue fileValue = (FileValue) env.getValueOrThrow(fileSkyKey, IOException.class,
FileSymlinkException.class, InconsistentFilesystemException.class);

if (fileValue == null || !fileValue.isFile()) {
return false;
}

return Objects.equals(value, Integer.toString(fileValue.realFileStateValue().hashCode()));
} catch (LabelSyntaxException e) {
throw new IllegalStateException(
"Key " + key + " is not a correct file key (should be in form FILE:label)", e);
} catch (IOException | FileSymlinkException | InconsistentFilesystemException
| EvalException e) {
// Consider those exception to be a cause for invalidation
return false;
}
}

static boolean verifyMarkerDataForFiles(Map<String, String> markerData, Environment env)
throws InterruptedException {
for (Map.Entry<String, String> entry : markerData.entrySet()) {
if (entry.getKey().startsWith("FILE:")) {
if (!verifyLabelMarkerData(entry.getKey(), entry.getValue(), env)) {
return false;
}
}
}
return true;
}
}
32 changes: 1 addition & 31 deletions ...ava/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.RuleDefinition;
import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
Expand All @@ -40,32 +39,12 @@
*/
public class SkylarkRepositoryFunction extends RepositoryFunction {

/**
* An exception thrown when a dependency is missing to notify the SkyFunction from a skylark
* evaluation.
*/
private static class SkylarkRepositoryMissingDependencyException extends EvalException {

SkylarkRepositoryMissingDependencyException() {
super(Location.BUILTIN, "Internal exception");
}
}

private final HttpDownloader httpDownloader;

public SkylarkRepositoryFunction(HttpDownloader httpDownloader) {
this.httpDownloader = httpDownloader;
}

/**
* Skylark repository context functions can throw the result of this function to notify the
* SkylarkRepositoryFunction that a dependency was missing and the evaluation of the function must
* be restarted.
*/
static EvalException restart() {
return new SkylarkRepositoryMissingDependencyException();
}

@Nullable
@Override
public RepositoryDirectoryValue.Builder fetch(Rule rule, Path outputDirectory,
Expand Down Expand Up @@ -106,7 +85,7 @@ public RepositoryDirectoryValue.Builder fetch(Rule rule, Path outputDirectory,
Transience.PERSISTENT);
}
} catch (EvalException e) {
if (e.getCause() instanceof SkylarkRepositoryMissingDependencyException) {
if (e.getCause() instanceof RepositoryMissingDependencyException) {
// A dependency is missing, cleanup and returns null
try {
if (outputDirectory.exists()) {
Expand Down Expand Up @@ -137,15 +116,6 @@ private static Iterable<String> getEnviron(Rule rule) {
return (Iterable<String>) rule.getAttributeContainer().getAttr("$environ");
}

@Override
public boolean verifyMarkerData(Rule rule, Map<String, String> markerData, Environment env)
throws InterruptedException {
if (verifyEnvironMarkerData(markerData, env, getEnviron(rule))) {
return SkylarkRepositoryContext.verifyMarkerDataForFiles(markerData, env);
}
return false;
}

@Override
protected boolean isLocal(Rule rule) {
return (Boolean) rule.getAttributeContainer().getAttr("$local");
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/google/devtools/build/lib/rules/repository/NewLocalRepositoryFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public RepositoryDirectoryValue.Builder fetch(Rule rule, Path outputDirectory,
return null;
}

fileHandler.finishFile(outputDirectory);
fileHandler.finishFile(rule, outputDirectory, markerData);

return RepositoryDirectoryValue.builder().setPath(outputDirectory).setSourceDir(directoryValue);
}
Expand Down
82 changes: 53 additions & 29 deletions src/main/java/com/google/devtools/build/lib/rules/repository/NewRepositoryFileHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
import com.google.devtools.build.skyframe.SkyKey;
import java.io.IOException;
import java.util.Map;

/**
* Encapsulates the 2-step behavior of creating workspace and build files for the new_*_repository
Expand Down Expand Up @@ -59,9 +60,10 @@ public boolean prepareFile(Rule rule, Environment env)
return true;
}

public void finishFile(Path outputDirectory) throws RepositoryFunctionException {
this.workspaceFileHandler.finishFile(outputDirectory);
this.buildFileHandler.finishFile(outputDirectory);
public void finishFile(Rule rule, Path outputDirectory, Map<String, String> markerData)
throws RepositoryFunctionException {
this.workspaceFileHandler.finishFile(rule, outputDirectory, markerData);
this.buildFileHandler.finishFile(rule, outputDirectory, markerData);
}

/**
Expand Down Expand Up @@ -142,57 +144,79 @@ public boolean prepareFile(Rule rule, Environment env)
* @throws IllegalStateException if {@link #prepareFile} was not called before this, or if
* {@link #prepareFile} failed and this was called.
*/
public void finishFile(Path outputDirectory) throws RepositoryFunctionException {
public void finishFile(Rule rule, Path outputDirectory, Map<String, String> markerData)
throws RepositoryFunctionException {
if (fileValue != null) {
// Link x/FILENAME to <build_root>/x.FILENAME.
symlinkFile(fileValue, filename, outputDirectory);
String fileAttribute = getFileAttributeValue(rule);
String fileKey;
if (LabelValidator.isAbsolute(fileAttribute)) {
fileKey = getFileAttributeAsLabel(rule).toString();
} else {
// TODO(pcloudy): Don't add absolute path into markerData once it's not supported
fileKey = fileValue.realRootedPath().asPath().getPathString();
}
markerData.put(
"FILE:" + fileKey, Integer.toString(fileValue.realFileStateValue().hashCode()));
} else if (fileContent != null) {
RepositoryFunction.writeFile(outputDirectory, filename, fileContent);
} else {
throw new IllegalStateException("prepareFile() must be called before finishFile()");
}
}

private FileValue getFileValue(Rule rule, Environment env)
throws RepositoryFunctionException, InterruptedException {
private String getFileAttributeValue(Rule rule) throws RepositoryFunctionException {
WorkspaceAttributeMapper mapper = WorkspaceAttributeMapper.of(rule);
String fileAttribute;
try {
fileAttribute = mapper.get(getFileAttrName(), Type.STRING);
} catch (EvalException e) {
throw new RepositoryFunctionException(e, Transience.PERSISTENT);
}
return fileAttribute;
}

private Label getFileAttributeAsLabel(Rule rule) throws RepositoryFunctionException {
Label label;
try {
// Parse a label
label = Label.parseAbsolute(getFileAttributeValue(rule));
} catch (LabelSyntaxException ex) {
throw new RepositoryFunctionException(
new EvalException(
rule.getLocation(),
String.format(
"In %s the '%s' attribute does not specify a valid label: %s",
rule, getFileAttrName(), ex.getMessage())),
Transience.PERSISTENT);
}
return label;
}

private FileValue getFileValue(Rule rule, Environment env)
throws RepositoryFunctionException, InterruptedException {
String fileAttribute = getFileAttributeValue(rule);
RootedPath rootedFile;

if (LabelValidator.isAbsolute(fileAttribute)) {
try {
// Parse a label
Label label = Label.parseAbsolute(fileAttribute);
SkyKey pkgSkyKey = PackageLookupValue.key(label.getPackageIdentifier());
PackageLookupValue pkgLookupValue = (PackageLookupValue) env.getValue(pkgSkyKey);
if (pkgLookupValue == null) {
return null;
}
if (!pkgLookupValue.packageExists()) {
throw new RepositoryFunctionException(
new EvalException(
rule.getLocation(),
"Unable to load package for " + fileAttribute + ": not found."),
Transience.PERSISTENT);
}

// And now for the file
Path packageRoot = pkgLookupValue.getRoot();
rootedFile = RootedPath.toRootedPath(packageRoot, label.toPathFragment());
} catch (LabelSyntaxException ex) {
Label label = getFileAttributeAsLabel(rule);
SkyKey pkgSkyKey = PackageLookupValue.key(label.getPackageIdentifier());
PackageLookupValue pkgLookupValue = (PackageLookupValue) env.getValue(pkgSkyKey);
if (pkgLookupValue == null) {
return null;
}
if (!pkgLookupValue.packageExists()) {
throw new RepositoryFunctionException(
new EvalException(
rule.getLocation(),
String.format(
"In %s the '%s' attribute does not specify a valid label: %s",
rule, getFileAttrName(), ex.getMessage())),
"Unable to load package for " + fileAttribute + ": not found."),
Transience.PERSISTENT);
}

// And now for the file
Path packageRoot = pkgLookupValue.getRoot();
rootedFile = RootedPath.toRootedPath(packageRoot, label.toPathFragment());
} else {
// TODO(dmarting): deprecate using a path for the workspace_file attribute.
PathFragment file = PathFragment.create(fileAttribute);
Expand Down
Loading

0 comments on commit ac51982

Please sign in to comment.