From fe89c99a56b6de40df97e3f83ccf71ce713699b5 Mon Sep 17 00:00:00 2001 From: Harry Callahan Date: Sat, 17 Aug 2024 19:24:19 +0100 Subject: [PATCH] [CLEANUP/REFACTOR] Moves, renames and refactors These things were bothering me as I went along, so this commit captures my many WIP fixes. In particular, try to use pathlib instead of strings where possible, and give variables more descriptive names. Also add typehints. Signed-off-by: Harry Callahan --- hw/data/common_project_cfg.hjson | 15 +- .../dv/uvm/common_project_cfg.hjson | 8 +- util/dvsim/FlowCfg.py | 254 +++++++++++------- util/dvsim/LintCfg.py | 3 +- util/dvsim/dvsim.py | 7 + 5 files changed, 171 insertions(+), 116 deletions(-) diff --git a/hw/data/common_project_cfg.hjson b/hw/data/common_project_cfg.hjson index f1cb1907029ae4..8b60517ef1df0a 100644 --- a/hw/data/common_project_cfg.hjson +++ b/hw/data/common_project_cfg.hjson @@ -2,11 +2,10 @@ // Licensed under the Apache License, Version 2.0, see LICENSE for details. // SPDX-License-Identifier: Apache-2.0 { - project: opentitan - repo_server: "github.com/lowrisc/opentitan" - book: opentitan.org/book - doc_server: docs.opentitan.org - results_server: reports.opentitan.org + project: opentitan + repo_server: "github.com/lowrisc/opentitan" + book: opentitan.org/book + results_server_fqdn: reports.opentitan.org // Default directory structure for the output scratch_base_path: "{scratch_root}/{branch}" @@ -23,12 +22,6 @@ { proj_root: "{proj_root}" } ] - // Results server stuff - indicate what command to use to copy over the results. - // Workaround for gsutil to fall back to using python2.7. - results_server_prefix: "gs://" - results_server_cmd: "/usr/bin/gsutil" - results_html_name: "report.html" - // If defined, this is printed into the results md files revision: '''{eval_cmd} COMMIT_L=`git rev-parse HEAD`; \ diff --git a/hw/vendor/lowrisc_ibex/dv/uvm/common_project_cfg.hjson b/hw/vendor/lowrisc_ibex/dv/uvm/common_project_cfg.hjson index ef0499fe055bf1..11408c260bd414 100644 --- a/hw/vendor/lowrisc_ibex/dv/uvm/common_project_cfg.hjson +++ b/hw/vendor/lowrisc_ibex/dv/uvm/common_project_cfg.hjson @@ -2,13 +2,11 @@ // Licensed under the Apache License, Version 2.0, see LICENSE for details. // SPDX-License-Identifier: Apache-2.0 { - project: ibex + project: ibex // These keys are expected by dvsim.py, so we have to set them to something. - book: bogus.book.domain - doc_server: bogus.doc.server - results_server: bogus.results.server - results_html_name: report.html + book: bogus.book.domain + results_server_fqdn: bogus.results.server // Default directory structure for the output scratch_base_path: "{scratch_root}/{dut}.{flow}.{tool}" diff --git a/util/dvsim/FlowCfg.py b/util/dvsim/FlowCfg.py index daeae879745465..60353177e39f92 100644 --- a/util/dvsim/FlowCfg.py +++ b/util/dvsim/FlowCfg.py @@ -10,6 +10,7 @@ import sys from pathlib import Path from shutil import which +import itertools import hjson from results_server import NoGCPError, ResultsServer @@ -46,7 +47,7 @@ def __init__(self, flow_cfg_file, hjson_data, args, mk_config): self.items = list(dict.fromkeys(args.items)) self.list_items = args.list self.select_cfgs = args.select_cfgs - self.flow_cfg_file = flow_cfg_file + self.flow_cfg_file = Path(flow_cfg_file) self.args = args self.scratch_root = args.scratch_root self.branch = args.branch @@ -86,28 +87,50 @@ def __init__(self, flow_cfg_file, hjson_data, args, mk_config): # Results self.errors_seen = False - self.rel_path = "" self.results_title = "" self.revision = "" - self.results_server_prefix = "" - self.results_server_cmd = "" self.css_file = os.path.join( os.path.dirname(os.path.realpath(__file__)), "style.css") # `self.results_*` below will be updated after `self.rel_path` and # `self.scratch_base_root` variables are updated. + + # The relative path from the project root to the flow_cfg_file directory + # This is used as a unique identifier under which reports from the flow are saved. + self.rel_path = "" + # Multiple runs from the same flow are uniquely identified by appending a unique + # 'timestamp' identifier to the relative-path string. This is 'latest' for all new + # runs. + self.rel_path_with_ts = "" + + # Variables defining the local output path for results + self.results_root = "" self.results_dir = "" - self.results_page = "" + self.results_name_stem = "report" + self.results_md_name = f"{self.results_name_stem}.md" + self.results_html_name = f"{self.results_name_stem}.html" + self.results_json_name = f"{self.results_name_stem}.json" + self.md_report_path = Path() + self.html_report_path = Path() + self.json_report_path = Path() + self.results_page = Path() + + # Results server variables self.results_server_path = "" self.results_server_dir = "" self.results_server_page = "" + self.results_server_scheme = "gs://" self.results_server_url = "" - self.results_html_name = "" + self.results_server_url_scheme = "https://" - # Full results in md text + self.results: dict = {} + self.results_dict: dict = {} + self.results_json = "" self.results_md = "" + # Selectively sanitized md results to be published self.publish_results_md = "" self.sanitize_publish_results = False + # Summary results, generated by over-arching cfg self.results_summary_md = "" @@ -130,8 +153,17 @@ def __init__(self, flow_cfg_file, hjson_data, args, mk_config): self._load_child_cfg(entry, mk_config) if self.rel_path == "": - self.rel_path = os.path.dirname(self.flow_cfg_file).replace( - self.proj_root + '/', '') + # By default, 'rel_path' is the path to the directory of the sim_cfg.hjson file + # for this flow. + self.rel_path = (self.flow_cfg_file.parent).relative_to(Path(self.proj_root)) + else: + # We must have picked up a value overriding the default from one of the + # .hjson files. Convert the string to a Path object. + self.rel_path = Path(self.rel_path) + + # By default, the current run timestamp/versioning information is always 'latest'. + # If this path already exists, it should be renamed to make way for the latest run. + self.rel_path_with_ts = self.rel_path / 'latest' # Process overrides before substituting wildcards self._process_overrides() @@ -141,15 +173,21 @@ def __init__(self, flow_cfg_file, hjson_data, args, mk_config): # _expand and add the code at the start. self._expand() - # Construct the path variables after variable expansion. - self.results_dir = (Path(self.scratch_base_path) / "reports" / - self.rel_path / "latest") - self.results_page = (self.results_dir / self.results_html_name) - - tmp_path = (self.results_server + "/" + self.rel_path + - "/latest/" + self.results_html_name) - self.results_server_page = self.results_server_prefix + tmp_path - self.results_server_url = "https://" + tmp_path + # After wildcards have been expanded, we can now construct the (absolute) path variables. + self.results_root = Path(self.scratch_base_path) / "reports" + self.results_dir = self.results_root / self.rel_path_with_ts # Unique local results dir for this run + self.md_report_path = self.results_dir / self.results_md_name + self.html_report_path = self.results_dir / self.results_html_name + self.json_report_path = self.results_dir / self.results_json_name + self.results_page = self.html_report_path + + # Results server paths + results_server_html_report_rel_path = \ + self.results_server_fqdn / self.rel_path_with_ts / self.results_html_name + self.results_server_page = \ + f"{self.results_server_scheme}{results_server_html_report_rel_path}" + self.results_server_url = \ + f"{self.results_server_url_scheme}{results_server_html_report_rel_path}" # Run any final checks self._post_init() @@ -378,63 +416,79 @@ def create_deploy_objects(self): for item in self.cfgs: item._create_deploy_objects() - def deploy_objects(self): + def deploy_objects(self) -> dict: '''Public facing API for deploying all available objects. Runs each job and returns a map from item to status. ''' - deploy = [] - for item in self.cfgs: - deploy.extend(item.deploy) - if not deploy: + # Form a single list of all deployable objects from all nested cfgs + deploy_items = list(itertools.chain.from_iterable([cfg.deploy for cfg in self.cfgs])) + if not deploy_items: log.error("Nothing to run!") sys.exit(1) - return Scheduler(deploy, get_launcher_cls(), self.interactive).run() + # Schedule and run all objects. Return a results object when we complete. + return Scheduler(deploy_items, get_launcher_cls(), self.interactive).run() - def _gen_results(self, results): + + def _gen_json_results(self) -> str: + return "" + + + def _gen_md_results(self) -> str: + return "" + + + def _gen_results(self) -> None: ''' The function is called after the flow has completed. It collates the status of all run targets and generates a dict. It parses the log to identify the errors, warnings and failures as applicable. It also prints the full list of failures for debug / triage to the final report, which is in markdown format. - - results should be a dictionary mapping deployed item to result. ''' - return - def gen_results(self, results): + # The base class flow always generates results in a markdown format. + self.results_md = self._gen_md_results() + + # Post-process the markdown results into a HTML report. + self.results_html = md_results_to_html(self.results_title, self.css_file, self.results_md) + + + def gen_results(self, results: dict): '''Public facing API for _gen_results(). - results should be a dictionary mapping deployed item to result. + Args: + results: A dictionary mapping deployed item to result. ''' for item in self.cfgs: - json_str = (item._gen_json_results(results) - if hasattr(item, '_gen_json_results') - else None) - result = item._gen_results(results) - log.info("[results]: [%s]:\n%s\n", item.name, result) - log.info("[scratch_path]: [%s] [%s]", item.name, item.scratch_path) - item.write_results(self.results_html_name, item.results_md, - json_str) - log.log(VERBOSE, "[report]: [%s] [%s/report.html]", item.name, - item.results_dir) + # Give each cfg a reference to the results returned by the deploy steps + item.results = results + + # Analyze and generate result data + item._gen_results() self.errors_seen |= item.errors_seen + # Print some useful results data to the console + log.info("[scratch_path]: [%s] [%s]", item.name, item.scratch_path) + log.info("[results(md)]: [%s]:\n%s\n", item.name, item.results_md) + log.info("[report]: [%s] [%s]", item.name, item.results_page) + + # Now write the result files to disk + item.write_results() + if self.is_primary_cfg: self.gen_results_summary() - self.write_results(self.results_html_name, - self.results_summary_md) + self.write_results() def gen_results_summary(self): '''Public facing API to generate summary results for each IP/cfg file ''' return - def write_results(self, html_filename, text_md, json_str=None): + def write_results(self) -> None: """Write results to files. This function converts text_md to HTML and writes the result to a file @@ -449,22 +503,26 @@ def write_results(self, html_filename, text_md, json_str=None): mk_path(self.results_dir) # Write results to the report area. - with open(self.results_dir / html_filename, "w") as f: - f.write( - md_results_to_html(self.results_title, self.css_file, text_md)) + self.md_report_path.write_text(self.results_md) + self.html_report_path.write_text(self.results_html) + if self.results_json is not None: + self.json_report_path.write_text(self.results_json) - if json_str is not None: - filename = Path(html_filename).with_suffix('.json') - with open(self.results_dir / filename, "w") as f: - f.write(json_str) + def _get_md_relative_link_html_report(self, from_path=Path(), to_path=Path(), link_text=''): + """Create a relative markdown link. - def _get_results_page_link(self, relative_to, link_text=''): - """Create a relative markdown link to the results page.""" + The default link is from the results directory to the html report in the results directory + """ link_text = self.name.upper() if not link_text else link_text - relative_link = os.path.relpath(self.results_page, - relative_to) - return "[%s](%s)" % (link_text, relative_link) + + if not from_path: + from_path = self.results_dir + if not to_path: + to_path = self.results_page + relative_link = to_path.relative_to(from_path) + + return f"[{link_text}]({relative_link})" def _publish_results(self, results_server: ResultsServer): '''Publish results to the opentitan web server. @@ -487,17 +545,19 @@ def _publish_results(self, results_server: ResultsServer): # creation time. # Try to get the creation time of any existing "latest/report.html" - latest_dir = '{}/latest'.format(self.rel_path) - latest_report_path = '{}/report.html'.format(latest_dir) - old_results_time = results_server.get_creation_time(latest_report_path) + latest_dir = self.rel_path / "latest" + latest_report_path = latest_dir / "report.html" + old_results_time = results_server.get_creation_time(latest_report_path) if old_results_time is not None: # If there is indeed a creation time, we will need to move the # "latest" directory to a path based on that time. old_results_ts = old_results_time.strftime(tf) - backup_dir = '{}/{}'.format(self.rel_path, old_results_ts) + backup_dir = f"{self.rel_path} / {old_results_ts}" - results_server.mv(latest_dir, backup_dir) + results_server.mv( + from_path=latest_dir, + to_path=backup_dir) # Do an ls in the results root dir to check what directories exist. If # something goes wrong then continue, behaving as if there were none. @@ -525,44 +585,48 @@ def _publish_results(self, results_server: ResultsServer): existing_basenames.sort(reverse=True) history_txt = "\n## Past Results\n" - history_txt += "- [Latest](../latest/" + self.results_html_name + ")\n" + history_txt += f"- [Latest](../latest/{self.results_html_name})\n" for existing_basename in existing_basenames[:max_old_page_links]: - relative_url = '../{}/{}'.format(existing_basename, - self.results_html_name) - history_txt += '- [{}]({})\n'.format(existing_basename, - relative_url) + relative_url = f"../{existing_basename}/{self.results_html_name}" + history_txt += f"- [{existing_basename}]({relative_url})\n" # Append the history to the results. publish_results_md = self.publish_results_md or self.results_md publish_results_md = publish_results_md + history_txt - # Export any results dictionary to json - suffixes = ['html'] - json_str = None - if hasattr(self, 'results_dict'): - suffixes.append('json') - json_str = json.dumps(self.results_dict) - # Export our markdown page to HTML and dump the json to a local file. - # These are called publish.html and publish.json locally, but we'll - # rename them as part of the upload. - self.write_results("publish.html", publish_results_md, json_str) + # Finally, copy our local files over to the results server + log.info("Publishing results to %s", self.results_server_url) - html_name_no_suffix = self.results_html_name.split('.', 1)[0] - dst_no_suffix = '{}/latest/{}'.format(self.rel_path, - html_name_no_suffix) + def get_srcdst_pair(report_path: Path): + """Return a src/dst tuple defining an upload operation to the results server. + + The 'dst' path is derived by removing the root/parent components of the local + path that are specific to the build environment. The remaining path fragment + is of the form 'self.rel_path_with_ts / report_name'. + e.g. + src : /home/otfan1/projects/opentitan/scratch/master/reports/hw/ip/i2c/dv/latest/report.html + dst : hw/ip/i2c/dv/latest/report.html + + This remaining path fragment becomes an anchored/rooted path on the results server. + e.g. + https://reports.opentitan.org/hw/ip/i2c/dv/latest/report.html + """ + return (report_path, report_path.relative_to(self.results_root)) + + # Always copy the HTML report, and also copy the .json report if it exists. + src_reports = [ self.html_report_path ] + if self.results_dict: + src_reports.append(self.json_report_path) + + for (src, dst) in (get_srcdst_pair(src) for src in src_reports): + results_server.upload(str(src), str(dst)) - # Now copy our local files over to the server - log.info("Publishing results to %s", self.results_server_url) - for suffix in suffixes: - src = "{}/publish.{}".format(self.results_dir, suffix) - dst = "{}.{}".format(dst_no_suffix, suffix) - results_server.upload(src, dst) def publish_results(self): """Publish these results to the opentitan web server.""" try: - server_handle = ResultsServer(self.results_server) + server_handle = ResultsServer(self.results_server_fqdn) except NoGCPError: # We failed to create a results server object at all, so we're not going to be able # to publish any results right now. @@ -576,20 +640,14 @@ def publish_results(self): if self.is_primary_cfg: self.publish_results_summary(server_handle) - # Trigger a rebuild of the site/docs which may pull new data from - # the published results. - self.rebuild_site() - def publish_results_summary(self, results_server: ResultsServer): - '''Public facing API for publishing md format results to the opentitan - web server. - ''' + '''Public facing API for publishing the HTML-formatted results report to the opentitan web server.''' + # Publish the results page. - log.info("Publishing results summary to %s", self.results_server_url) + log.info(f"Publishing results summary to {self.results_server_url}") - latest_dir = '{}/latest'.format(self.rel_path) - latest_report_path = '{}/report.html'.format(latest_dir) - results_server.upload(self.results_page, latest_report_path) + latest_report_path = self.rel_path_with_ts / self.results_html_name + results_server.upload(str(self.results_page), str(latest_report_path)) def rebuild_site(self): '''Trigger a rebuild of the opentitan.org site using a Cloud Build trigger. @@ -608,9 +666,9 @@ def rebuild_site(self): project = 'gold-hybrid-255313' trigger_name = 'site-builder-manual' - cmd = f"gcloud beta --project {project} builds triggers run {trigger_name}".split(' ') + cmd = f"gcloud beta --project {project} builds triggers run {trigger_name}" try: - cmd_output = subprocess.run(args=cmd, + cmd_output = subprocess.run(args=cmd.split(' '), shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) diff --git a/util/dvsim/LintCfg.py b/util/dvsim/LintCfg.py index ced749083a99a7..3defc1221ab3bb 100644 --- a/util/dvsim/LintCfg.py +++ b/util/dvsim/LintCfg.py @@ -76,8 +76,7 @@ def gen_results_summary(self): keys = self.totals.get_keys(self.report_severities) for cfg in self.cfgs: - name_with_link = cfg._get_results_page_link( - self.results_dir) + name_with_link = cfg._get_md_relative_link_html_report() row = [name_with_link] row += cfg.result_summary.get_counts_md(keys) diff --git a/util/dvsim/dvsim.py b/util/dvsim/dvsim.py index 38765a2f8181e6..cc44a75a49148c 100755 --- a/util/dvsim/dvsim.py +++ b/util/dvsim/dvsim.py @@ -758,6 +758,8 @@ def main(): if args.items: # Create deploy objects. cfg.create_deploy_objects() + + # Run the deploy objects results = cfg.deploy_objects() # Generate results. @@ -765,8 +767,13 @@ def main(): # Publish results if args.publish: + # Publish the generated results to the OpenTitan results server cfg.publish_results() + # Trigger a rebuild of the site/docs which may pull new data from + # the published results. + cfg.rebuild_site() + else: log.error("Nothing to run!") sys.exit(1)