Skip to content

Commit 8d8181e

Browse files
committed
fix: git commit and SimCfg revision info
This is still a bit messy, because we do not have ideal abstractions in place for the cfgs/flows. It might also be nice to separate out the revision info from the IP Meta even more in the future - some of the abstractions here still feel a bit arbitrary. This corrects a few issues with regards to current git commit / revision handling: - Report the short commit hash returned by git, not just an arbitrarily truncated hash. - The deploy commit is now listed correctly, instead of picking up the sim_cfg "revision" string which is generally not the same as the commit. - While the commit hash, branch and URL are used alternatively, the overriding "revision" string from the expanded SimCfg is now also picked up into IP metadata right now. This will need to be surfaced for Markdown reporting later. It currently isn't used in the HTML report, but probably should be later. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
1 parent 7022421 commit 8d8181e

6 files changed

Lines changed: 22 additions & 9 deletions

File tree

src/dvsim/job/deploy.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,11 @@ def get_job_spec(self) -> "JobSpec":
117117
block=IPMeta(
118118
name=self.sim_cfg.name,
119119
variant=self.sim_cfg.variant,
120-
commit=self.sim_cfg.revision,
120+
commit=self.sim_cfg.commit,
121+
commit_short=self.sim_cfg.commit_short,
121122
branch=self.sim_cfg.branch,
122123
url="",
124+
revision_info=self.sim_cfg.revision,
123125
),
124126
tool=ToolMeta(
125127
name=self.sim_cfg.tool,

src/dvsim/report/data.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,14 @@ class IPMeta(BaseModel):
2424

2525
commit: str
2626
"""Git commit sha of the IP the tests are run against."""
27+
commit_short: str
28+
"""Shortened Git commit sha of the IP the tests are run against."""
2729
branch: str
2830
"""Git branch"""
2931
url: str
3032
"""URL to where the IP can be found in git (e.g. github)."""
33+
revision_info: str | None
34+
"""Optional revision info string to use for custom info instead of the above fields."""
3135

3236
def variant_name(self, sep: str = "_") -> str:
3337
"""Name suffixed with the variant for disambiguation."""

src/dvsim/sim/flow.py

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,11 @@ def __init__(self, flow_cfg_file, hjson_data, args, mk_config) -> None:
168168

169169
super().__init__(flow_cfg_file, hjson_data, args, mk_config)
170170

171+
# After initialisation & expansion, save some useful revision metadata
172+
proj_root = Path(self.proj_root)
173+
self.commit = git_commit_hash(path=proj_root, short=False)
174+
self.commit_short = git_commit_hash(path=proj_root, short=True)
175+
171176
def _expand(self) -> None:
172177
# Choose a wave format now. Note that this has to happen after parsing
173178
# the configuration format because our choice might depend on the
@@ -603,7 +608,6 @@ def gen_results(self, results: Sequence[CompletedJobStatus]) -> None:
603608
"""
604609
repo_root = Path(self.proj_root)
605610
reports_dir = Path(self.scratch_base_path) / "reports"
606-
commit = git_commit_hash(path=repo_root)
607611
url = git_https_url_with_commit(path=repo_root)
608612
build_seed = self.build_seed if not self.run_only else None
609613

@@ -625,7 +629,6 @@ def gen_results(self, results: Sequence[CompletedJobStatus]) -> None:
625629

626630
flow_results: SimFlowResults = item._gen_json_results(
627631
run_results=item_results,
628-
commit=commit,
629632
url=url,
630633
)
631634

@@ -651,9 +654,11 @@ def gen_results(self, results: Sequence[CompletedJobStatus]) -> None:
651654
top = IPMeta(
652655
name=self.name,
653656
variant=self.variant,
654-
commit=commit,
657+
commit=self.commit,
658+
commit_short=self.commit_short,
655659
branch=self.branch,
656660
url=url,
661+
revision_info=self.revision,
657662
)
658663

659664
results_summary = SimResultsSummary(
@@ -674,14 +679,12 @@ def gen_results(self, results: Sequence[CompletedJobStatus]) -> None:
674679
def _gen_json_results(
675680
self,
676681
run_results: Sequence[CompletedJobStatus],
677-
commit: str,
678682
url: str,
679683
) -> SimFlowResults:
680684
"""Generate structured SimFlowResults from simulation run data.
681685
682686
Args:
683687
run_results: completed job status.
684-
commit: git commit Hash
685688
url: for the IP source
686689
687690
Returns:
@@ -698,9 +701,11 @@ def _gen_json_results(
698701
block = IPMeta(
699702
name=self.name.lower(),
700703
variant=(self.variant or "").lower() or None,
701-
commit=commit,
704+
commit=self.commit,
705+
commit_short=self.commit_short,
702706
branch=self.branch or "",
703707
url=url,
708+
revision_info=self.revision,
704709
)
705710
tool = ToolMeta(name=self.tool.lower(), version="unknown")
706711

src/dvsim/templates/reports/block_report.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ <h2>Simulation Results: {{ block.variant_name(sep='/') }}</h2>
5151
{% endif %}
5252
<a class="badge text-bg-secondary link-underline link-underline-opacity-0"
5353
href="{{ block.url }}">
54-
sha: {{ block.commit[:7] }}
54+
sha: {{ block.commit_short }}
5555
</a>
5656
<a class="badge text-bg-secondary link-underline link-underline-opacity-0"
5757
href="{{ block.variant_name() }}.json">json</a>

src/dvsim/templates/reports/summary_report.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ <h2>{{ title }}</h2>
5151
{% endif %}
5252
<a class="badge text-bg-secondary link-underline link-underline-opacity-0"
5353
href="{{ meta_info.url }}">
54-
sha: {{ meta_info.commit[:7] }}
54+
sha: {{ meta_info.commit_short }}
5555
</a>
5656
<a class="badge text-bg-secondary link-underline link-underline-opacity-0"
5757
href="index.json">json</a>

tests/test_scheduler.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,10 @@ def ip_meta_factory(**overrides: str | None) -> IPMeta:
247247
"name": "test_ip",
248248
"variant": None,
249249
"commit": "test_commit",
250+
"commit_short": "test",
250251
"branch": "test_branch",
251252
"url": "test_url",
253+
"revision_info": None,
252254
}
253255
meta.update(overrides)
254256
return IPMeta(**meta)

0 commit comments

Comments
 (0)