From d2e307cdf6513130fbf4005f6e97703b7e7fe1d7 Mon Sep 17 00:00:00 2001 From: Damien Martin-Guillerez Date: Wed, 7 Sep 2016 11:31:43 +0000 Subject: Use named parameters for function in SkylarkRepositoryContext when it make sense RELNOTES[INC]: repository_ctx.{download,download_and_extract,execute} API now use named parameters for optional parameters and no longer uses argument type to distinguished between arguments (executable attribute name must be specified when preceding optional arguments are missing). Fixes #1426. -- Change-Id: I41d174cd9fd5c47edf4e2e9a86ce7f6c7882a104 Reviewed-on: https://bazel-review.googlesource.com/#/c/5673 MOS_MIGRATED_REVID=132421955 --- .../skylark/SkylarkRepositoryContext.java | 331 +++++++++++++-------- 1 file changed, 211 insertions(+), 120 deletions(-) (limited to 'src/main') 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 bfe4aec0a0..eca0e2434c 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 @@ -32,11 +32,15 @@ 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; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.Runtime; +import com.google.devtools.build.lib.syntax.SkylarkDict; +import com.google.devtools.build.lib.syntax.SkylarkList; import com.google.devtools.build.lib.syntax.SkylarkType; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.StringUtilities; @@ -51,7 +55,6 @@ import java.io.File; import java.io.IOException; import java.io.OutputStream; import java.nio.charset.StandardCharsets; -import java.util.List; import java.util.Map; /** Skylark API for the repository_rule's context. */ @@ -137,7 +140,7 @@ public class SkylarkRepositoryContext { name = "path", doc = "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 " + + "relative to the repository 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)." @@ -168,9 +171,27 @@ public class SkylarkRepositoryContext { @SkylarkCallable( name = "symlink", - doc = - "Create a symlink on the filesystem, the destination of the symlink should be in the " - + "output directory. from can also be a label to a file." + doc = "Create a symlink on the filesystem.", + parameters = { + @Param( + name = "from", + allowedTypes = { + @ParamType(type = String.class), + @ParamType(type = Label.class), + @ParamType(type = SkylarkPath.class) + }, + doc = "path to which the created symlink should point to." + ), + @Param( + name = "to", + allowedTypes = { + @ParamType(type = String.class), + @ParamType(type = Label.class), + @ParamType(type = SkylarkPath.class) + }, + doc = "path of the symlink to create, relative to the repository directory." + ), + } ) public void symlink(Object from, Object to) throws RepositoryFunctionException, EvalException, InterruptedException { @@ -194,28 +215,39 @@ public class SkylarkRepositoryContext { throw new RepositoryFunctionException( new EvalException( Location.fromFile(path.getPath()), - "Cannot write outside of the output directory for path " + path), + "Cannot write outside of the repository directory for path " + path), Transience.PERSISTENT); } } - @SkylarkCallable(name = "file", documented = false) - public void createFile(Object path) - throws RepositoryFunctionException, EvalException, InterruptedException { - createFile(path, ""); - } - - @SkylarkCallable(name = "file", documented = false) - public void createFile(Object path, String content) - throws RepositoryFunctionException, EvalException, InterruptedException { - createFile(path, content, true); - } - @SkylarkCallable( name = "file", - doc = - "Generate a file in the output directory with the provided content. An optional third " - + "argument set the executable bit to on or off (default to True)." + doc = "Generate a file in the repository directory with the provided content.", + parameters = { + @Param( + name = "path", + allowedTypes = { + @ParamType(type = String.class), + @ParamType(type = Label.class), + @ParamType(type = SkylarkPath.class) + }, + doc = "path of the file to create, relative to the repository directory." + ), + @Param( + name = "content", + type = String.class, + named = true, + defaultValue = "''", + doc = "the content of the file to create, empty by default." + ), + @Param( + name = "executable", + named = true, + type = Boolean.class, + defaultValue = "True", + doc = "set the executable flag on the created file, true by default." + ), + } ) public void createFile(Object path, String content, Boolean executable) throws RepositoryFunctionException, EvalException, InterruptedException { @@ -234,13 +266,6 @@ public class SkylarkRepositoryContext { } } - @SkylarkCallable(name = "template", documented = false) - public void createFileFromTemplate( - Object path, Object template, Map substitutions) - throws RepositoryFunctionException, EvalException, InterruptedException { - createFileFromTemplate(path, template, substitutions, true); - } - @SkylarkCallable( name = "template", doc = @@ -248,10 +273,44 @@ public class SkylarkRepositoryContext { + "template of a key of substitutions will be replaced by " + "the corresponding value. The result is written in path. An optional" + "executable argument (default to true) can be set to turn on or off" - + "the executable bit." + + "the executable bit.", + parameters = { + @Param( + name = "path", + allowedTypes = { + @ParamType(type = String.class), + @ParamType(type = Label.class), + @ParamType(type = SkylarkPath.class) + }, + doc = "path of the file to create, relative to the repository directory." + ), + @Param( + name = "template", + allowedTypes = { + @ParamType(type = String.class), + @ParamType(type = Label.class), + @ParamType(type = SkylarkPath.class) + }, + doc = "path to the template file." + ), + @Param( + name = "substitutions", + type = SkylarkDict.class, + defaultValue = "{}", + named = true, + doc = "substitutions to make when expanding the template." + ), + @Param( + name = "executable", + type = Boolean.class, + defaultValue = "True", + named = true, + doc = "set the executable flag on the created file, true by default." + ), + } ) public void createFileFromTemplate( - Object path, Object template, Map substitutions, Boolean executable) + Object path, Object template, SkylarkDict substitutions, Boolean executable) throws RepositoryFunctionException, EvalException, InterruptedException { SkylarkPath p = getPath("template()", path); SkylarkPath t = getPath("template()", template); @@ -312,37 +371,39 @@ public class SkylarkRepositoryContext { + " is limited by timeout (in seconds, default 600 seconds). This method" + " returns an exec_result structure containing the output of the" + " command. The environment map can be used to override some environment" - + " variables to be passed to the process." + + " variables to be passed to the process.", + parameters = { + @Param( + name = "arguments", + type = SkylarkList.class, + doc = + "List of arguments, the first element should be the path to the program to " + + "execute." + ), + @Param( + name = "timeout", + type = Integer.class, + named = true, + defaultValue = "600", + doc = "maximum duration of the command in seconds (default is 600 seconds)." + ), + @Param( + name = "environment", + type = SkylarkDict.class, + defaultValue = "{}", + named = true, + doc = "force some environment variables to be set to be passed to the process." + ), + } ) - public SkylarkExecutionResult execute(List arguments, Integer timeout, - Map environment) throws EvalException, RepositoryFunctionException { - createDirectory(outputDirectory); - return SkylarkExecutionResult.builder(osObject.getEnvironmentVariables()) - .addArguments(arguments) - .setDirectory(outputDirectory.getPathFile()) - .addEnvironmentVariables(environment) - .setTimeout(timeout.longValue() * 1000) - .execute(); - } - - @SkylarkCallable(name = "execute", documented = false) - public SkylarkExecutionResult execute(List arguments) + public SkylarkExecutionResult execute( + SkylarkList arguments, Integer timeout, SkylarkDict environment) throws EvalException, RepositoryFunctionException { createDirectory(outputDirectory); return SkylarkExecutionResult.builder(osObject.getEnvironmentVariables()) - .setDirectory(outputDirectory.getPathFile()) .addArguments(arguments) - .setTimeout(600000) - .execute(); - } - - @SkylarkCallable(name = "execute", documented = false) - public SkylarkExecutionResult execute(List arguments, Integer timeout) - throws EvalException, RepositoryFunctionException { - createDirectory(outputDirectory); - return SkylarkExecutionResult.builder(osObject.getEnvironmentVariables()) .setDirectory(outputDirectory.getPathFile()) - .addArguments(arguments) + .addEnvironmentVariables(environment) .setTimeout(timeout.longValue() * 1000) .execute(); } @@ -380,20 +441,44 @@ public class SkylarkRepositoryContext { @SkylarkCallable( name = "download", - doc = - "Download a file to the output path for the provided url." - + "\nParameters:" - + "\nurl: a URL referencing an archive file containing a Bazel repository." - + " Archives of type .zip, .jar, .war, .tar.gz or .tgz are supported." - + " There is no support for authentication. Redirections are followed." - + "\noutput: " - + "(optional) sha256: the expected SHA-256 hash of the file downloaded." - + " This must match the SHA-256 hash of the file downloaded. It is a security risk to" - + " omit the SHA-256 as remote files can change. At best omitting this field will make" - + " your build non-hermetic. It is optional to make development easier but should" - + " be set before shipping." - + "\nexecutable: (optional) set the executable bit to on or off " - + "for downloaded file(default to False)." + doc = "Download a file to the output path for the provided url.", + parameters = { + @Param( + name = "url", + type = String.class, + doc = + "URL to the file to download. There is no authentication." + + " Redirection are followed." + ), + @Param( + name = "output", + allowedTypes = { + @ParamType(type = String.class), + @ParamType(type = Label.class), + @ParamType(type = SkylarkPath.class) + }, + doc = "path to the output file, relative to the repository directory." + ), + @Param( + name = "sha256", + type = String.class, + defaultValue = "''", + named = true, + doc = + "the expected SHA-256 hash of the file downloaded." + + " This must match the SHA-256 hash of the file downloaded. It is a security risk" + + " to omit the SHA-256 as remote files can change. At best omitting this field" + + " will make your build non-hermetic. It is optional to make development easier" + + " but should be set before shipping." + ), + @Param( + name = "executable", + type = Boolean.class, + defaultValue = "False", + named = true, + doc = "set the executable flag on the created file, false by default." + ), + } ) public void download(String url, Object output, String sha256, Boolean executable) throws RepositoryFunctionException, EvalException, InterruptedException { @@ -411,47 +496,65 @@ public class SkylarkRepositoryContext { } } - @SkylarkCallable(name = "download", documented = false) - public void download(String url, Object output, String sha256) - throws RepositoryFunctionException, EvalException, InterruptedException { - download(url, output, sha256, false); - } - - @SkylarkCallable(name = "download", documented = false) - public void download(String url, Object output, Boolean executable) - throws RepositoryFunctionException, EvalException, InterruptedException { - download(url, output, "", executable); - } - - @SkylarkCallable(name = "download", documented = false) - public void download(String url, Object output) - throws RepositoryFunctionException, EvalException, InterruptedException { - download(url, output, "", false); - } - @SkylarkCallable( name = "download_and_extract", - doc = - "Download a file to the output path for the provided url, and extract it." - + "\nParameters:" - + "\nurl: a URL referencing an archive file containing a Bazel repository." - + " Archives of type .zip, .jar, .war, .tar.gz or .tgz are supported." - + " There is no support for authentication. Redirections are followed." - + "\noutput: " - + "\n(optional) sha256: the expected SHA-256 hash of the file downloaded." - + " This must match the SHA-256 hash of the file downloaded. It is a security risk to" - + " omit the SHA-256 as remote files can change. At best omitting this field will make" - + " your build non-hermetic. It is optional to make development easier but should" - + " be set before shipping." - + "\n(optional) type: The archive type of the downloaded file." - + " By default, the archive type is determined from the file extension of the URL." - + " If the file has no extension, you can explicitly specify either" - + "\"zip\", \"jar\", \"tar.gz\", or \"tgz\" here." - + "(optional) stripPrefix: a directory prefix to strip from the extracted files." - + "\nMany archives contain a top-level directory that contains alfiles in" - + " archive. Instead of needing to specify this prefix over and over in the" - + " build_file, this field can be used to strip it extracted" - + " files." + doc = "Download a file to the output path for the provided url, and extract it.", + parameters = { + @Param( + name = "url", + type = String.class, + doc = + "a URL referencing an archive file containing a Bazel repository." + + " Archives of type .zip, .jar, .war, .tar.gz or .tgz are supported." + + " There is no support for authentication. Redirections are followed." + ), + @Param( + name = "output", + allowedTypes = { + @ParamType(type = String.class), + @ParamType(type = Label.class), + @ParamType(type = SkylarkPath.class) + }, + doc = + "path to the directory where the archive will be unpacked," + + " relative to the repository directory." + ), + @Param( + name = "sha256", + type = String.class, + defaultValue = "''", + named = true, + doc = + "the expected SHA-256 hash of the file downloaded." + + " This must match the SHA-256 hash of the file downloaded. It is a security risk" + + " to omit the SHA-256 as remote files can change. At best omitting this field" + + " will make your build non-hermetic. It is optional to make development easier" + + " but should be set before shipping." + ), + @Param( + name = "type", + type = String.class, + defaultValue = "''", + named = true, + doc = + "the archive type of the downloaded file." + + " By default, the archive type is determined from the file extension of the URL." + + " If the file has no extension, you can explicitly specify either" + + "\"zip\", \"jar\", \"tar.gz\", or \"tgz\" here." + ), + @Param( + name = "stripPrefix", + type = String.class, + defaultValue = "''", + named = true, + doc = + "a directory prefix to strip from the extracted files." + + "\nMany archives contain a top-level directory that contains alfiles in" + + " archive. Instead of needing to specify this prefix over and over in the" + + " build_file, this field can be used to strip it extracted" + + " files." + ), + } ) public void downloadAndExtract( String url, Object output, String sha256, String type, String stripPrefix) @@ -483,18 +586,6 @@ public class SkylarkRepositoryContext { } } - @SkylarkCallable(name = "download_and_extract", documented = false) - public void downloadAndExtract(String url, Object output, String type) - throws RepositoryFunctionException, InterruptedException, EvalException { - downloadAndExtract(url, output, "", "", type); - } - - @SkylarkCallable(name = "download_and_extract", documented = false) - public void downloadAndExtract(String url, Object output) - throws RepositoryFunctionException, InterruptedException, EvalException { - downloadAndExtract(url, output, "", "", ""); - } - // This is just for test to overwrite the path environment private static ImmutableList pathEnv = null; -- cgit v1.2.3