Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork937
usesys.platform == "cygwin" to figure out when we are using cygwin#2027
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
only this PR or#2026 should be merged |
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.
Pull Request Overview
This PR simplifies the detection of Cygwin Git by replacing the former multi-step logic with a concise check that relies on sys.platform.
- Removed deprecated functions and caching logic for Cygwin detection.
- Updated the Git command interface to directly use sys.platform and the Git executable's existence.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| git/util.py | Removed the py_where and _is_cygwin_git functions along with unused imports. |
| git/cmd.py | Updated the is_cygwin method to use a simplified sys.platform check. |
| @@ -661,7 +660,7 @@ def refresh(cls, path: Union[None, PathLike] = None) -> bool: | |||
| @classmethod | |||
| def is_cygwin(cls) -> bool: | |||
| returnis_cygwin_git(cls.GIT_PYTHON_GIT_EXECUTABLE) | |||
| return cls.GIT_PYTHON_GIT_EXECUTABLE is not None and sys.platform == "cygwin" | |||
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.
Consider adding an inline comment explaining that sys.platform being 'cygwin' reliably indicates a Cygwin environment per Python 3.7+, to improve clarity for future maintainers.
Copilot uses AI. Check for mistakes.
sys.platform == "cygwin" to figure out when we are using cygwin
The current logic for detecting cygwin is:
sys.platform == "win32", then it's not cygwinunameexecutable in the same folder as the git executable, and if so, does the output of that command include"CYGWIN"? if so, then it's cygwinIn thepython 3.7 docs for sys.platform, Cygwin systems have
sys.platform == "cygwin". Since Python 3.7 is the oldest version still supported by GitPython, it stands to reason that we can rely on that being true for all supported Python versions.The logic I propose is:
sys.platform == 'cygwin', then it's cygwin gitThis is simple enough for a single expression, so I replaced the body of the
is_cygwinfunction with that expression and removedis_cygwin_gitand friends completely.I don't know if there's such a thing as
sys.platform == 'cygwin'and thegitnot being cygwingit, but if there is, I don't think the existing code deals with that anyway, so this seems to return the same result and would fail in the same way.