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

Ask to install RISCOF before running arch-test locally #507

Merged
merged 1 commit into from
Oct 28, 2024

Conversation

ChinYikMing
Copy link
Collaborator

A specific version of RISCOF is used in CI, so requesting for that version as well.

@ChinYikMing ChinYikMing requested a review from vacantron October 27, 2024 16:56
@jserv jserv added this to the release-2024.2 milestone Oct 28, 2024
@@ -1,3 +1,9 @@
riscof-check:
$(Q)if [ "$(shell pip show riscof 2>&1 | head -n 1 | cut -d' ' -f1)" = "WARNING:" ]; then \
$(PRINTF) "Run 'pip3 install git+https://github.com/riscv/riscof.git@d38859f85fe407bcacddd2efcd355ada4683aee4' to install RISCOF\n"; \
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this workaround still making sense?

Copy link
Collaborator Author

@ChinYikMing ChinYikMing Oct 28, 2024

Choose a reason for hiding this comment

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

Is this workaround still making sense?

This may not be the solution for issue #506, but if someone attempts to run make arch-test instead of executing the script ".ci/riscv-tests.sh", the target will bail because riscof is not the default Python module. This alert can easily clarify any confusion.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it. Mention the above rationale in git commit messages.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

If someone attempts to run make arch-test instead of executing the
script ".ci/riscv-tests.sh", the target will bail because riscof
is not the default Python module. This alert can easily clarify
any confusion.

A specific version of RISCOF is used in CI, so requesting for that
version as well.
@ChinYikMing ChinYikMing force-pushed the feat/ask-to-install-riscof branch from def6c3f to f39af30 Compare October 28, 2024 01:39
@jserv jserv merged commit 5e67220 into sysprog21:master Oct 28, 2024
8 checks passed
@jserv
Copy link
Contributor

jserv commented Oct 28, 2024

Thank @ChinYikMing for contributing!

@ChinYikMing ChinYikMing deleted the feat/ask-to-install-riscof branch October 28, 2024 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants