Allow Starlark rules to observe the --stamp setting

[alanfalloon]

Description of the feature request:

As noted in a comment on #1054, Starlark rules can access volatile-status.txt as ctx.version_file and stable-status.txt as ctx.info_file, but there is no way for a Starlark rule to observe the --stamp flag setting to know if the rule should access the files or not.

What underlying problem are you trying to solve with this feature?

I want to write Starlark rules that do not access the status files unless the user specifies the --stamp setting.

Because of issues like #10075 and #10177 it is important that unstamped builds are shielded from any access to the status files:

• Remove system specific information from stable-status.txt #10177 breaks all caching when you access stable-status.txt even if you set your workspace status command to /bin/true.
• Working around Bazel remote-caching does not properly treat volatile-status as if it never changes #10075 forces us to use no-remote-exec on our stamped actions that access volatile-status.txt, which is a bottleneck for remote builds.

I tried using a custom build flag as a work-around to propagate my own version of the stamp setting, but it causes all outputs through transitions to change their output path and destroys all caching -- even on targets that don't depend on the new stamp-like setting.

What operating system are you running Bazel on?

macOS and Linux

What's the output of bazel info release?

release 3.0.0

[aiuto]

This would be incredibly useful. All the rules in bazelbuild/rules_pkg need the capability to either use epoch based times in distribution archives (for reproduceibilty) OR to use the current time (for the release build). To get that behavior, we need it as a runtime flag, not a rule attribute.

[alanfalloon]

So, I found a work-around. You can define a config_setting that examines the stamp flag, and select a boolean attribute.

If, like me, you want a private attr, then you can use a label attr and default to an alias that selects over bool_setting instances. This avoids the issue where the bool_flag invalidates the transitions.

[aiuto]

configurabilty team looked at this. It might trigger some other feature changes we can do, but we are not planning to work on this issue right now, since there is a workaround.

[aiuto]

Also: #10907

[ulfjack]

What kind of feature changes? It seems straightforward to add a stamp field or method to the context, and I don't see any immediate issue with that.

[moroten]

It is possible to use a transition to read the command line option and let a provider pass the information back. See the stamp_selected target in the example below. Note that with_stamp.txt is preferably generated using genrule(stamp=True) or ctx.info_file whereas no_stamp.txt is often a static file.

# BUILD.bazel
load("stamp.bzl", "select_stamp_file", "stamp_info_rule")
load("@bazel_skylib//rules:common_settings.bzl", "bool_setting")

bool_setting(
    name = "stamp_setting",
    build_setting_default = False,
    visibility = ["//visibility:private"],
)

stamp_info_rule(
    name = "stamp_info",
    visibility = ["//visibility:private"],
)

select_stamp_file(
    name = "stamp_selected",
    no_stamp = "no_stamp.txt",
    with_stamp = "with_stamp.txt",
)

# stamp.bzl
load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo")

StampInfo = provider("Holds the value of --stamp", fields = ["enabled"])

def _stamp_transition_impl(settings, attr):
    _ignore = attr
    return {
        "@//:stamp_setting": settings["//command_line_option:stamp"],
    }

stamp_transition = transition(
    implementation = _stamp_transition_impl,
    inputs = ["//command_line_option:stamp"],
    outputs = ["//:stamp_setting"],
)

def _stamp_info_rule_impl(ctx):
    return StampInfo(enabled = ctx.attr._enabled[BuildSettingInfo].value)

stamp_info_rule = rule(
    implementation = _stamp_info_rule_impl,
    cfg = stamp_transition,
    attrs = {
        "_enabled": attr.label(default = "//:stamp_setting"),
        "_allowlist_function_transition": attr.label(
            default = "@bazel_tools//tools/allowlists/function_transition_allowlist"
        ),
    },
)

def _select_stamp_file_impl(ctx):
    if ctx.attr._stamp_info[StampInfo].enabled:
        ret = ctx.file.with_stamp
    else:
        ret = ctx.file.no_stamp
    return DefaultInfo(
        files = depset([ret]),
    )

select_stamp_file = rule(
    implementation = _select_stamp_file_impl,
    attrs = {
        "_stamp_info": attr.label(default = "@//:stamp_info"),
        "no_stamp": attr.label(mandatory = True, allow_single_file=True),
        "with_stamp": attr.label(mandatory = True, allow_single_file=True),
    },
)

[aiuto]

You can do this without a transition. Something like

//my/BUILD

config_setting(
    name = "private_stamp_detect",
    values = {"stamp": "1"},
)

//some/rule.bzl

foo_inner = rule(
    implementation = _foo_impl,
    attrs = {
        "stamp": attr.int(
            default = 0,  # Mimic the *_binary stamp behavior.
        ),
        # Is --stamp set on the command line?
        "private_stamp_detect": attr.bool(default = False),
   ...
)

def foo(name, **kwargs):
    foo_inner(
        name = name,
        private_stamp_detect = select({
            //my:private_stamp_detect: True,
            "//conditions:default": False,
        }),
        **kwargs,
    )

def _foo_impl(ctx):
 ...
  if ctx.attr.stamp == 1 or (ctx.attr.stamp == -1 and ctx.attr.private_stamp_detect):
      # stamped path
      args.append("--stamp_from=%s" % ctx.version_file.path)
      files.append(ctx.version_file)
  else:
      # unstamped path.

But this has the same limitation as your example - that we need an external helper to extract the time text from a file. What is missing for me is having the time stamp directly accessible from ctx, so we could pass it directly to a helper tool.

[moroten]

@aiuto Your solution with a single config_setting is much simpler. I also prefer filtering stable-status.txt before using the data to improve cache hit rate, so I never tend to be dependent on using the stamp flag in rule implementations.

[aiuto]

That is what I did for rules_pkg. What I don't like is that I need a process run in an action to parse the status file. That either means some sort of shell vs .bat script or a binary of some sort, then your rule may have to depend on some language toolchain.

…

On Thu, Dec 9, 2021 at 1:55 PM Fredrik Medley ***@***.***> wrote: @aiuto <https://github.com/aiuto> Your solution with a single config_setting is much simpler. I also prefer filtering stable-status.txt before using the data to improve cache hit rate, so I never tend to be dependent on using the stamp flag in rule implementations. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#11164 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXHHHFLAYIA6T4PDJLQX7DUQD3RVANCNFSM4MMS7EVQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[rickeylev]

I just went through the same as aiuto for the Python rules; having to run another executable is w/e, but having to figure out the necessary shell to transform things was a pain. Plus, since I'm now relying on bash, that inevitably means e.g. Windows or other platforms without bash need more work. This would have been trivially easy and easily cross-platform if they were directly available to Starlark on ctx somewhere.

That said, a couple tips for other who need to process these files:

1. declare -A and read can be used to easily parse the files into an associate array in bash:

declare -A info
while IFS= read -r line; do
  read key value <<< "$line"
  info["$key"]="$value"
done < <(cat "$VERSION_FILE" "$INFO_FILE")s
echo ${info[BUILD_ID}

Will basically give you an array with all the keys/values from the files in it.

2. When running this, create an implicit attribute on an executable and use ctx.actions.run instead of run_shell (remember to +x for a shell script file). Now you can easily use an alias with select() to switch to different parser programs depending on the platform.

[alexeagle]

We added a stamping feature to Aspect's bazel-lib which solves this problem in a nice, tidy way:
https://docs.aspect.build/rules/aspect_bazel_lib/docs/stamping
We use that in all our rules.

[comius]

It seems the issue is resolved with improved api @buildbreaker2021 is working on