Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework how bazel stamping work in the repository #20881

Merged
merged 10 commits into from
Feb 2, 2024
4 changes: 4 additions & 0 deletions rules/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,14 @@
# SPDX-License-Identifier: Apache-2.0

load("//rules:opentitan.bzl", "OPENTITAN_PLATFORM")
load("//rules:stamp.bzl", "stamp_flag")

package(default_visibility = ["//visibility:public"])

config_setting(
name = "opentitan_platform",
values = {"platforms": OPENTITAN_PLATFORM},
)

# See stamp.bzl for explanation.
stamp_flag(name = "stamp_flag")
39 changes: 21 additions & 18 deletions rules/autogen.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
# Licensed under the Apache License, Version 2.0, see LICENSE for details.
# SPDX-License-Identifier: Apache-2.0

load("//rules:stamp.bzl", "stamp_attr", "stamping_enabled")

"""Autogeneration rules for OpenTitan.

The rules in this file are for autogenerating various file resources
Expand All @@ -27,17 +29,22 @@ def _hjson_header(ctx):
executable = ctx.executable._regtool,
)

stamp_args = []
stamp_files = []
if stamping_enabled(ctx):
stamp_files = [ctx.version_file]
stamp_args.append("--version-stamp={}".format(ctx.version_file.path))

tock = ctx.actions.declare_file("{}.rs".format(ctx.label.name))
ctx.actions.run(
outputs = [tock],
inputs = ctx.files.srcs + [ctx.executable._regtool, ctx.file.version_stamp],
inputs = ctx.files.srcs + [ctx.executable._regtool] + stamp_files,
arguments = [
"--tock",
"--version-stamp={}".format(ctx.file.version_stamp.path),
"-q",
"-o",
tock.path,
] + node + [src.path for src in ctx.files.srcs],
] + stamp_args + node + [src.path for src in ctx.files.srcs],
executable = ctx.executable._regtool,
)

Expand All @@ -60,34 +67,34 @@ autogen_hjson_header = rule(
"node": attr.string(
doc = "Register block node to generate",
),
"version_stamp": attr.label(
default = "//util:full_version_file",
allow_single_file = True,
),
"_regtool": attr.label(
default = "//util:regtool",
executable = True,
cfg = "exec",
),
},
} | stamp_attr(-1, "//rules:stamp_flag"),
)

def _chip_info_src(ctx):
stamp_args = []
stamp_files = []
if stamping_enabled(ctx):
stamp_files = [ctx.version_file]
stamp_args.append("--ot_version_file")
stamp_args.append(ctx.version_file.path)

out_source = ctx.actions.declare_file("chip_info.c")
ctx.actions.run(
outputs = [
out_source,
],
inputs = [
ctx.file.version,
ctx.executable._tool,
],
] + stamp_files,
arguments = [
"-o",
out_source.dirname,
"--ot_version_file",
ctx.file.version.path,
],
] + stamp_args,
executable = ctx.executable._tool,
)

Expand All @@ -98,16 +105,12 @@ def _chip_info_src(ctx):
autogen_chip_info_src = rule(
implementation = _chip_info_src,
attrs = {
"version": attr.label(
default = "//util:scm_revision_file",
allow_single_file = True,
),
"_tool": attr.label(
default = "//util:rom_chip_info",
executable = True,
cfg = "exec",
),
},
} | stamp_attr(1, "//rules:stamp_flag"),
)

def autogen_chip_info(name):
Expand Down
82 changes: 82 additions & 0 deletions rules/stamp.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
# Copyright lowRISC contributors.
# Licensed under the Apache License, Version 2.0, see LICENSE for details.
# SPDX-License-Identifier: Apache-2.0

# Utilities to detect whether --stamp was specified on the command-line.
# This relies on the workaround explained in
# https://github.com/bazelbuild/bazel/issues/11164#issuecomment-617336309

# Return a rule attribute to add a stamp parameters. This stamping parameter follows
# the usual bazel convention:
# - `stamp = 1` means that stamping information is available, even if stamping is
jwnrt marked this conversation as resolved.
Show resolved Hide resolved
# is disabled on the command line (`--nostamp`).
# - `stamp = 0` means that stamping information is not available, even if stamping is
# is enabled on the command line (`--stamp`).
# - `stamp = -1` means that stamping is controlled by the command line `--stamp` or
# `--nostamp` flags.
# See https://bazel.build/docs/user-manual#workspace-status for more information
# on stamping.
#
# The first argument of that macro is the default value of the `stamp` attribute.
# The second argument of that macro is the label of a stamp flag defined using `stamp_flag`.
#
# Use the `stamping_enabled` macro in the rule implementation to determine whether stamping
# is enabled.
def stamp_attr(default_value, stamp_flag):
return {
"stamp": attr.int(
doc = """Whether to provide access to stamping information to the rule.""",
default = default_value,
values = [-1, 0, 1],
),
"_stamp_flag": attr.label(
default = stamp_flag,
providers = [StampFlag],
),
}

# See `stamp_flag`
StampFlag = provider("Value of --stamp", fields = ["stamp_flag"])

def _stamp_flag_impl(ctx):
return [StampFlag(stamp_flag = ctx.attr.value)]

_stamp_flag = rule(
implementation = _stamp_flag_impl,
attrs = {
"value": attr.bool(
doc = "The value of the stamp flag",
mandatory = True,
),
},
provides = [StampFlag],
)

# Define a stamping configuration flag to be used with `stamp_attr`.
# This is convoluted because we cannot use select in default attributes of rules.
# Hence we need define a rule just for the purpose of selecting its value using
# a configuration setting.
def stamp_flag(name):
native.config_setting(
name = name + "_config",
values = {"stamp": "1"},
)

_stamp_flag(
name = name,
value = select({
name + "_config": True,
"//conditions:default": False,
}),
)

# Determine whether stamping is enabled, see `stamp_attr`.
# Pass the rule context as argument.
def stamping_enabled(ctx):
stamp_attr = ctx.attr.stamp
if stamp_attr == -1:
return ctx.attr._stamp_flag[StampFlag].stamp_flag
elif stamp_attr == 0:
return False
else:
return True
5 changes: 1 addition & 4 deletions sw/host/opentitantool/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,14 @@ rust_binary(
"src/command/version.rs",
"src/main.rs",
],
compile_data = [
"//util:full_version.txt",
],
proc_macro_deps = [
"//sw/host/opentitanlib/opentitantool_derive",
],
rustc_env_files = [
"stamp-env.txt",
],
# stamping is necessary because opentitantool builds version.rs that needs it
stamp = 1,
stamp = -1,
deps = [
"//sw/host/opentitanlib",
"//sw/host/ot_certs",
Expand Down
15 changes: 11 additions & 4 deletions sw/host/opentitantool/src/command/gpio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,10 +410,17 @@ impl CommandDispatch for GpioMonitoringVcd {
let version = super::version::VersionResponse::default();
writeln!(
&mut file,
" opentitantool {} {} {}",
version.version,
if version.clean { "clean" } else { "modified" },
version.timestamp,
" opentitantool {} {}",
version.version.unwrap_or("<unknown>".into()),
if let Some(clean) = version.clean {
if clean {
"clean"
} else {
"modified"
}
} else {
"<unknown>"
}
)?;
writeln!(&mut file, "$end")?;
match clock_nature {
Expand Down
26 changes: 18 additions & 8 deletions sw/host/opentitantool/src/command/version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,29 @@ pub struct Version {}

#[derive(Debug, serde::Serialize)]
pub struct VersionResponse {
pub version: String,
pub revision: String,
pub clean: bool,
pub timestamp: i64,
pub version: Option<String>,
pub revision: Option<String>,
pub clean: Option<bool>,
}

fn parse_variable(content: &str) -> Option<String> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like a hack. Could you make it so that if stamping is disabled, the stamp-env is not given at all? Then you can just use std::option_env!(...).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that this is not possible with the way rules_rust handles rustc_env_file: stamping is handled by replacing {BUILD_SCM_REVISION} and others literally in the env file. If stamping is disabled, the replacement does not occur. But the content of the stamp-env.txt file does not depend on whether stamping is enabled or not. This, admitedly, make the way stamping is handled by rustc not so useful.
We could add custom feature to our rules_rust fork so that writing "{BUILD_SCM_REVISION?}" deletes the variables if not present. Maybe this could even be accepted upstream?

Alternatively, this could be achieved with some custom bazel rule to produce either an empty file or a version file, depending on whether stamping is enabled or not, and then give the target to rustc_env_file, but this could be more trouble than it is worth. Something like:

version_file_or(
    name = "ott_version_file",
    default_file = "//path/to/empty/file",
)
genrule(
  name = "ott_env_file",
  src = [":ott_version_file"],
  outs = ["stamp-env.txt"],
  cmd = "<inset awk script>", # convert from "KEY VALUE" to "KEY=VALUE"
)

// If stamping is disabled, the variable substition in stamp-env.txt
// will not happen and the content of the environment variable will be
// `{NAME_OF_VARIABLE}`. If we detect that this is the case, we return
// None, otherwise we return the content.
if content.starts_with('{') {
None
} else {
Some(content.to_string())
}
}

impl Default for VersionResponse {
fn default() -> Self {
VersionResponse {
version: std::env!("BUILD_GIT_VERSION").to_string(),
revision: std::env!("BUILD_SCM_REVISION").to_string(),
clean: std::env!("BUILD_SCM_STATUS") == "clean",
timestamp: std::env!("BUILD_TIMESTAMP").parse::<i64>().unwrap(),
version: parse_variable(std::env!("BUILD_GIT_VERSION")),
revision: parse_variable(std::env!("BUILD_SCM_REVISION")),
clean: parse_variable(std::env!("BUILD_SCM_STATUS")).map(|x| x == "clean"),
}
}
}
Expand Down
21 changes: 0 additions & 21 deletions util/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,6 @@ package(default_visibility = ["//visibility:public"])

exports_files(glob(["**"]))

genrule(
name = "ot_version_file",
outs = ["ot_version.txt"],
cmd = """awk '/BUILD_GIT_VERSION/ { print $$2 }' bazel-out/volatile-status.txt > $@""",
stamp = 1, # this provides volatile-status.txt
)

genrule(
name = "full_version_file",
outs = ["full_version.txt"],
cmd = """cp -f bazel-out/volatile-status.txt $@""",
stamp = 1, # this provides volatile-status.txt
)

genrule(
name = "scm_revision_file",
outs = ["scm_revision.txt"],
cmd = """awk '/BUILD_SCM_REVISION/ { print $$2 }' bazel-out/volatile-status.txt > $@""",
stamp = 1, # this provides volatile-status.txt
)

py_binary(
name = "otbn_build",
srcs = ["otbn_build.py"],
Expand Down
24 changes: 11 additions & 13 deletions util/reggen/gen_tock.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,16 @@
import sys
import textwrap
import warnings
from datetime import datetime
from typing import Any, Dict, Optional, Set, TextIO
from typing import Any, Optional, Set, TextIO

from reggen.ip_block import IpBlock
from reggen.multi_register import MultiRegister
from reggen.params import LocalParam
from reggen.register import Register
from reggen.window import Window

from version_file import VersionInformation

REG_VISIBILITY = 'pub(crate)'
FIELD_VISIBILITY = 'pub(crate)'

Expand Down Expand Up @@ -327,7 +328,7 @@ def gen_const_interrupts(fieldout: TextIO, block: IpBlock, component: str,

def gen_tock(block: IpBlock, outfile: TextIO, src_file: Optional[str],
src_lic: Optional[str], src_copy: str,
version_stamp: Dict[str, str]) -> int:
version: VersionInformation) -> int:
rnames = block.get_rnames()

paramout = io.StringIO()
Expand Down Expand Up @@ -384,8 +385,6 @@ def gen_tock(block: IpBlock, outfile: TextIO, src_file: Optional[str],
fieldstr = fieldout.getvalue()
fieldout.close()

tm = int(version_stamp.get('BUILD_TIMESTAMP', 0))
dt = datetime.utcfromtimestamp(tm) if tm else datetime.utcnow()
# Opensource council has approved dual-licensing the generated files under
# both Apache and MIT licenses.
# Since these generated files are meant to be imported into the Tock
Expand All @@ -395,17 +394,16 @@ def gen_tock(block: IpBlock, outfile: TextIO, src_file: Optional[str],
"// Licensed under the Apache License, Version 2.0 or the MIT License.\n"
)
genout(outfile, "// SPDX-License-Identifier: Apache-2.0 OR MIT\n")
genout(outfile, "// Copyright lowRISC contributors {}.\n", dt.year)
genout(outfile, "// Copyright lowRISC contributors.\n")
genout(outfile, '\n')
genout(outfile, '// Generated register constants for {}.\n', block.name)

genout(outfile, '// Built for {}\n',
version_stamp.get('BUILD_GIT_VERSION', '<unknown>'))
genout(outfile, '// https://github.com/lowRISC/opentitan/tree/{}\n',
version_stamp.get('BUILD_SCM_REVISION', '<unknown>'))
genout(outfile, '// Tree status: {}\n',
version_stamp.get('BUILD_SCM_STATUS', '<unknown>'))
genout(outfile, '// Build date: {}\n\n', dt.isoformat())
if version.scm_version() is not None:
genout(outfile, '// Built for {}\n', version.scm_version())
if version.scm_revision() is not None:
genout(outfile, '// https://github.com/lowRISC/opentitan/tree/{}\n', version.scm_revision())
if version.scm_status() is not None:
genout(outfile, '// Tree status: {}\n', version.scm_status())

if src_file:
genout(outfile, '// Original reference file: {}\n', src_file)
Expand Down
10 changes: 4 additions & 6 deletions util/regtool.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
from reggen.countermeasure import CounterMeasure
from reggen.ip_block import IpBlock

import version_file

DESC = """regtool, generate register info from Hjson source"""

USAGE = '''
Expand Down Expand Up @@ -226,12 +228,8 @@ def main():
fmt))
sys.exit(1)

version_stamp = {}
if args.version_stamp is not None:
with open(args.version_stamp, 'rt') as f:
for line in f:
k, v = line.strip().split(' ', 1)
version_stamp[k] = v
# Extract version stamp from file
version_stamp = version_file.VersionInformation(args.version_stamp)

if fmt == 'doc':
with outfile:
Expand Down
Loading
Loading