-
Notifications
You must be signed in to change notification settings - Fork 36
chore: add check_env.py to verify developer toolchain. #97
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
base: master
Are you sure you want to change the base?
chore: add check_env.py to verify developer toolchain. #97
Conversation
📝 WalkthroughWalkthroughA new utility module Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@check_env.py`:
- Line 2: Remove the unused import by deleting the top-level "import sys"
statement in check_env.py so there are no unused module imports; ensure no other
code in check_env.py references sys before removing it (if it does, replace
usage appropriately or re-add a necessary import).
🧹 Nitpick comments (2)
check_env.py (2)
8-12: Catch specific exceptions and add a timeout.The bare
Exceptioncatch is overly broad and can mask unexpected errors. Additionally, the subprocess call lacks a timeout, which could cause the script to hang indefinitely if a tool doesn't respond.Proposed fix
try: - subprocess.check_output([tool, "--version"], stderr=subprocess.STDOUT) + subprocess.check_output([tool, "--version"], stderr=subprocess.STDOUT, timeout=10) print(f"? {tool} is installed.") - except Exception: + except (FileNotFoundError, subprocess.CalledProcessError, subprocess.TimeoutExpired): print(f"? {tool} NOT found. Please install it to build NucleusRV.")
4-15: Consider returning a status for CI/scripting usage.For use in CI pipelines or scripts, it would be helpful if the function returned a boolean or the script exited with a non-zero code when tools are missing.
Example implementation
def check_toolchain(): tools = ["riscv64-unknown-elf-gcc", "verilator", "sbt"] print("?? Checking MERL Development Environment...") + all_found = True for tool in tools: try: - subprocess.check_output([tool, "--version"], stderr=subprocess.STDOUT) + subprocess.check_output([tool, "--version"], stderr=subprocess.STDOUT, timeout=10) print(f"? {tool} is installed.") - except Exception: + except (FileNotFoundError, subprocess.CalledProcessError, subprocess.TimeoutExpired): print(f"? {tool} NOT found. Please install it to build NucleusRV.") + all_found = False + return all_found if __name__ == "__main__": - check_toolchain() + import sys + if not check_toolchain(): + sys.exit(1)
|
Hi mentors! I noticed that the Read the Docs build is failing due to a missing configuration file in the master branch. I've updated my PR to ensure my contribution doesn't interfere with the documentation build. I'm very interested in the NucleusRV core verification for GSoC 2026 and look forward to your feedback! |
Hi mentors! Preparing for GSoC 2026. I noticed that setting up the RISC-V toolchain (gcc, verilator, sbt) can be a hurdle for new contributors. I've added a simple check_env.py script that automatically verifies if these tools are installed and in the system path, making the onboarding process for the NucleusRV core much smoother.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.