Base stamping decisions on --stamp configuration, not attributes on t…

…he rule This follows the example of rules_go and rules_nodejs. It means we don't produce cache-busting outputs unless the user explicitly requests non-deteriminism in the build with --stamp. BREAKING CHANGE: - To get stamped outputs, users must now call bazel with `--stamp` - the stamp attribute was removed from some providers - the stamp attribute was deprecated on container_push and container_bundle, it is now removed - the stamp attribute was removed from container_image, there is no replacement Fixes bazelbuild#1451
alexeagle · Jun 4, 2021 · 6dc5906 · 6dc5906
1 parent e005f17
commit 6dc5906
Show file tree

Hide file tree

Showing 9 changed files with 82 additions and 131 deletions.
diff --git a/README.md b/README.md
@@ -256,8 +256,15 @@ at present for doing this.
 
 ### Stamping
 
-The first option is to use stamping. Stamping is enabled when a supported
-attribute contains a python format placeholder (e.g. `{BUILD_USER}`).
+The first option is to use stamping.
+Stamping is enabled when a bazel is run with `--stamp`.
+This enables replacements in stamp-aware attributes, where a python format placeholder
+(e.g. `{BUILD_USER}`)
+is replaced by the corresponding key from the `bazel-out/stable-status.txt` or
+`bazel-out/volatile-status.txt` file.
+(Note that changes to the stable-status file
+cause a rebuild of the action, while volatile-status does not.
+)
 
 ```python
 # A common pattern when users want to avoid trampling
@@ -293,7 +300,9 @@ BUILD_TIMESTAMP 1498740967769
 ```
 
 You can augment these variables via `--workspace_status_command`,
