From f90b810d9316acaa1b5736cc0b258ad90c157fea Mon Sep 17 00:00:00 2001 From: Amaury Pouly Date: Thu, 18 Jan 2024 15:47:25 +0000 Subject: [PATCH 01/10] [util] Refactor version info parsing Several tools need to extract some information from bazel's version file so create small library for that purpose to avoid duplicating code. Signed-off-by: Amaury Pouly (cherry picked from commit adebfaacf2af02aba44a344635d7a1a1076e7a91) --- util/regtool.py | 10 ++++------ util/topgen.py | 13 ++----------- util/version_file.py | 25 +++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 17 deletions(-) create mode 100644 util/version_file.py diff --git a/util/regtool.py b/util/regtool.py index eb12c34b27839..769518f93bb36 100755 --- a/util/regtool.py +++ b/util/regtool.py @@ -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 = ''' @@ -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.parse_version_file(args.version_stamp) if fmt == 'doc': with outfile: diff --git a/util/topgen.py b/util/topgen.py index 4fbefe691d21a..ab6785237d226 100755 --- a/util/topgen.py +++ b/util/topgen.py @@ -18,6 +18,7 @@ import hjson import tlgen +import version_file from design.lib.common import expand_seed from ipgen import (IpBlockRenderer, IpConfig, IpDescriptionOnlyRenderer, IpTemplate, TemplateRenderError) @@ -1126,17 +1127,7 @@ def main(): raise SystemExit(sys.exc_info()[1]) # Extract version stamp from file - version_stamp = {} - if args.version_stamp is not None: - try: - with open(args.version_stamp, 'rt') as f: - log.info("version stamp path: {}", args.version_stamp) - for line in f: - k, v = line.strip().split(' ', 1) - version_stamp[k] = v - log.info("{} {}", k, v) - except ValueError: - raise SystemExit(sys.exc_info()[1]) + version_stamp = version_file.parse_version_file(args.version_stamp) # Initialize RNG for compile-time netlist constants. # If specified, override the seed for random netlist constant computation. diff --git a/util/version_file.py b/util/version_file.py new file mode 100644 index 0000000000000..6c0e86d28f165 --- /dev/null +++ b/util/version_file.py @@ -0,0 +1,25 @@ +#!/usr/bin/env python3 +# Copyright lowRISC contributors. +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +import sys + + +def parse_version_file(path): + """ + Parse a bazel version file and return a dictionary of the values. + If `path` is None, an empty dictionary is returned. + If an error occurs during parsing, an exception is raised. + """ + if path is None: + return {} + version_stamp = {} + try: + with open(path, 'rt') as f: + for line in f: + k, v = line.strip().split(' ', 1) + version_stamp[k] = v + except ValueError: + raise SystemExit(sys.exc_info()[1]) + return version_stamp From 80ba57f3b6ece0c08a1a634847abc3833e479475 Mon Sep 17 00:00:00 2001 From: Amaury Pouly Date: Thu, 18 Jan 2024 15:48:42 +0000 Subject: [PATCH 02/10] [bazel] Introduce a stamping module The purpose of this module is to enabled rules to detect whether stamping is enabled on the command-line or not. It requires some hackery inspired by a bazel bug thread and which is very similar to how rules_rust handles this as well. Signed-off-by: Amaury Pouly (cherry picked from commit bc4fd072abebf52b1bf15bb540115c5c245f183a) --- rules/stamp.bzl | 82 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 rules/stamp.bzl diff --git a/rules/stamp.bzl b/rules/stamp.bzl new file mode 100644 index 0000000000000..3aa3efb6a439c --- /dev/null +++ b/rules/stamp.bzl @@ -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 +# 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 From 8e1cc595c95994f339023d20cf28e17ff130091e Mon Sep 17 00:00:00 2001 From: Amaury Pouly Date: Thu, 18 Jan 2024 15:50:25 +0000 Subject: [PATCH 03/10] [bazel] Create a stamp_flag using the new stamp module Signed-off-by: Amaury Pouly (cherry picked from commit 62dcd3f0fbb20ae59e99ebd9f13d4da489a299f8) --- rules/BUILD | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rules/BUILD b/rules/BUILD index ca0e934524287..7dacea45605c8 100644 --- a/rules/BUILD +++ b/rules/BUILD @@ -3,6 +3,7 @@ # SPDX-License-Identifier: Apache-2.0 load("//rules:opentitan.bzl", "OPENTITAN_PLATFORM") +load("//rules:stamp.bzl", "stamp_flag") package(default_visibility = ["//visibility:public"]) @@ -10,3 +11,6 @@ config_setting( name = "opentitan_platform", values = {"platforms": OPENTITAN_PLATFORM}, ) + +# See stamp.bzl for explanation. +stamp_flag(name = "stamp_flag") From 456812818f7a2007d472c3916e7dbbfaa648e96e Mon Sep 17 00:00:00 2001 From: Amaury Pouly Date: Thu, 18 Jan 2024 15:51:24 +0000 Subject: [PATCH 04/10] [bazel] Convert `autogen_hjson_header` to using stamping With this change, `autogen_hjson_header` will now only pass stamping information to regtool if required on the command-line. Signed-off-by: Amaury Pouly (cherry picked from commit f0ec914a48d196eda4d9677c08ee588e256032f6) --- rules/autogen.bzl | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/rules/autogen.bzl b/rules/autogen.bzl index e147e6bc6eb16..66a670f6b55d5 100644 --- a/rules/autogen.bzl +++ b/rules/autogen.bzl @@ -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 @@ -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, ) @@ -60,16 +67,12 @@ 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): From 8a0d7201578aecd88b8f1f9dcfaac07dda6ce23d Mon Sep 17 00:00:00 2001 From: Amaury Pouly Date: Thu, 18 Jan 2024 15:52:40 +0000 Subject: [PATCH 05/10] [bazel] Convert `autogen_chip_info_src` to stamping With this change, `autogen_chip_info_src` will only pass version information if stamping is enabled on the command-line. This commit changes the meaning of the `--ot_version_file` flag of `//util:rom_chip_info`. Previously, it was a path to a file that contains the git sha. It is now a path to the bazel version info file which is parsed using the `version_info` library. Note: with this change, unless stamping is enabled, the ROM will not contained the git sha in the chip_info anymore. Signed-off-by: Amaury Pouly (cherry picked from commit 375a0379cf5cf1f7bd073359332157c60758ecec) --- rules/autogen.bzl | 20 ++++++++++---------- util/rom_chip_info.py | 17 +++++++++++------ util/rom_chip_info_test.py | 8 ++++---- 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/rules/autogen.bzl b/rules/autogen.bzl index 66a670f6b55d5..618dbb9b82f71 100644 --- a/rules/autogen.bzl +++ b/rules/autogen.bzl @@ -76,21 +76,25 @@ autogen_hjson_header = rule( ) 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, ) @@ -101,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): diff --git a/util/rom_chip_info.py b/util/rom_chip_info.py index c73fec8d339a2..aa641609762f4 100755 --- a/util/rom_chip_info.py +++ b/util/rom_chip_info.py @@ -8,6 +8,8 @@ import logging as log from pathlib import Path +import version_file + def generate_chip_info_c_source(scm_revision: int) -> str: """Return the contents of a C source file that defines `kChipInfo`. @@ -43,11 +45,14 @@ def generate_chip_info_c_source(scm_revision: int) -> str: """ -def read_version_file(path: Path) -> int: - """Interprets the contents of `path` as a big-endian hex literal.""" +def read_version_file(version_info_path) -> int: + """ + Search for the scm revision variable and interprets + the contents as a big-endian hex literal. Return an error + if the revision cannot be found. + """ - with open(path, "rt") as f: - version = f.read().strip() + version = version_info.get('BUILD_SCM_REVISION', "8badF00d") return int(version, base=16) @@ -73,13 +78,13 @@ def main(): type=Path, help='Output Directory') parser.add_argument('--ot_version_file', - required=True, type=Path, help='Path to a file with the OpenTitan Version') args = parser.parse_args() - version = read_version_file(args.ot_version_file) + # Extract version stamp from file + version = read_version_file(version_file.parse_version_file(args.ot_version_file)) log.info("Version: %x" % (version, )) generated_source = generate_chip_info_c_source(version) diff --git a/util/rom_chip_info_test.py b/util/rom_chip_info_test.py index 1838f054c9542..2d433ee1ceff6 100644 --- a/util/rom_chip_info_test.py +++ b/util/rom_chip_info_test.py @@ -78,8 +78,8 @@ def test_sha1_digest_too_small(self): class TestFileOperations(unittest.TestCase): - @patch('rom_chip_info.open', - mock_open(read_data=f'{EXAMPLE_SHA1_DIGEST:x}')) + @patch('version_file.open', + mock_open(read_data=f'BUILD_SCM_REVISION {EXAMPLE_SHA1_DIGEST:x}')) def test_read_version_file(self): """Reading a properly-formatted version file produces the expected int. """ @@ -87,7 +87,7 @@ def test_read_version_file(self): pathlib.Path("fake/path/version.txt")) self.assertEqual(version, EXAMPLE_SHA1_DIGEST) - @patch("rom_chip_info.open", mock_open(read_data='')) + @patch("version_file.open", mock_open(read_data='')) def test_read_version_file_empty(self): """Reading an empty version file raises an exception. """ @@ -95,7 +95,7 @@ def test_read_version_file_empty(self): rom_chip_info.read_version_file( pathlib.Path("fake/path/version.txt")) - @patch("rom_chip_info.open", mock_open(read_data='xyz')) + @patch("version_file.open", mock_open(read_data='BUILD_SCM_REVISION xyz')) def test_read_version_file_invalid_hex(self): """Reading an invalid version file raises an exception. """ From b40dd739d0e47ef4f9aa5384dae11509e70a6ca8 Mon Sep 17 00:00:00 2001 From: Amaury Pouly Date: Thu, 18 Jan 2024 15:56:23 +0000 Subject: [PATCH 06/10] [bazel] Remove dependency of opentitantool of version file This dependency is not used because the version information comes from `stamp-env.txt` which is handled directly by `rust_binary`. Signed-off-by: Amaury Pouly (cherry picked from commit 4df546f95802d0a268d128486379c7444992da70) --- sw/host/opentitantool/BUILD | 3 --- 1 file changed, 3 deletions(-) diff --git a/sw/host/opentitantool/BUILD b/sw/host/opentitantool/BUILD index e98c52af57733..958f698d48b12 100644 --- a/sw/host/opentitantool/BUILD +++ b/sw/host/opentitantool/BUILD @@ -36,9 +36,6 @@ rust_binary( "src/command/version.rs", "src/main.rs", ], - compile_data = [ - "//util:full_version.txt", - ], proc_macro_deps = [ "//sw/host/opentitanlib/opentitantool_derive", ], From a2572f0c2981694a8c133b0013905d6772d1546e Mon Sep 17 00:00:00 2001 From: Amaury Pouly Date: Thu, 18 Jan 2024 15:55:52 +0000 Subject: [PATCH 07/10] [bazel] Remove unused targets With the previous changes, those targets have become useless and can be removed. Signed-off-by: Amaury Pouly (cherry picked from commit bc22e8bba266b4d98f94608dca5729f46242cc55) --- util/BUILD | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/util/BUILD b/util/BUILD index bfec5bca7a345..93632c41a5f37 100644 --- a/util/BUILD +++ b/util/BUILD @@ -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"], From 11d8202e5b814207b03b4b72b2f88bf5c98b1acb Mon Sep 17 00:00:00 2001 From: Amaury Pouly Date: Mon, 22 Jan 2024 15:38:19 +0000 Subject: [PATCH 08/10] [util/reggen] Do not generate stamping information if not available If stamping is disable, version_stamp will be an empty dictionary and the code will print something like ``` // Generated register constants for rv_plic. // Built for // https://github.com/lowRISC/opentitan/tree/ // Tree status: // Build date: ``` This is unhelpful and still prints the current time which is defeats the point of not stamping. The new code will not print anything when a piece of information is not available. Signed-off-by: Amaury Pouly (cherry picked from commit 24903e9131195be53773a899b1a15c2744211118) --- util/reggen/gen_tock.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/util/reggen/gen_tock.py b/util/reggen/gen_tock.py index 13a8696249ce6..009de52b09dbf 100644 --- a/util/reggen/gen_tock.py +++ b/util/reggen/gen_tock.py @@ -384,8 +384,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 @@ -395,17 +393,20 @@ 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', '')) - genout(outfile, '// https://github.com/lowRISC/opentitan/tree/{}\n', - version_stamp.get('BUILD_SCM_REVISION', '')) - genout(outfile, '// Tree status: {}\n', - version_stamp.get('BUILD_SCM_STATUS', '')) - genout(outfile, '// Build date: {}\n\n', dt.isoformat()) + if 'BUILD_GIT_VERSION' in version_stamp: + genout(outfile, '// Built for {}\n', version_stamp['BUILD_GIT_VERSION']) + if 'BUILD_SCM_REVISION' in version_stamp: + genout(outfile, '// https://github.com/lowRISC/opentitan/tree/{}\n', version_stamp['BUILD_SCM_REVISION']) + if 'BUILD_SCM_STATUS' in version_stamp: + genout(outfile, '// Tree status: {}\n', version_stamp['BUILD_SCM_STATUS']) + if 'BUILD_TIMESTAMP' in version_stamp: + tm = int(version_stamp['BUILD_TIMESTAMP']) + dt = datetime.utcfromtimestamp(tm) + genout(outfile, '// Build date: {}\n\n', dt.isoformat()) if src_file: genout(outfile, '// Original reference file: {}\n', src_file) From a81a070d5a0e2bef9958418c68ee5eb5fb512607 Mon Sep 17 00:00:00 2001 From: Amaury Pouly Date: Mon, 22 Jan 2024 16:52:54 +0000 Subject: [PATCH 09/10] [util] Make version information a class Instead of treating version_stamp like a dictionary with specific name entries, turn it into a class with proper accessors so that the particular representation of the version information is abstracted. Signed-off-by: Amaury Pouly (cherry picked from commit 595d28c22b1d0e603c50dc2209957a59f6f8a700) --- util/reggen/gen_tock.py | 23 ++++++++++----------- util/regtool.py | 2 +- util/rom_chip_info.py | 5 +++-- util/topgen.py | 2 +- util/topgen/rust.py | 43 ++++++++++++++-------------------------- util/version_file.py | 44 +++++++++++++++++++++++++---------------- 6 files changed, 57 insertions(+), 62 deletions(-) diff --git a/util/reggen/gen_tock.py b/util/reggen/gen_tock.py index 009de52b09dbf..0d2a09d005e96 100644 --- a/util/reggen/gen_tock.py +++ b/util/reggen/gen_tock.py @@ -10,8 +10,7 @@ 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 @@ -19,6 +18,8 @@ from reggen.register import Register from reggen.window import Window +from version_file import VersionInformation + REG_VISIBILITY = 'pub(crate)' FIELD_VISIBILITY = 'pub(crate)' @@ -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() @@ -397,16 +398,12 @@ def gen_tock(block: IpBlock, outfile: TextIO, src_file: Optional[str], genout(outfile, '\n') genout(outfile, '// Generated register constants for {}.\n', block.name) - if 'BUILD_GIT_VERSION' in version_stamp: - genout(outfile, '// Built for {}\n', version_stamp['BUILD_GIT_VERSION']) - if 'BUILD_SCM_REVISION' in version_stamp: - genout(outfile, '// https://github.com/lowRISC/opentitan/tree/{}\n', version_stamp['BUILD_SCM_REVISION']) - if 'BUILD_SCM_STATUS' in version_stamp: - genout(outfile, '// Tree status: {}\n', version_stamp['BUILD_SCM_STATUS']) - if 'BUILD_TIMESTAMP' in version_stamp: - tm = int(version_stamp['BUILD_TIMESTAMP']) - dt = datetime.utcfromtimestamp(tm) - 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) diff --git a/util/regtool.py b/util/regtool.py index 769518f93bb36..4ac0ba5af14b6 100755 --- a/util/regtool.py +++ b/util/regtool.py @@ -229,7 +229,7 @@ def main(): sys.exit(1) # Extract version stamp from file - version_stamp = version_file.parse_version_file(args.version_stamp) + version_stamp = version_file.VersionInformation(args.version_stamp) if fmt == 'doc': with outfile: diff --git a/util/rom_chip_info.py b/util/rom_chip_info.py index aa641609762f4..7a63f7e1f430b 100755 --- a/util/rom_chip_info.py +++ b/util/rom_chip_info.py @@ -52,7 +52,8 @@ def read_version_file(version_info_path) -> int: if the revision cannot be found. """ - version = version_info.get('BUILD_SCM_REVISION', "8badF00d") + version_info = version_file.VersionInformation(version_info_path) + version = version_info.scm_revision("") return int(version, base=16) @@ -84,7 +85,7 @@ def main(): args = parser.parse_args() # Extract version stamp from file - version = read_version_file(version_file.parse_version_file(args.ot_version_file)) + version = read_version_file(args.ot_version_file) log.info("Version: %x" % (version, )) generated_source = generate_chip_info_c_source(version) diff --git a/util/topgen.py b/util/topgen.py index ab6785237d226..ccf1fef167f0a 100755 --- a/util/topgen.py +++ b/util/topgen.py @@ -1127,7 +1127,7 @@ def main(): raise SystemExit(sys.exc_info()[1]) # Extract version stamp from file - version_stamp = version_file.parse_version_file(args.version_stamp) + version_stamp = version_file.VersionInformation(args.version_stamp) # Initialize RNG for compile-time netlist constants. # If specified, override the seed for random netlist constant computation. diff --git a/util/topgen/rust.py b/util/topgen/rust.py index 8dbb3a59225ca..b293bc448bfc3 100644 --- a/util/topgen/rust.py +++ b/util/topgen/rust.py @@ -4,13 +4,14 @@ """This contains a class which is used to help generate `top_{name}.rs`.""" from collections import OrderedDict, defaultdict from typing import Dict, List, Optional, Tuple -from datetime import datetime from mako.template import Template from reggen.ip_block import IpBlock from .lib import Name, get_base_and_size +from version_file import VersionInformation + RUST_FILE_EXTENSIONS = (".rs") @@ -188,35 +189,21 @@ def render_definition(self): class RustFileHeader(object): - def __init__(self, name: str, version_stamp: Dict[str, str], skip: bool): - self.name = name + def __init__(self, version_stamp: VersionInformation): self.data = version_stamp - self.skip = skip - tm = int(version_stamp.get('BUILD_TIMESTAMP', 0)) - self.tstamp = datetime.utcfromtimestamp(tm) if tm else datetime.utcnow() - - def build(self) -> str: - return self.data.get('BUILD_GIT_VERSION', '') - - def scm_sha(self) -> str: - return self.data.get('BUILD_SCM_REVISION', '') - - def scm_status(self) -> str: - return self.data.get('BUILD_SCM_STATUS', '') - - def time_stamp(self) -> str: - return self.tstamp.isoformat() def render(self): - if self.skip: - return Template(("")).render(header=self) - else: - template = ("\n" - "// Built for ${header.build()}\n" - "// https://github.com/lowRISC/opentitan/tree/${header.scm_sha()}\n" - "// Tree status: ${header.scm_status()}\n" - "// Build date: ${header.time_stamp()}\n") - return Template(template).render(header=self) + template = "" + if self.data.scm_version() is not None: + template += "// Built for ${header.data.scm_version()}\n" + if self.data.scm_revision() is not None: + template += ("// https://github.com/lowRISC/opentitan/tree/" + "${header.data.scm_revision()}\n") + if self.data.scm_status() is not None: + template += "// Tree status: ${header.data.scm_status()}\n" + if template != "": + template = "\n" + template + return Template(template).render(header=self) class TopGenRust: @@ -225,7 +212,7 @@ def __init__(self, top_info, name_to_block: Dict[str, IpBlock], version_stamp: D self._top_name = Name(["top"]) + Name.from_snake_case(top_info["name"]) self._name_to_block = name_to_block self.regwidth = int(top_info["datawidth"]) - self.file_header = RustFileHeader("foo.tpl", version_stamp, len(version_stamp) == 0) + self.file_header = RustFileHeader(version_stamp) self._init_plic_targets() self._init_plic_mapping() diff --git a/util/version_file.py b/util/version_file.py index 6c0e86d28f165..15136194bd086 100644 --- a/util/version_file.py +++ b/util/version_file.py @@ -4,22 +4,32 @@ # SPDX-License-Identifier: Apache-2.0 import sys +from typing import Union -def parse_version_file(path): - """ - Parse a bazel version file and return a dictionary of the values. - If `path` is None, an empty dictionary is returned. - If an error occurs during parsing, an exception is raised. - """ - if path is None: - return {} - version_stamp = {} - try: - with open(path, 'rt') as f: - for line in f: - k, v = line.strip().split(' ', 1) - version_stamp[k] = v - except ValueError: - raise SystemExit(sys.exc_info()[1]) - return version_stamp +class VersionInformation(): + def __init__(self, path: str): + """ + Parse a bazel version file and store a dictionary of the values. + If `path` is None, an empty dictionary will be created. + If an error occurs during parsing, an exception is raised. + """ + self.version_stamp = {} + if path is None: + return + try: + with open(path, 'rt') as f: + for line in f: + k, v = line.strip().split(' ', 1) + self.version_stamp[k] = v + except ValueError: + raise SystemExit(sys.exc_info()[1]) + + def scm_version(self, default: Union[str, None] = None) -> Union[str, None]: + return self.version_stamp.get('BUILD_GIT_VERSION', default) + + def scm_revision(self, default: Union[str, None] = None) -> Union[str, None]: + return self.version_stamp.get('BUILD_SCM_REVISION', default) + + def scm_status(self, default: Union[str, None] = None) -> Union[str, None]: + return self.version_stamp.get('BUILD_SCM_STATUS', default) From 04b03abfac7968de2620558d3023bf607dbcfda2 Mon Sep 17 00:00:00 2001 From: Amaury Pouly Date: Tue, 23 Jan 2024 16:25:28 +0000 Subject: [PATCH 10/10] [opentitantool] Rework stamping Allow opentitantool to be built with stamping disabled, and disable it by default. Update the `version` command to handle the case of missing information. The build time/date is removed since it adds no useful information. Signed-off-by: Amaury Pouly (cherry picked from commit 97cf2c70fe2912ee9ba7e1f37731cbba331958a2) --- sw/host/opentitantool/BUILD | 2 +- sw/host/opentitantool/src/command/gpio.rs | 15 ++++++++--- sw/host/opentitantool/src/command/version.rs | 26 ++++++++++++++------ 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/sw/host/opentitantool/BUILD b/sw/host/opentitantool/BUILD index 958f698d48b12..0f5089f8187a1 100644 --- a/sw/host/opentitantool/BUILD +++ b/sw/host/opentitantool/BUILD @@ -43,7 +43,7 @@ rust_binary( "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", diff --git a/sw/host/opentitantool/src/command/gpio.rs b/sw/host/opentitantool/src/command/gpio.rs index eaafeaafb15a4..4f0b27e9df8b1 100644 --- a/sw/host/opentitantool/src/command/gpio.rs +++ b/sw/host/opentitantool/src/command/gpio.rs @@ -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("".into()), + if let Some(clean) = version.clean { + if clean { + "clean" + } else { + "modified" + } + } else { + "" + } )?; writeln!(&mut file, "$end")?; match clock_nature { diff --git a/sw/host/opentitantool/src/command/version.rs b/sw/host/opentitantool/src/command/version.rs index 71a5e2477c5a1..8a88d7ece7cd2 100644 --- a/sw/host/opentitantool/src/command/version.rs +++ b/sw/host/opentitantool/src/command/version.rs @@ -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, + pub revision: Option, + pub clean: Option, +} + +fn parse_variable(content: &str) -> Option { + // 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::().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"), } } }