From 8a4ab37560a197e9401fb561bdf9aa0c9cd61269 Mon Sep 17 00:00:00 2001 From: Siddhartha Bagaria Date: Wed, 19 Feb 2020 23:41:44 -0800 Subject: [PATCH] rules_r: stamp attribute for r_binary and r_test This stamp attribute will determine whether we depend on the stable status file in the runfiles of the binary/test. The volatile status file is always available. This will help people get better cache hits across builds when their workspace status command is outputting STABLE_ vars but an individual test does not really need these vars. NOTE: This is a breaking change for people who were depending on the stable status file in their r_test or r_markdown target. This file will not be available by default because the attribute value is False by default. Volatile status files will continue to be available. See #37 for more context. We expect the usage of this attribute to be limited, and we expect cache hits in stamping to be governed mostly through the management of the content of stable status file. The logic extends to r_markdown. Co-authored-by: forestfang-stripe <45770568+forestfang-stripe@users.noreply.github.com> --- R/internal/binary.bzl | 9 ++++++++- README.md | 12 ++++++++++-- tests/.bazelrc | 1 + tests/stamping/BUILD | 26 ++++++++++++++++++++++++++ tests/stamping/nostamp_test.sh | 29 +++++++++++++++++++++++++++++ tests/stamping/stamp_test.sh | 30 ++++++++++++++++++++++++++++++ tests/stamping/workspace_status.sh | 17 +++++++++++++++++ 7 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 tests/.bazelrc create mode 100644 tests/stamping/BUILD create mode 100755 tests/stamping/nostamp_test.sh create mode 100755 tests/stamping/stamp_test.sh create mode 100755 tests/stamping/workspace_status.sh diff --git a/R/internal/binary.bzl b/R/internal/binary.bzl index a887e8b..9ea2c35 100644 --- a/R/internal/binary.bzl +++ b/R/internal/binary.bzl @@ -103,7 +103,9 @@ def _r_binary_impl(ctx): ) layered_lib_files = _layer_library_deps(ctx, library_deps) - stamp_files = [ctx.info_file, ctx.version_file] + stamp_files = [ctx.version_file] + if ctx.attr.stamp: + stamp_files.append(ctx.info_file) runfiles = ctx.runfiles( files = library_deps.lib_dirs + stamp_files, transitive_files = depset(transitive = [srcs, exe, tools]), @@ -159,6 +161,11 @@ _R_BINARY_ATTRS = { "script_args": attr.string_list( doc = "A list of arguments to pass to the src script", ), + "stamp": attr.bool( + doc = ("Include the stable status file in the runfiles of the binary. " + + "The volatile status file is always included."), + default = False, + ), "_binary_sh_tpl": attr.label( allow_single_file = True, default = "@com_grail_rules_r//R/scripts:binary.sh.tpl", diff --git a/README.md b/README.md index e46ed38..d94da2f 100644 --- a/README.md +++ b/README.md @@ -608,6 +608,14 @@ the runfiles of the root executable.

A list of arguments to pass to the src script.

+ + stamp + +

Bool; default False

+

Include the stable status file in the runfiles of the binary. + The volatile status file is always included.

+ + @@ -615,7 +623,7 @@ the runfiles of the root executable. ## r_test ```python -r_test(name, src, deps, data, env_vars, tools, rscript_args, script_args) +r_test(name, src, deps, data, env_vars, tools, rscript_args, script_args, stamp) ``` This is identical to [r_binary](#r_binary) but is run as a test. @@ -624,7 +632,7 @@ This is identical to [r_binary](#r_binary) but is run as a test. ## r_markdown ```python -r_markdown(name, src, deps, data, env_vars, tools, rscript_args, script_args, +r_markdown(name, src, deps, data, env_vars, tools, rscript_args, script_args, stamp, render_function="rmarkdown::render", input_argument="input", output_dir_argument="output_dir", render_args) ``` diff --git a/tests/.bazelrc b/tests/.bazelrc new file mode 100644 index 0000000..5853bc3 --- /dev/null +++ b/tests/.bazelrc @@ -0,0 +1 @@ +build --workspace_status_command=stamping/workspace_status.sh diff --git a/tests/stamping/BUILD b/tests/stamping/BUILD new file mode 100644 index 0000000..50c164a --- /dev/null +++ b/tests/stamping/BUILD @@ -0,0 +1,26 @@ +# Copyright 2020 The Bazel Authors. +# +# 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. + +load("@com_grail_rules_r//R:defs.bzl", "r_test") + +r_test( + name = "stamp_test", + src = "stamp_test.sh", + stamp = True, +) + +r_test( + name = "nostamp_test", + src = "nostamp_test.sh", +) diff --git a/tests/stamping/nostamp_test.sh b/tests/stamping/nostamp_test.sh new file mode 100755 index 0000000..6344d27 --- /dev/null +++ b/tests/stamping/nostamp_test.sh @@ -0,0 +1,29 @@ +#!/bin/bash +# Copyright 2020 The Bazel Authors. +# +# 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. + +set -euo pipefail + +fail() { + echo "$@" + exit 1 +} + +if ! grep "^VAR" "volatile-status.txt"; then + fail "volatile status file expected" +fi + +if [[ -e stable-info.txt ]]; then + fail "stable status file not expected" +fi diff --git a/tests/stamping/stamp_test.sh b/tests/stamping/stamp_test.sh new file mode 100755 index 0000000..581b8b9 --- /dev/null +++ b/tests/stamping/stamp_test.sh @@ -0,0 +1,30 @@ +#!/bin/bash +# Copyright 2020 The Bazel Authors. +# +# 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. + + +set -euo pipefail + +fail() { + echo "$@" + exit 1 +} + +if ! grep "^VAR" "volatile-status.txt"; then + fail "volatile status file expected" +fi + +if ! grep "^STABLE_VAR" "stable-status.txt"; then + fail "stable status file expected and not empty" +fi diff --git a/tests/stamping/workspace_status.sh b/tests/stamping/workspace_status.sh new file mode 100755 index 0000000..6753e7b --- /dev/null +++ b/tests/stamping/workspace_status.sh @@ -0,0 +1,17 @@ +#!/bin/bash +# Copyright 2020 The Bazel Authors. +# +# 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. + +echo "VAR foo" +echo "STABLE_VAR bar"