-including through the use of [`.bazelrc`](https://github.com/kubernetes/kubernetes/blob/81ce94ae1d8f5d04058eeb214e9af498afe78ff2/build/root/.bazelrc#L6).
+see the [bazel docs](https://docs.bazel.build/versions/master/user-manual.html#workspace_status).
+
+That flag is typically passed in the `.bazelrc` file, see for example [`.bazelrc` in kubernetes](https://github.com/kubernetes/kubernetes/blob/81ce94ae1d8f5d04058eeb214e9af498afe78ff2/build/root/.bazelrc#L6).
 
 
 ### Make variables
@@ -1546,20 +1555,6 @@ configuration. See [here](#container_push-custom-client-configuration) for detai
 	<p>This field supports stamp variables.</p>
       </td>
     </tr>
-    <tr>
-      <td><code>stamp</code></td>
-      <td>
-        <p><code>Bool; optional</code></p>
-        <p>Deprecated: it is now automatically inferred.</p>
-        <p>If true, enable use of workspace status variables
-        (e.g. <code>BUILD_USER</code>, <code>BUILD_EMBED_LABEL</code>,
-        and custom values set using <code>--workspace_status_command</code>)
-        in tags.</p>
-        <p>These fields are specified in the tag using Python format
-        syntax, e.g.
-        <code>example.org/{BUILD_USER}/image:{BUILD_EMBED_LABEL}</code>.</p>
-      </td>
-    </tr>
   </tbody>
 </table>
 
@@ -1794,26 +1789,12 @@ A rule that aliases and saves N images into a single `docker save` tarball.
         <p>A collection of the images to save into the tarball.</p>
         <p>The keys are the tags with which to alias the image specified by the
            value. These tags may contain make variables (<code>$FOO</code>),
-           and if <code>stamp</code> is set to true, may also contain workspace
+           and may also contain workspace
            status variables (<code>{BAR}</code>).</p>
         <p>The values may be the output of <code>container_pull</code>,
            <code>container_image</code>, or a <code>docker save</code> tarball.</p>
       </td>
     </tr>
-    <tr>Toolchains
-      <td><code>stamp</code></td>
-      <td>
-        <p><code>Bool; optional</code></p>
-        <p>Deprecated: it is now automatically inferred.</p>
-        <p>If true, enable use of workspace status variables
-        (e.g. <code>BUILD_USER</code>, <code>BUILD_EMBED_LABEL</code>,
-        and custom values set using <code>--workspace_status_command</code>)
-        in tags.</p>
-        <p>These fields are specified in the tag using Python format
-        syntax, e.g.
-        <code>example.org/{BUILD_USER}/image:{BUILD_EMBED_LABEL}</code>.</p>
-      </td>
-    </tr>
   </tbody>
 </table>
 

diff --git a/container/BUILD b/container/BUILD
@@ -73,9 +73,6 @@ TEST_TARGETS = [
     ":with_unix_epoch_creation_time",
     ":with_millisecond_unix_epoch_creation_time",
     ":with_rfc_3339_creation_time",
-    ":with_stamped_creation_time",
-    ":with_default_stamped_creation_time",
-    ":with_base_stamped_image",
     ":with_env",
     ":layers_with_env",
     ":with_double_env",
@@ -100,7 +97,6 @@ TEST_TARGETS = [
     # TODO(mattmoor): Re-enable once archive is visible
     # "extras_with_deb",
     ":bundle_test",
-    ":stamped_bundle_test",
     ":pause_based",
     ":dashdash_entrypoint",
     ":base_based_warning",
@@ -126,7 +122,6 @@ TEST_DATA = [
 ] + [
     "//testdata:push_compression_gzip_fast",
     "//testdata:push_compression_gzip_normal",
-    "//testdata:stamped_bundle_test",
     "//testdata:stamp_info_file.txt",
     "//tests/container:basic_windows_image.tar",
 ]

diff --git a/container/bundle.bzl b/container/bundle.bzl
@@ -14,7 +14,7 @@
 """Rule for bundling Container images into a tarball."""
 
 load("@bazel_skylib//lib:dicts.bzl", "dicts")
-load("@io_bazel_rules_docker//container:providers.bzl", "BundleInfo")
+load("@io_bazel_rules_docker//container:providers.bzl", "BUILD_CONTEXT_ATTRS", "BuildContextInfo", "BundleInfo")
 load(
     "//container:layer_tools.bzl",
     _assemble_image = "assemble",
@@ -38,18 +38,11 @@ def _container_bundle_impl(ctx):
 
     images = {}
     runfiles = []
-    if ctx.attr.stamp:
-        print("Attr 'stamp' is deprecated; it is now automatically inferred. Please remove it from %s" % ctx.label)
-    stamp = False
+    stamp = ctx.attr.build_context_data[BuildContextInfo].stamp
     for unresolved_tag in ctx.attr.images:
         # Allow users to put make variables into the tag name.
         tag = ctx.expand_make_variables("images", unresolved_tag, {})
 
-        # If any tag contains python format syntax (which is how users
-        # configure stamping), we enable stamping.
-        if "{" in tag:
-            stamp = True
-
         target = ctx.attr.images[unresolved_tag]
 
         layer = _get_layers(ctx, ctx.label.name, image_target_dict[target])
@@ -96,15 +89,11 @@ def _container_bundle_impl(ctx):
     ]
 
 container_bundle_ = rule(
-    attrs = dicts.add({
+    attrs = dicts.add(BUILD_CONTEXT_ATTRS, {
         "image_target_strings": attr.string_list(),
         # Implicit dependencies.
         "image_targets": attr.label_list(allow_files = True),
         "images": attr.string_dict(),
-        "stamp": attr.bool(
-            default = False,
-            mandatory = False,
-        ),
         "tar_output": attr.output(),
     }, _layer_tools),
     executable = True,

diff --git a/container/image.bzl b/container/image.bzl
@@ -26,6 +26,8 @@ load(
 )
 load(
     "@io_bazel_rules_docker//container:providers.bzl",
+    "BUILD_CONTEXT_ATTRS",
+    "BuildContextInfo",
     "ImageInfo",
     "LayerInfo",
 )
@@ -105,14 +107,7 @@ def _add_create_image_config_args(
     args.add_all(ctx.attr.ports, before_each = "-ports")
     args.add_all(ctx.attr.volumes, before_each = "-volumes")
 
-    stamp = None
-
-    # If base image is having enabled stamping then it is propagated
-    # to child images.
-    if ctx.attr.stamp == True:
-        stamp = ctx.attr.stamp
-    elif ctx.attr.base and ImageInfo in ctx.attr.base:
-        stamp = ctx.attr.base[ImageInfo].stamp
+    stamp = ctx.attr.build_context_data[BuildContextInfo].stamp
 
     if creation_time:
         args.add("-creationTime", creation_time)
@@ -540,31 +535,11 @@ def _impl(
                 ([container_parts["legacy"]] if container_parts["legacy"] else []),
     )
 
-    # Stamp attribute needs to be propagated between definitions to enhance actions
-    # with ability to determine properly whether root image has activated stamping.
-    #
-    # This covers the following example case:
-    # container_image(
-    #     name = “base_image”,
-    #     base = “@base//image”,
-    #     stamp = True,
-    # )
-    #
-    # lang_image(
-    #     base = “:base_image”,
-    # )
-    stamp = None
-    if ctx.attr.stamp:
-        stamp = ctx.attr.stamp
-    elif ctx.attr.base and ImageInfo in ctx.attr.base:
-        stamp = ctx.attr.base[ImageInfo].stamp
-
     return [
         ImageInfo(
             container_parts = container_parts,
             legacy_run_behavior = ctx.attr.legacy_run_behavior,
             docker_run_flags = docker_run_flags,
-            stamp = stamp,
         ),
         DefaultInfo(
             executable = build_executable,
@@ -577,7 +552,7 @@ def _impl(
         ),
     ]
 
-_attrs = dicts.add(_layer.attrs, {
+_attrs = dicts.add(BUILD_CONTEXT_ATTRS, _layer.attrs, {
     "architecture": attr.string(
         doc = "The desired CPU architecture to be used as label in the container image.",
         default = "amd64",
@@ -745,16 +720,6 @@ _attrs = dicts.add(_layer.attrs, {
         Setting this attribute to `gcr.io/dummy` would set the default tag to
         `gcr.io/dummy/package_name:target`.""",
     ),
-    "stamp": attr.bool(
-        default = False,
-        doc = """If true, enable use of workspace status variables
-        (e.g. `BUILD_USER`, `BUILD_EMBED_LABEL`,
-        and custom values set using `--workspace_status_command`)
-        in tags.
-
-        These fields are specified in attributes using Python format
-        syntax, e.g. `foo{BUILD_USER}bar`.""",
-    ),
     "user": attr.string(
         doc = """The user that the image should run as.
 

diff --git a/container/providers.bzl b/container/providers.bzl
@@ -27,7 +27,6 @@ ImageInfo = provider(fields = [
     "container_parts",
     "legacy_run_behavior",
     "docker_run_flags",
-    "stamp",
 ])
 
 # A provider containing information exposed by container_import rules
@@ -54,7 +53,6 @@ PushInfo = provider(fields = [
     "registry",
     "repository",
     "tag",
-    "stamp",
     "stamp_inputs",
     "digest",
 ])
@@ -73,3 +71,25 @@ FilterAspectInfo = provider(
         "depset": "a depset of struct(target=<target>, target_deps=<depset>)",
     },
 )
+
+#Modelled after _GoContextData in rules_go/go/private/context.bzl
+BuildContextInfo = provider(
+    doc = "Provides data about the build context, like config_setting's",
+    fields = {
+        "stamp": "If stamping is enabled",
+    },
+)
+
+BUILD_CONTEXT_ATTRS = {
+    "build_context_data": attr.label(
+        default = "@io_bazel_rules_docker//docker:build_context_data",
+        providers = [BuildContextInfo],
+        doc = """Provides info about the build context, such as stamping.
+        
+By default it reads from the bazel command line, such as the `--stamp` argument.
+Use this to override values for this target, such as enabling or disabling stamping.
+You can use the `build_context_data` rule
+to create a BuildContextInfo and override the --stamp config setting.
+""",
+    ),
+}
diff --git a/container/push.bzl b/container/push.bzl
@@ -17,7 +17,7 @@ Bazel rule for publishing images.
 """
 
 load("@bazel_skylib//lib:dicts.bzl", "dicts")
-load("@io_bazel_rules_docker//container:providers.bzl", "PushInfo")
+load("@io_bazel_rules_docker//container:providers.bzl", "BUILD_CONTEXT_ATTRS", "BuildContextInfo", "PushInfo")
 load(
     "//container:layer_tools.bzl",
     _gen_img_args = "generate_args_for_image",
@@ -61,11 +61,7 @@ def _impl(ctx):
         tag = "$(cat {})".format(_get_runfile_path(ctx, ctx.file.tag_file))
         pusher_input.append(ctx.file.tag_file)
 
-    # If any stampable attr contains python format syntax (which is how users
-    # configure stamping), we enable stamping.
-    if ctx.attr.stamp:
-        print("Attr 'stamp' is deprecated; it is now automatically inferred. Please remove it from %s" % ctx.label)
-    stamp = "{" in tag or "{" in registry or "{" in repository
+    stamp = ctx.attr.build_context_data[BuildContextInfo].stamp
     stamp_inputs = [ctx.info_file, ctx.version_file] if stamp else []
     for f in stamp_inputs:
         pusher_args += ["-stamp-info-file", "%s" % _get_runfile_path(ctx, f)]
@@ -139,14 +135,13 @@ def _impl(ctx):
             registry = registry,
             repository = repository,
             tag = tag,
-            stamp = stamp,
             stamp_inputs = stamp_inputs,
             digest = ctx.outputs.digest,
         ),
     ]
 
 _container_push = rule(
-    attrs = dicts.add({
+    attrs = dicts.add(BUILD_CONTEXT_ATTRS, {
         "extension": attr.string(
             doc = "(optional) The file extension for the push script.",
         ),
@@ -179,10 +174,6 @@ _container_push = rule(
             default = False,
             doc = "Only push images if the digest has changed, default to False",
         ),
-        "stamp": attr.bool(
-            default = False,
-            mandatory = False,
-        ),
         "tag": attr.string(
             default = "latest",
             doc = "(optional) The tag of the image, default to 'latest'.",

diff --git a/docker/BUILD b/docker/BUILD
@@ -13,6 +13,7 @@
 # limitations under the License.
 
 load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
+load(":context.bzl", "build_context_data")
 
 package(default_visibility = ["//visibility:public"])
 
@@ -26,3 +27,19 @@ bzl_library(
         "@io_bazel_rules_docker//toolchains/docker",
     ],
 )
+
+# Detect if the build is running under --stamp
+config_setting(
+    name = "stamp",
+    values = {"stamp": "true"},
+)
+
+# Modelled after go_context_data from rules_go
+# passes config values to starlark via a provider
+build_context_data(
+    name = "build_context_data",
+    stamp = select({
+        "@io_bazel_rules_docker//docker:stamp": True,
+        "//conditions:default": False,
+    }),
+)
diff --git a/docker/context.bzl b/docker/context.bzl
@@ -0,0 +1,19 @@
+"build_context_data rule"
+
+load("@io_bazel_rules_docker//container:providers.bzl", "BuildContextInfo")
+
+_DOC = """build_context_data gathers information about the build configuration.
+It is a common dependency of all targets that are sensitive to configuration."""
+
+def _impl(ctx):
+    return [BuildContextInfo(stamp = ctx.attr.stamp)]
+
+# Modelled after go_context_data in rules_go
+# Works around github.com/bazelbuild/bazel/issues/1054
+build_context_data = rule(
+    implementation = _impl,
+    attrs = {
+        "stamp": attr.bool(mandatory = True),
+    },
+    doc = _DOC,
+)