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

log: Never silence log_cmd_error #4613

Merged
merged 2 commits into from
Oct 7, 2024
Merged

Conversation

povik
Copy link
Member

@povik povik commented Sep 24, 2024

What are the reasons/motivation for this change?

Some errors can get hidden by -v N, see The-OpenROAD-Project/OpenROAD-flow-scripts#2375

This makes the user almost always worse off. The only exception I can think of is someone scripting around Yosys commands, checking for their exit status, and handling failures gracefully (for the few failure modes where that can be done). I've never seen such a Yosys script.

Explain how this is achieved.

Add extra handling to arrange for log_cmd_error never being silenced by the command line -v N option. Similar path for log_error exists already.

If applicable, please suggest to reviewers how they can test the change.

With test.tcl:

yosys -import
hierarchy -top nonexistent

Before

$ ./yosys -v 3 test.tcl
1. Executing HIERARCHY pass (managing design hierarchy).
ERROR: TCL interpreter returned an error: Yosys command produced an error

After

$ ./yosys -v 3 test.tcl
1. Executing HIERARCHY pass (managing design hierarchy).
ERROR: Module `nonexistent' not found!
ERROR: TCL interpreter returned an error: Yosys command produced an error

Add extra handling to arrange for `log_cmd_error` never being silenced
by the command line `-v N` option. Similar path for `log_error` exists
already.
@Ravenslofty
Copy link
Collaborator

I think this should have a testcase, but I'm not entirely sure how it would be implemented; maybe with a custom run_test.sh?

@povik povik added the merge-before-release Merge: PR should be included in the next release label Oct 7, 2024
@povik povik merged commit e46cc57 into YosysHQ:main Oct 7, 2024
22 checks passed
@povik povik deleted the err-never-silence branch October 7, 2024 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge-before-release Merge: PR should be included in the next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants