[Bugfix] Bug fix when git command is not installed #823
Conversation
WalkthroughThe change updates get_git_commit_id in tilelang/version.py to also catch FileNotFoundError when invoking git, handling environments without git. Behavior remains: return HEAD commit hash when available; otherwise None. Version string composition that may include the commit id is unaffected. No public signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant Version as version.py
participant Git as git CLI
App->>Version: get_git_commit_id()
alt git available
Version->>Git: git rev-parse HEAD
Git-->>Version: commit hash
Version-->>App: commit hash
else git errors
Note over Version: Handles SubprocessError<br/>or FileNotFoundError
Version-->>App: None
end
App->>Version: Compose __version__
Note over App,Version: Append commit id if present
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
|
👋 Hi! Thank you for contributing to the TileLang project. Please remember to run We appreciate you taking this step! Our team will review your contribution, and we look forward to your awesome work! 🚀 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @LeiWang1999, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request addresses a bug in the tilelang/version.py module by enhancing the error handling within the get_git_commit_id function. Previously, if the git command was not present on the system, the function would raise a FileNotFoundError, causing unexpected behavior. The change ensures that this specific error, along with other subprocess.SubprocessError exceptions, is gracefully caught, allowing the function to return None as intended when git is unavailable, thus improving the robustness of the version retrieval mechanism.
Highlights
- Bug Fix: Resolved an issue where the
get_git_commit_idfunction would fail if thegitcommand-line tool was not installed on the system. - Error Handling: Enhanced error handling in
tilelang/version.pyto explicitly catchFileNotFoundErrorwhen attempting to execute git commands, ensuring graceful degradation.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request correctly fixes a crash that occurs when the git command is not installed by handling FileNotFoundError in tilelang/version.py. The fix is effective. However, a similar issue exists in setup.py within the get_tilelang_version function (lines 168-173) which should also be addressed to prevent the setup script from failing. I have also added a review comment with a suggestion to make the exception handling more specific, which would be a good improvement for both files.
| encoding='utf-8').strip() | ||
| except subprocess.SubprocessError: | ||
| # FileNotFoundError is raised when git is not installed | ||
| except (subprocess.SubprocessError, FileNotFoundError): |
There was a problem hiding this comment.
While this change correctly fixes the bug, it's a good practice to catch more specific exceptions. subprocess.SubprocessError is a general base class. The most likely error here, besides FileNotFoundError, is subprocess.CalledProcessError, which is raised when the command returns a non-zero exit code (e.g., if the directory is not a git repository). Using subprocess.CalledProcessError makes the error handling more precise and the code's intent clearer.
| except (subprocess.SubprocessError, FileNotFoundError): | |
| except (subprocess.CalledProcessError, FileNotFoundError): |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tilelang/version.py (2)
28-37: Tighten exception scope and use Optional typing (nit).Catching subprocess.SubprocessError is a bit broad. Prefer CalledProcessError to avoid masking unrelated subprocess issues, and use Optional[str] for clarity.
-from typing import Union +from typing import Optional -def get_git_commit_id() -> Union[str, None]: - """Get the current git commit hash by running git in the current file's directory.""" +def get_git_commit_id() -> Optional[str]: + """Return current git commit hash, or None if git/HEAD unavailable.""" try: return subprocess.check_output(['git', 'rev-parse', 'HEAD'], cwd=os.path.dirname(os.path.abspath(__file__)), stderr=subprocess.DEVNULL, encoding='utf-8').strip() - # FileNotFoundError is raised when git is not installed - except (subprocess.SubprocessError, FileNotFoundError): + # FileNotFoundError is raised when git is not installed + except (subprocess.CalledProcessError, FileNotFoundError): return None
44-45: Consider shortening the embedded commit hash (optional).Long hashes can bloat version strings; PEP 440 local versions work fine with a short hash.
-if "+" not in __version__ and (commit_id := get_git_commit_id()): - __version__ = f"{__version__}+{commit_id}" +if "+" not in __version__ and (commit_id := get_git_commit_id()): + __version__ = f"{__version__}+{commit_id[:12]}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tilelang/version.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: format-check
🔇 Additional comments (1)
tilelang/version.py (1)
35-36: Handle missing git gracefully — LGTM.Catching FileNotFoundError fixes the crash when git isn’t installed. Nice, minimal change with clear comment.
Consider adding a small unit test that mocks subprocess.check_output to raise FileNotFoundError and asserts get_git_commit_id() returns None.
1 participant
Summary by CodeRabbit