From 44847d8442d5c93c2ffbf76e11fd76e9a1567ff3 Mon Sep 17 00:00:00 2001 From: Damien Martin-Guillerez Date: Wed, 24 Feb 2016 12:41:12 +0000 Subject: Enable access to file values in repository context ctx.path now works with labels. When given a label, ctx.path() will returns the path to the corresponding file if the label point to a file, or error out if it does not point to file. Also fix a small case with repository_ctx.symlink that wasn't creating its directories. This is needed to enable reading template files to create a template function (see http://goo.gl/fD4ZsY) for issue #893 (step 4 of http://goo.gl/OZV3o0). -- MOS_MIGRATED_REVID=115438400 --- .../skylark/SkylarkRepositoryContext.java | 113 +++++++++++++++++---- .../skylark/SkylarkRepositoryFunction.java | 36 ++++++- src/test/java/com/google/devtools/build/lib/BUILD | 1 + .../skylark/SkylarkRepositoryContextTest.java | 9 +- .../skylark/SkylarkRepositoryIntegrationTest.java | 28 ++++- 5 files changed, 166 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java index c48a964d82..45138da572 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContext.java @@ -17,10 +17,15 @@ package com.google.devtools.build.lib.bazel.repository.skylark; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.packages.AggregatingAttributeMapper; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException; +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.SkylarkCallable; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.syntax.ClassObject.SkylarkClassObject; @@ -30,7 +35,10 @@ import com.google.devtools.build.lib.syntax.SkylarkType; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.vfs.RootedPath; +import com.google.devtools.build.skyframe.SkyFunction.Environment; import com.google.devtools.build.skyframe.SkyFunctionException.Transience; +import com.google.devtools.build.skyframe.SkyKey; import java.io.File; import java.io.IOException; @@ -56,6 +64,7 @@ public class SkylarkRepositoryContext { private final Path outputDirectory; private final SkylarkClassObject attrObject; private final SkylarkOS osObject; + private final Environment env; /** * In native code, private values start with $. In Skylark, private values start with _, because @@ -71,9 +80,11 @@ public class SkylarkRepositoryContext { /** * Create a new context (ctx) object for a skylark repository rule ({@code rule} argument). */ - SkylarkRepositoryContext(Rule rule, Path outputDirectory, Map env) { + SkylarkRepositoryContext( + Rule rule, Path outputDirectory, Environment environment, Map env) { this.rule = rule; this.outputDirectory = outputDirectory; + this.env = environment; this.osObject = new SkylarkOS(env); AggregatingAttributeMapper attrs = AggregatingAttributeMapper.of(rule); ImmutableMap.Builder attrBuilder = new ImmutableMap.Builder<>(); @@ -106,15 +117,34 @@ public class SkylarkRepositoryContext { @SkylarkCallable( name = "path", doc = - "Returns a path from a string. If the path is relative, it will resolved relative " - + "to the output directory." + "Returns a path from a string or a label. If the path is relative, it will resolve " + + "relative to the output directory. If the path is a label, it will resolve to " + + "the path of the corresponding file. Note that remote repositories are executed " + + "during the analysis phase and thus cannot depends on a target result (the " + + "label should point to a non-generated file)." ) - public SkylarkPath path(String path) { - PathFragment pathFragment = new PathFragment(path); - if (pathFragment.isAbsolute()) { - return new SkylarkPath(outputDirectory.getFileSystem().getPath(path)); + public SkylarkPath path(Object path) throws EvalException { + return getPath("path()", path); + } + + private SkylarkPath getPath(String method, Object path) throws EvalException { + if (path instanceof String) { + PathFragment pathFragment = new PathFragment(path.toString()); + if (pathFragment.isAbsolute()) { + return new SkylarkPath(outputDirectory.getFileSystem().getPath(path.toString())); + } else { + return new SkylarkPath(outputDirectory.getRelative(pathFragment)); + } + } else if (path instanceof Label) { + SkylarkPath result = getPathFromLabel((Label) path); + if (result == null) { + SkylarkRepositoryFunction.restart(); + } + return result; + } else if (path instanceof SkylarkPath) { + return (SkylarkPath) path; } else { - return new SkylarkPath(outputDirectory.getRelative(pathFragment)); + throw new EvalException(Location.BUILTIN, method + " can only take a string or a label."); } } @@ -122,16 +152,20 @@ public class SkylarkRepositoryContext { name = "symlink", doc = "Create a symlink on the filesystem, the destination of the symlink should be in the " - + "output directory." + + "output directory. from can also be a label to a file." ) - public void symlink(SkylarkPath from, SkylarkPath to) throws RepositoryFunctionException { + public void symlink(Object from, Object to) throws RepositoryFunctionException, EvalException { + SkylarkPath fromPath = getPath("symlink()", from); + SkylarkPath toPath = getPath("symlink()", to); try { - checkInOutputDirectory(to); - to.path.createSymbolicLink(from.path); + checkInOutputDirectory(toPath); + makeDirectories(toPath.path); + toPath.path.createSymbolicLink(fromPath.path); } catch (IOException e) { throw new RepositoryFunctionException( new IOException( - "Could not create symlink from " + from + " to " + to + ": " + e.getMessage(), e), + "Could not create symlink from " + fromPath + " to " + toPath + ": " + e.getMessage(), + e), Transience.TRANSIENT); } } @@ -145,7 +179,7 @@ public class SkylarkRepositoryContext { } @SkylarkCallable(name = "file", documented = false) - public void createFile(SkylarkPath path) throws RepositoryFunctionException { + public void createFile(Object path) throws RepositoryFunctionException, EvalException { createFile(path, ""); } @@ -153,11 +187,13 @@ public class SkylarkRepositoryContext { name = "file", doc = "Generate a file in the output directory with the provided content" ) - public void createFile(SkylarkPath path, String content) throws RepositoryFunctionException { + public void createFile(Object path, String content) + throws RepositoryFunctionException, EvalException { + SkylarkPath p = getPath("file()", path); try { - checkInOutputDirectory(path); - makeDirectories(path.path); - try (OutputStream stream = path.path.getOutputStream()) { + checkInOutputDirectory(p); + makeDirectories(p.path); + try (OutputStream stream = p.path.getOutputStream()) { stream.write(content.getBytes(StandardCharsets.UTF_8)); } } catch (IOException e) { @@ -253,4 +289,45 @@ public class SkylarkRepositoryContext { public String toString() { return "repository_ctx[" + rule.getLabel() + "]"; } + + // Resolve the label given by value into a file path. + private SkylarkPath getPathFromLabel(Label label) throws EvalException { + // Look for package. + SkyKey pkgSkyKey = PackageLookupValue.key(label.getPackageIdentifier()); + PackageLookupValue pkgLookupValue = (PackageLookupValue) env.getValue(pkgSkyKey); + if (pkgLookupValue == null) { + return null; + } + 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(); + RootedPath rootedPath = RootedPath.toRootedPath(packageRoot, label.toPathFragment()); + SkyKey fileSkyKey = FileValue.key(rootedPath); + FileValue fileValue = null; + try { + fileValue = + (FileValue) + env.getValueOrThrow( + fileSkyKey, + IOException.class, + FileSymlinkException.class, + InconsistentFilesystemException.class); + } catch (IOException | FileSymlinkException | InconsistentFilesystemException e) { + throw new EvalException(Location.BUILTIN, new IOException(e)); + } + + if (fileValue == null) { + return null; + } + if (!fileValue.isFile()) { + throw new EvalException( + Location.BUILTIN, "Not a file: " + rootedPath.asPath().getPathString()); + } + + return new SkylarkPath(rootedPath.asPath()); + } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java index 9c9a777610..5f76bce594 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java @@ -17,6 +17,7 @@ package com.google.devtools.build.lib.bazel.repository.skylark; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.RuleDefinition; +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; @@ -26,6 +27,7 @@ import com.google.devtools.build.lib.syntax.BaseFunction; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.Mutability; import com.google.devtools.build.lib.syntax.Runtime; +import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.skyframe.SkyFunction.Environment; import com.google.devtools.build.skyframe.SkyFunctionException; @@ -42,6 +44,26 @@ import javax.annotation.Nullable; */ 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"); + } + } + + /** + * Skylark repository context functions can use this function to notify the + * SkylarkRepositoryFunction that a dependency was missing and the evaluation of the function must + * be restarted. + */ + static void restart() throws EvalException { + throw new SkylarkRepositoryMissingDependencyException(); + } + private CommandEnvironment commandEnvironment = null; public void setCommandEnvironment(CommandEnvironment commandEnvironment) { @@ -67,7 +89,8 @@ public class SkylarkRepositoryFunction extends RepositoryFunction { .setEventHandler(env.getListener()) .build(); SkylarkRepositoryContext skylarkRepositoryContext = - new SkylarkRepositoryContext(rule, outputDirectory, getClientEnvironment()); + new SkylarkRepositoryContext(rule, outputDirectory, env, getClientEnvironment()); + // This has side-effect, we don't care about the output. // Also we do a lot of stuff in there, maybe blocking operations and we should certainly make // it possible to return null and not block but it doesn't seem to be easy with Skylark @@ -88,6 +111,17 @@ public class SkylarkRepositoryFunction extends RepositoryFunction { Transience.PERSISTENT); } } catch (EvalException e) { + if (e.getCause() instanceof SkylarkRepositoryMissingDependencyException) { + // A dependency is missing, cleanup and returns null + try { + if (outputDirectory.exists()) { + FileSystemUtils.deleteTree(outputDirectory); + } + } catch (IOException e1) { + throw new RepositoryFunctionException(e1, Transience.TRANSIENT); + } + return null; + } throw new RepositoryFunctionException(e, Transience.TRANSIENT); } diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD index 2951ca5e3c..7d2a2d0637 100644 --- a/src/test/java/com/google/devtools/build/lib/BUILD +++ b/src/test/java/com/google/devtools/build/lib/BUILD @@ -1053,6 +1053,7 @@ java_test( "//third_party:guava-testlib", "//third_party:jsr305", "//third_party:junit4", + "//third_party:mockito", "//third_party:truth", ], ) diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java index 22b185e603..7d7a81444e 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryContextTest.java @@ -35,11 +35,13 @@ import com.google.devtools.build.lib.syntax.Runtime; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.testutil.Scratch; import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.skyframe.SkyFunction; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import org.mockito.Mockito; import java.io.IOException; import java.io.InputStreamReader; @@ -87,7 +89,12 @@ public class SkylarkRepositoryContextTest { .externalPackageData() .createAndAddRepositoryRule( packageBuilder, buildRuleClass(attributes), null, kwargs, ast); - context = new SkylarkRepositoryContext(rule, outputDirectory, ImmutableMap.of("FOO", "BAR")); + context = + new SkylarkRepositoryContext( + rule, + outputDirectory, + Mockito.mock(SkyFunction.Environment.class), + ImmutableMap.of("FOO", "BAR")); } protected void setUpContexForRule(String name) throws Exception { diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java index 86a2a485a5..aaca4d0edb 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java @@ -153,7 +153,7 @@ public class SkylarkRepositoryIntegrationTest extends BuildViewTestCase { scratch.file( "def.bzl", "def _impl(ctx):", - " ctx.symlink(ctx.path(ctx.attr.path), ctx.path(''))", + " ctx.symlink(ctx.attr.path, '')", "", "repo = repository_rule(", " implementation=_impl,", @@ -169,4 +169,30 @@ public class SkylarkRepositoryIntegrationTest extends BuildViewTestCase { Object path = target.getTarget().getAssociatedRule().getAttributeContainer().getAttr("path"); assertThat(path).isEqualTo("foo"); } + + @Test + public void testSkylarkSymlinkFileFromRepository() throws Exception { + scratch.file("/repo2/bar.txt", "filegroup(name='bar', srcs=['foo.txt'], path='foo')"); + scratch.file("/repo2/BUILD"); + scratch.file("/repo2/WORKSPACE"); + scratch.file( + "def.bzl", + "def _impl(ctx):", + " ctx.symlink(Label('@repo2//:bar.txt'), 'BUILD')", + " ctx.file('foo.txt', 'foo')", + "", + "repo = repository_rule(", + " implementation=_impl,", + " local=True)"); + scratch.file(rootDirectory.getRelative("BUILD").getPathString()); + scratch.overwriteFile( + rootDirectory.getRelative("WORKSPACE").getPathString(), + "local_repository(name='repo2', path='/repo2')", + "load('//:def.bzl', 'repo')", + "repo(name='foo')"); + invalidatePackages(); + ConfiguredTarget target = getConfiguredTarget("@foo//:bar"); + Object path = target.getTarget().getAssociatedRule().getAttributeContainer().getAttr("path"); + assertThat(path).isEqualTo("foo"); + } } -- cgit v1.2.3