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

Overhaul noqa directives #1760

Merged
merged 3 commits into from
Dec 9, 2023
Merged

Overhaul noqa directives #1760

merged 3 commits into from
Dec 9, 2023

Conversation

EliahKagan
Copy link
Contributor

@EliahKagan EliahKagan commented Dec 9, 2023

This includes a few small style improvements I had missed in previous PRs, but consist mostly of improvements to how and where noqa directives are used, and code changes (intended to be non-breaking and uncontroversial) to go along with that.

The directives are made more precise. They always give specific errors/warnings to suppress, both where that had been intended but idiosyncrasies of the syntax caused them to be broader, and where nothing had been given before. All noqa directives are now statement-level rather than file-level. Some of those that were no longer needed are removed with no other associated change. Some others are removed together with minimal changes to the code they complained about or to other noqa directives that had been ineffective (such as due to their placement).

The commit messages have some more information about the details of the changes.

This performs an exact type comparison with "is" instead of "==".
Most (maybe all?) of the rest of the places where "==" was used
have been changed previously, but this on fell through the cracks.
Like the None object, type objects use reference-based equality
comparison, and the idiomatic ways to check against them are "is"
for exactly comparisons and isinstance/issubclass otherwise.
- Slightly improve grammar ("it's" -> "its").
- Add a missing space after a "." (between sentences).
- Use more consistent hard wrap in a place where 88 was intended.
- Remove some noqa directives that are not needed at all anymore.
  But there may still be some others I did not find.

- Fix the code and remove the noqa direcive in places where the
  change is very simple and does not affect the interface (e.g.,
  what could be taken to be public or not, what symbols are
  available and what they refer to, etc.), the change makes the
  code more idiomatic or clearer, and it fixes the flake8 complaint
  completely.

- Turn file-level noqa directives into statement-level ones. A noqa
  directive at file level, even if it lists one or more specific
  error/warning numbers, actually suppresses flake8 for the entire
  file. So the main benefit here is to enable other warnings. But
  this also makes it clear what is producing the ones we suppress,
  which makes it easier to know what is intentional and how it may
  be reasonable to change it in the future.

- Move noqa directives at the end of multi-line imports to the top
  line, where flake8 honors them, and eliminate redundant
  directives that were added to work around their ineffectiveness.
  (This is not really separate from the above point, since this is
  often what allowed file-level directives to be removed.)

- Specify the errors/warnings each "noqa" should suppress. That is,
  this turns every bare "noqa", aside from those eliminated as
  described above, into a "noqa" for one or more specific
  errors/warnings. This makes the intent clearer and avoids
  oversuppression.

- Add missing ":" between "noqa" and error/warning numbers. When
  these were absent, the intention was to suppress only the
  specifically listed errors/warnings, but the effect was that the
  listed numbers were ignored and *everything* was suppressed. In a
  couple of cases it was necessary to change the listed numbers to
  correct the list to what actually needed to be suppressed.

- Write every "noqa" in lower-case (not "NOQA"). There did not
  appear to be a systematic reason for the different casing, and
  having them all cased the same makes it easier to avoid mistakes
  when grepping for them.

- Where an import is unused and does not appear intended to be
  accesed from other modules, and is present only to support code
  in the same module that is commented out, but whose removal
  should be considered separately, comment out the statement or
  part of the statement that imports it as well, rather than
  writing a "noqa" for the unused import. This applies to only one
  file, git/objects/util.py.
@EliahKagan EliahKagan marked this pull request as ready for review December 9, 2023 00:38
Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks so much for your continued contributions, and for (nearly) single-handedly making GitPython more maintainable!

@Byron Byron merged commit a58a6be into gitpython-developers:main Dec 9, 2023
20 checks passed
@EliahKagan EliahKagan deleted the noqa branch December 9, 2023 19:06
@EliahKagan
Copy link
Contributor Author

EliahKagan commented Dec 12, 2023

Thanks so much for your continued contributions, and for (nearly) single-handedly making GitPython more maintainable!

Well, a couple of the broken line-level suppressions had also been added by me. But thanks!

More broadly, this is actually a case where ruff has already helped, even though I don't really have it working for this project and I'm not sure when I will.

More challenges than I expected have arisen in trying to migrate flake8 to ruff for GitPython that I think have to be solved to preserve some of the checks GitPython has been benefiting from, including that some of those checks are still considered to be in preview for Ruff and I want to make I understand the implications of preview = true, that ruff has to be compiled if installed on Cygwin but this always fails with the test-cygwin.yml runner is set up (and also it would be bad to make that workflow take even longer to accommodate a compilation), and Literal["foo"] is only special-cased to recognize "foo" as a literal string rather than a type name when Literal is imported from typing or typing_extensions, whereas GitPython imports it from one of its own modules, which conditionally imports it from one or the other of those modules based on the Python version (I'd rather not suppress the warning about accessing a name that was never defined).

But when experimenting with it, I did find that it warned about not recognizing some of the file-level suppressions that flake8 recognized but as something broader and quite different from what they intended. (I consider it a good thing about ruff that it did not recognize them!) I considered fixing them, but I decided that instead they could be removed, leading to this PR.

@Byron
Copy link
Member

Byron commented Dec 12, 2023

Thanks for sharing the details on how this PR came to be, it's exciting to hear some Rustuff will be happening.

I wonder if cygwin has to use ruff (or flake for that matter) at all. Will it make a difference given that the linter will probably work the same on any platform? But probably there is a misconception somewhere on my side.

@EliahKagan
Copy link
Contributor Author

Cygwin doesn't have to use ruff. This is a minor issue, just one that has to be figured out. ruff can be omitted as a test dependency altogether and just used via pre-commit, which is the situation with flake8. Or maybe it could be listed but omitted for platform == "cygwin". Or a test-minimal group could be created. There are probably other options.

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request Dec 21, 2023
This removes the "fmt: off" / "fmt: on" directives around the
`@pytest.mark.parametrize` decoration on test_blob_filter, and
reformats it with black, for consistency with other such
decorations.

The style used there, *if* it could be specified as a rule and thus
used without "fmt:" directives, may be nicer than how black formats
multi-line mark decorations. However, since that decoration was
written, there have been a number of other such decorations, which
have been in black style.

This also removes the only (or only remaining?) "fmt:" directive in
the codebase. As such, it should possibly have been done in gitpython-developers#1760.
lettuce-bot bot referenced this pull request in lettuce-financial/github-bot-signed-commit Jan 10, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [GitPython](https://github.com/gitpython-developers/GitPython) |
`==3.1.40` -> `==3.1.41` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/GitPython/3.1.41?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/GitPython/3.1.41?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/GitPython/3.1.40/3.1.41?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/GitPython/3.1.40/3.1.41?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>gitpython-developers/GitPython (GitPython)</summary>

###
[`v3.1.41`](https://github.com/gitpython-developers/GitPython/releases/tag/3.1.41):
- fix Windows security issue

[Compare
Source](https://github.com/gitpython-developers/GitPython/compare/3.1.40...3.1.41)

The details about the Windows security issue [can be found in this
advisory](https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-2mqj-m65w-jghx).

Special thanks go to
[@&#8203;EliahKagan](https://github.com/EliahKagan) who reported the
issue and fixed it in a single stroke, while being responsible for an
incredible amount of improvements that he contributed over the last
couple of months ❤️.

#### What's Changed

- Add `__all__` in git.exc by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1719](https://github.com/gitpython-developers/GitPython/pull/1719)
- Set submodule update cadence to weekly by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1721](https://github.com/gitpython-developers/GitPython/pull/1721)
- Never modify sys.path by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1720](https://github.com/gitpython-developers/GitPython/pull/1720)
- Bump git/ext/gitdb from `8ec2390` to `ec58b7e` by
[@&#8203;dependabot](https://github.com/dependabot) in
[https://github.com/gitpython-developers/GitPython/pull/1722](https://github.com/gitpython-developers/GitPython/pull/1722)
- Revise comments, docstrings, some messages, and a bit of code by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1725](https://github.com/gitpython-developers/GitPython/pull/1725)
- Use zero-argument super() by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1726](https://github.com/gitpython-developers/GitPython/pull/1726)
- Remove obsolete note in \_iter_packed_refs by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1727](https://github.com/gitpython-developers/GitPython/pull/1727)
- Reorganize test_util and make xfail marks precise by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1729](https://github.com/gitpython-developers/GitPython/pull/1729)
- Clarify license and make module top comments more consistent by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1730](https://github.com/gitpython-developers/GitPython/pull/1730)
- Deprecate compat.is\_<platform>, rewriting all uses by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1732](https://github.com/gitpython-developers/GitPython/pull/1732)
- Revise and restore some module docstrings by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1735](https://github.com/gitpython-developers/GitPython/pull/1735)
- Make the rmtree callback Windows-only by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1739](https://github.com/gitpython-developers/GitPython/pull/1739)
- List all non-passing tests in test summaries by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1740](https://github.com/gitpython-developers/GitPython/pull/1740)
- Document some minor subtleties in test_util.py by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1749](https://github.com/gitpython-developers/GitPython/pull/1749)
- Always read metadata files as UTF-8 in setup.py by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1748](https://github.com/gitpython-developers/GitPython/pull/1748)
- Test native Windows on CI by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1745](https://github.com/gitpython-developers/GitPython/pull/1745)
- Test macOS on CI by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1752](https://github.com/gitpython-developers/GitPython/pull/1752)
- Let close_fds be True on all platforms by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1753](https://github.com/gitpython-developers/GitPython/pull/1753)
- Fix IndexFile.from_tree on Windows by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1751](https://github.com/gitpython-developers/GitPython/pull/1751)
- Remove unused TASKKILL fallback in AutoInterrupt by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1754](https://github.com/gitpython-developers/GitPython/pull/1754)
- Don't return with operand when conceptually void by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1755](https://github.com/gitpython-developers/GitPython/pull/1755)
- Group .gitignore entries by purpose by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1758](https://github.com/gitpython-developers/GitPython/pull/1758)
- Adding dubious ownership handling by
[@&#8203;marioaag](https://github.com/marioaag) in
[https://github.com/gitpython-developers/GitPython/pull/1746](https://github.com/gitpython-developers/GitPython/pull/1746)
- Avoid brittle assumptions about preexisting temporary files in tests
by [@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1759](https://github.com/gitpython-developers/GitPython/pull/1759)
- Overhaul noqa directives by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1760](https://github.com/gitpython-developers/GitPython/pull/1760)
- Clarify some Git.execute kill_after_timeout limitations by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1761](https://github.com/gitpython-developers/GitPython/pull/1761)
- Bump actions/setup-python from 4 to 5 by
[@&#8203;dependabot](https://github.com/dependabot) in
[https://github.com/gitpython-developers/GitPython/pull/1763](https://github.com/gitpython-developers/GitPython/pull/1763)
- Don't install black on Cygwin by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1766](https://github.com/gitpython-developers/GitPython/pull/1766)
- Extract all "import gc" to module level by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1765](https://github.com/gitpython-developers/GitPython/pull/1765)
- Extract remaining local "import gc" to module level by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1768](https://github.com/gitpython-developers/GitPython/pull/1768)
- Replace xfail with gc.collect in TestSubmodule.test_rename by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1767](https://github.com/gitpython-developers/GitPython/pull/1767)
- Enable CodeQL by [@&#8203;EliahKagan](https://github.com/EliahKagan)
in
[https://github.com/gitpython-developers/GitPython/pull/1769](https://github.com/gitpython-developers/GitPython/pull/1769)
- Replace some uses of the deprecated mktemp function by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1770](https://github.com/gitpython-developers/GitPython/pull/1770)
- Bump github/codeql-action from 2 to 3 by
[@&#8203;dependabot](https://github.com/dependabot) in
[https://github.com/gitpython-developers/GitPython/pull/1773](https://github.com/gitpython-developers/GitPython/pull/1773)
- Run some Windows environment variable tests only on Windows by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1774](https://github.com/gitpython-developers/GitPython/pull/1774)
- Fix TemporaryFileSwap regression where file_path could not be Path by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1776](https://github.com/gitpython-developers/GitPython/pull/1776)
- Improve hooks tests by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1777](https://github.com/gitpython-developers/GitPython/pull/1777)
- Fix if items of Index is of type PathLike by
[@&#8203;stegm](https://github.com/stegm) in
[https://github.com/gitpython-developers/GitPython/pull/1778](https://github.com/gitpython-developers/GitPython/pull/1778)
- Better document IterableObj.iter_items and improve some subclasses by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1780](https://github.com/gitpython-developers/GitPython/pull/1780)
- Revert "Don't install black on Cygwin" by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1783](https://github.com/gitpython-developers/GitPython/pull/1783)
- Add missing pip in $PATH on Cygwin CI by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1784](https://github.com/gitpython-developers/GitPython/pull/1784)
- Shorten Iterable docstrings and put IterableObj first by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1785](https://github.com/gitpython-developers/GitPython/pull/1785)
- Fix incompletely revised Iterable/IterableObj docstrings by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1786](https://github.com/gitpython-developers/GitPython/pull/1786)
- Pre-deprecate setting Git.USE_SHELL by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1782](https://github.com/gitpython-developers/GitPython/pull/1782)
- Deprecate Git.USE_SHELL by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1787](https://github.com/gitpython-developers/GitPython/pull/1787)
- In handle_process_output don't forward finalizer result by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1788](https://github.com/gitpython-developers/GitPython/pull/1788)
- Fix mypy warning "Missing return statement" by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1789](https://github.com/gitpython-developers/GitPython/pull/1789)
- Fix two remaining Windows untrusted search path cases by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1792](https://github.com/gitpython-developers/GitPython/pull/1792)

#### New Contributors

- [@&#8203;marioaag](https://github.com/marioaag) made their first
contribution in
[https://github.com/gitpython-developers/GitPython/pull/1746](https://github.com/gitpython-developers/GitPython/pull/1746)
- [@&#8203;stegm](https://github.com/stegm) made their first
contribution in
[https://github.com/gitpython-developers/GitPython/pull/1778](https://github.com/gitpython-developers/GitPython/pull/1778)

**Full Changelog**:
gitpython-developers/GitPython@3.1.40...3.1.41

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/lettuce-financial/github-bot-signed-commit).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMjcuMCIsInVwZGF0ZWRJblZlciI6IjM3LjEyNy4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->
renovate bot referenced this pull request in allenporter/flux-local Jan 11, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [GitPython](https://github.com/gitpython-developers/GitPython) |
`==3.1.40` -> `==3.1.41` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/GitPython/3.1.41?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/GitPython/3.1.41?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/GitPython/3.1.40/3.1.41?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/GitPython/3.1.40/3.1.41?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>gitpython-developers/GitPython (GitPython)</summary>

###
[`v3.1.41`](https://github.com/gitpython-developers/GitPython/releases/tag/3.1.41):
- fix Windows security issue

[Compare
Source](https://github.com/gitpython-developers/GitPython/compare/3.1.40...3.1.41)

The details about the Windows security issue [can be found in this
advisory](https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-2mqj-m65w-jghx).

Special thanks go to
[@&#8203;EliahKagan](https://github.com/EliahKagan) who reported the
issue and fixed it in a single stroke, while being responsible for an
incredible amount of improvements that he contributed over the last
couple of months ❤️.

#### What's Changed

- Add `__all__` in git.exc by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1719](https://github.com/gitpython-developers/GitPython/pull/1719)
- Set submodule update cadence to weekly by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1721](https://github.com/gitpython-developers/GitPython/pull/1721)
- Never modify sys.path by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1720](https://github.com/gitpython-developers/GitPython/pull/1720)
- Bump git/ext/gitdb from `8ec2390` to `ec58b7e` by
[@&#8203;dependabot](https://github.com/dependabot) in
[https://github.com/gitpython-developers/GitPython/pull/1722](https://github.com/gitpython-developers/GitPython/pull/1722)
- Revise comments, docstrings, some messages, and a bit of code by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1725](https://github.com/gitpython-developers/GitPython/pull/1725)
- Use zero-argument super() by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1726](https://github.com/gitpython-developers/GitPython/pull/1726)
- Remove obsolete note in \_iter_packed_refs by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1727](https://github.com/gitpython-developers/GitPython/pull/1727)
- Reorganize test_util and make xfail marks precise by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1729](https://github.com/gitpython-developers/GitPython/pull/1729)
- Clarify license and make module top comments more consistent by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1730](https://github.com/gitpython-developers/GitPython/pull/1730)
- Deprecate compat.is\_<platform>, rewriting all uses by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1732](https://github.com/gitpython-developers/GitPython/pull/1732)
- Revise and restore some module docstrings by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1735](https://github.com/gitpython-developers/GitPython/pull/1735)
- Make the rmtree callback Windows-only by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1739](https://github.com/gitpython-developers/GitPython/pull/1739)
- List all non-passing tests in test summaries by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1740](https://github.com/gitpython-developers/GitPython/pull/1740)
- Document some minor subtleties in test_util.py by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1749](https://github.com/gitpython-developers/GitPython/pull/1749)
- Always read metadata files as UTF-8 in setup.py by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1748](https://github.com/gitpython-developers/GitPython/pull/1748)
- Test native Windows on CI by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1745](https://github.com/gitpython-developers/GitPython/pull/1745)
- Test macOS on CI by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1752](https://github.com/gitpython-developers/GitPython/pull/1752)
- Let close_fds be True on all platforms by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1753](https://github.com/gitpython-developers/GitPython/pull/1753)
- Fix IndexFile.from_tree on Windows by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1751](https://github.com/gitpython-developers/GitPython/pull/1751)
- Remove unused TASKKILL fallback in AutoInterrupt by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1754](https://github.com/gitpython-developers/GitPython/pull/1754)
- Don't return with operand when conceptually void by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1755](https://github.com/gitpython-developers/GitPython/pull/1755)
- Group .gitignore entries by purpose by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1758](https://github.com/gitpython-developers/GitPython/pull/1758)
- Adding dubious ownership handling by
[@&#8203;marioaag](https://github.com/marioaag) in
[https://github.com/gitpython-developers/GitPython/pull/1746](https://github.com/gitpython-developers/GitPython/pull/1746)
- Avoid brittle assumptions about preexisting temporary files in tests
by [@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1759](https://github.com/gitpython-developers/GitPython/pull/1759)
- Overhaul noqa directives by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1760](https://github.com/gitpython-developers/GitPython/pull/1760)
- Clarify some Git.execute kill_after_timeout limitations by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1761](https://github.com/gitpython-developers/GitPython/pull/1761)
- Bump actions/setup-python from 4 to 5 by
[@&#8203;dependabot](https://github.com/dependabot) in
[https://github.com/gitpython-developers/GitPython/pull/1763](https://github.com/gitpython-developers/GitPython/pull/1763)
- Don't install black on Cygwin by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1766](https://github.com/gitpython-developers/GitPython/pull/1766)
- Extract all "import gc" to module level by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1765](https://github.com/gitpython-developers/GitPython/pull/1765)
- Extract remaining local "import gc" to module level by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1768](https://github.com/gitpython-developers/GitPython/pull/1768)
- Replace xfail with gc.collect in TestSubmodule.test_rename by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1767](https://github.com/gitpython-developers/GitPython/pull/1767)
- Enable CodeQL by [@&#8203;EliahKagan](https://github.com/EliahKagan)
in
[https://github.com/gitpython-developers/GitPython/pull/1769](https://github.com/gitpython-developers/GitPython/pull/1769)
- Replace some uses of the deprecated mktemp function by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1770](https://github.com/gitpython-developers/GitPython/pull/1770)
- Bump github/codeql-action from 2 to 3 by
[@&#8203;dependabot](https://github.com/dependabot) in
[https://github.com/gitpython-developers/GitPython/pull/1773](https://github.com/gitpython-developers/GitPython/pull/1773)
- Run some Windows environment variable tests only on Windows by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1774](https://github.com/gitpython-developers/GitPython/pull/1774)
- Fix TemporaryFileSwap regression where file_path could not be Path by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1776](https://github.com/gitpython-developers/GitPython/pull/1776)
- Improve hooks tests by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1777](https://github.com/gitpython-developers/GitPython/pull/1777)
- Fix if items of Index is of type PathLike by
[@&#8203;stegm](https://github.com/stegm) in
[https://github.com/gitpython-developers/GitPython/pull/1778](https://github.com/gitpython-developers/GitPython/pull/1778)
- Better document IterableObj.iter_items and improve some subclasses by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1780](https://github.com/gitpython-developers/GitPython/pull/1780)
- Revert "Don't install black on Cygwin" by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1783](https://github.com/gitpython-developers/GitPython/pull/1783)
- Add missing pip in $PATH on Cygwin CI by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1784](https://github.com/gitpython-developers/GitPython/pull/1784)
- Shorten Iterable docstrings and put IterableObj first by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1785](https://github.com/gitpython-developers/GitPython/pull/1785)
- Fix incompletely revised Iterable/IterableObj docstrings by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1786](https://github.com/gitpython-developers/GitPython/pull/1786)
- Pre-deprecate setting Git.USE_SHELL by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1782](https://github.com/gitpython-developers/GitPython/pull/1782)
- Deprecate Git.USE_SHELL by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1787](https://github.com/gitpython-developers/GitPython/pull/1787)
- In handle_process_output don't forward finalizer result by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1788](https://github.com/gitpython-developers/GitPython/pull/1788)
- Fix mypy warning "Missing return statement" by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1789](https://github.com/gitpython-developers/GitPython/pull/1789)
- Fix two remaining Windows untrusted search path cases by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1792](https://github.com/gitpython-developers/GitPython/pull/1792)

#### New Contributors

- [@&#8203;marioaag](https://github.com/marioaag) made their first
contribution in
[https://github.com/gitpython-developers/GitPython/pull/1746](https://github.com/gitpython-developers/GitPython/pull/1746)
- [@&#8203;stegm](https://github.com/stegm) made their first
contribution in
[https://github.com/gitpython-developers/GitPython/pull/1778](https://github.com/gitpython-developers/GitPython/pull/1778)

**Full Changelog**:
gitpython-developers/GitPython@3.1.40...3.1.41

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/allenporter/flux-local).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMjcuMCIsInVwZGF0ZWRJblZlciI6IjM3LjEyNy4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
otc-zuul bot pushed a commit to opentelekomcloud-infra/eyes_on_docs that referenced this pull request Mar 6, 2024
Bump gitpython from 3.1.37 to 3.1.41

Bumps gitpython from 3.1.37 to 3.1.41.

Release notes
Sourced from gitpython's releases.

3.1.41 - fix Windows security issue
The details about the Windows security issue can be found in this advisory.
Special thanks go to @​EliahKagan who reported the issue and fixed it in a single stroke, while being responsible for an incredible amount of improvements that he contributed over the last couple of months ❤️.
What's Changed

Add __all__ in git.exc by @​EliahKagan in gitpython-developers/GitPython#1719
Set submodule update cadence to weekly by @​EliahKagan in gitpython-developers/GitPython#1721
Never modify sys.path by @​EliahKagan in gitpython-developers/GitPython#1720
Bump git/ext/gitdb from 8ec2390 to ec58b7e by @​dependabot in gitpython-developers/GitPython#1722
Revise comments, docstrings, some messages, and a bit of code by @​EliahKagan in gitpython-developers/GitPython#1725
Use zero-argument super() by @​EliahKagan in gitpython-developers/GitPython#1726
Remove obsolete note in _iter_packed_refs by @​EliahKagan in gitpython-developers/GitPython#1727
Reorganize test_util and make xfail marks precise by @​EliahKagan in gitpython-developers/GitPython#1729
Clarify license and make module top comments more consistent by @​EliahKagan in gitpython-developers/GitPython#1730
Deprecate compat.is_, rewriting all uses by @​EliahKagan in gitpython-developers/GitPython#1732
Revise and restore some module docstrings by @​EliahKagan in gitpython-developers/GitPython#1735
Make the rmtree callback Windows-only by @​EliahKagan in gitpython-developers/GitPython#1739
List all non-passing tests in test summaries by @​EliahKagan in gitpython-developers/GitPython#1740
Document some minor subtleties in test_util.py by @​EliahKagan in gitpython-developers/GitPython#1749
Always read metadata files as UTF-8 in setup.py by @​EliahKagan in gitpython-developers/GitPython#1748
Test native Windows on CI by @​EliahKagan in gitpython-developers/GitPython#1745
Test macOS on CI by @​EliahKagan in gitpython-developers/GitPython#1752
Let close_fds be True on all platforms by @​EliahKagan in gitpython-developers/GitPython#1753
Fix IndexFile.from_tree on Windows by @​EliahKagan in gitpython-developers/GitPython#1751
Remove unused TASKKILL fallback in AutoInterrupt by @​EliahKagan in gitpython-developers/GitPython#1754
Don't return with operand when conceptually void by @​EliahKagan in gitpython-developers/GitPython#1755
Group .gitignore entries by purpose by @​EliahKagan in gitpython-developers/GitPython#1758
Adding dubious ownership handling by @​marioaag in gitpython-developers/GitPython#1746
Avoid brittle assumptions about preexisting temporary files in tests by @​EliahKagan in gitpython-developers/GitPython#1759
Overhaul noqa directives by @​EliahKagan in gitpython-developers/GitPython#1760
Clarify some Git.execute kill_after_timeout limitations by @​EliahKagan in gitpython-developers/GitPython#1761
Bump actions/setup-python from 4 to 5 by @​dependabot in gitpython-developers/GitPython#1763
Don't install black on Cygwin by @​EliahKagan in gitpython-developers/GitPython#1766
Extract all "import gc" to module level by @​EliahKagan in gitpython-developers/GitPython#1765
Extract remaining local "import gc" to module level by @​EliahKagan in gitpython-developers/GitPython#1768
Replace xfail with gc.collect in TestSubmodule.test_rename by @​EliahKagan in gitpython-developers/GitPython#1767
Enable CodeQL by @​EliahKagan in gitpython-developers/GitPython#1769
Replace some uses of the deprecated mktemp function by @​EliahKagan in gitpython-developers/GitPython#1770
Bump github/codeql-action from 2 to 3 by @​dependabot in gitpython-developers/GitPython#1773
Run some Windows environment variable tests only on Windows by @​EliahKagan in gitpython-developers/GitPython#1774
Fix TemporaryFileSwap regression where file_path could not be Path by @​EliahKagan in gitpython-developers/GitPython#1776
Improve hooks tests by @​EliahKagan in gitpython-developers/GitPython#1777
Fix if items of Index is of type PathLike by @​stegm in gitpython-developers/GitPython#1778
Better document IterableObj.iter_items and improve some subclasses by @​EliahKagan in gitpython-developers/GitPython#1780
Revert "Don't install black on Cygwin" by @​EliahKagan in gitpython-developers/GitPython#1783
Add missing pip in $PATH on Cygwin CI by @​EliahKagan in gitpython-developers/GitPython#1784
Shorten Iterable docstrings and put IterableObj first by @​EliahKagan in gitpython-developers/GitPython#1785
Fix incompletely revised Iterable/IterableObj docstrings by @​EliahKagan in gitpython-developers/GitPython#1786
Pre-deprecate setting Git.USE_SHELL by @​EliahKagan in gitpython-developers/GitPython#1782



... (truncated)


Commits

f288738 bump patch level
ef3192c Merge pull request #1792 from EliahKagan/popen
1f3caa3 Further clarify comment in test_hook_uses_shell_not_from_cwd
3eb7c2a Move safer_popen from git.util to git.cmd
c551e91 Extract shared logic for using Popen safely on Windows
15ebb25 Clarify comment in test_hook_uses_shell_not_from_cwd
f44524a Avoid spurious "location may have moved" on Windows
a42ea0a Cover absent/no-distro bash.exe in hooks "not from cwd" test
7751436 Extract venv management from test_installation
66ff4c1 Omit CWD in search for bash.exe to run hooks on Windows
Additional commits viewable in compare view




You can trigger a rebase of this PR by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

@dependabot rebase will rebase this PR
@dependabot recreate will recreate this PR, overwriting any edits that have been made to it
@dependabot merge will merge this PR after your CI passes on it
@dependabot squash and merge will squash and merge this PR after your CI passes on it
@dependabot cancel merge will cancel a previously requested merge and block automerging
@dependabot reopen will reopen this PR if it is closed
@dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
@dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
@dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
@dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
@dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
You can disable automated security fix PRs for this repo from the Security Alerts page.



Note
Automatic rebases have been disabled on this pull request as it has been open for over 30 days.

Reviewed-by: Vladimir Vshivkov
JoeWang1127 referenced this pull request in googleapis/sdk-platform-java Apr 6, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [GitPython](https://github.com/gitpython-developers/GitPython) |
`==3.1.40` -> `==3.1.41` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/GitPython/3.1.41?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/GitPython/3.1.41?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/GitPython/3.1.40/3.1.41?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/GitPython/3.1.40/3.1.41?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

### GitHub Vulnerability Alerts

####
[CVE-2024-22190](https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-2mqj-m65w-jghx)

### Summary

This issue exists because of an incomplete fix for CVE-2023-40590. On
Windows, GitPython uses an untrusted search path if it uses a shell to
run `git`, as well as when it runs `bash.exe` to interpret hooks. If
either of those features are used on Windows, a malicious `git.exe` or
`bash.exe` may be run from an untrusted repository.

### Details

Although GitPython often avoids executing programs found in an untrusted
search path since 3.1.33, two situations remain where this still occurs.
Either can allow arbitrary code execution under some circumstances.

#### When a shell is used

GitPython can be told to run `git` commands through a shell rather than
as direct subprocesses, by passing `shell=True` to any method that
accepts it, or by both setting `Git.USE_SHELL = True` and not passing
`shell=False`. Then the Windows `cmd.exe` shell process performs the
path search, and GitPython does not prevent that shell from finding and
running `git` in the current directory.

When GitPython runs `git` directly rather than through a shell, the
GitPython process performs the path search, and currently omits the
current directory by setting `NoDefaultCurrentDirectoryInExePath` in its
own environment during the `Popen` call. Although the `cmd.exe` shell
will honor this environment variable when present, GitPython does not
currently pass it into the shell subprocess's environment.

Furthermore, because GitPython sets the subprocess CWD to the root of a
repository's working tree, using a shell will run a malicious `git.exe`
in an untrusted repository even if GitPython itself is run from a
trusted location.

This also applies if `Git.execute` is called directly with `shell=True`
(or after `Git.USE_SHELL = True`) to run any command.

#### When hook scripts are run

On Windows, GitPython uses `bash.exe` to run hooks that appear to be
scripts. However, unlike when running `git`, no steps are taken to avoid
finding and running `bash.exe` in the current directory.

This allows the author of an untrusted fork or branch to cause a
malicious `bash.exe` to be run in some otherwise safe workflows. An
example of such a scenario is if the user installs a trusted hook while
on a trusted branch, then switches to an untrusted feature branch
(possibly from a fork) to review proposed changes. If the untrusted
feature branch contains a malicious `bash.exe` and the user's current
working directory is the working tree, and the user performs an action
that runs the hook, then although the hook itself is uncorrupted, it
runs with the malicious `bash.exe`.

Note that, while `bash.exe` is a shell, this is a separate scenario from
when `git` is run using the unrelated Windows `cmd.exe` shell.

### PoC

On Windows, create a `git.exe` file in a repository. Then create a
`Repo` object, and call any method through it (directly or indirectly)
that supports the `shell` keyword argument with `shell=True`:

```powershell
mkdir testrepo
git init testrepo
cp ... testrepo git.exe # Replace "..." with any executable of choice.
python -c "import git; print(git.Repo('testrepo').git.version(shell=True))"
```

The `git.exe` executable in the repository directory will be run.

Or use no `Repo` object, but do it from the location with the `git.exe`:

```powershell
cd testrepo
python -c "import git; print(git.Git().version(shell=True))"
```

The `git.exe` executable in the current directory will be run.

For the scenario with hooks, install a hook in a repository, create a
`bash.exe` file in the current directory, and perform an operation that
causes GitPython to attempt to run the hook:

```powershell
mkdir testrepo
cd testrepo
git init
mv .git/hooks/pre-commit.sample .git/hooks/pre-commit
cp ... bash.exe # Replace "..." with any executable of choice.
echo "Some text" >file.txt
git add file.txt
python -c "import git; git.Repo().index.commit('Some message')"
```

The `bash.exe` executable in the current directory will be run.

### Impact

The greatest impact is probably in applications that set `Git.USE_SHELL
= True` for historical reasons. (Undesired console windows had, in the
past, been created in some kinds of applications, when it was not used.)
Such an application may be vulnerable to arbitrary code execution from a
malicious repository, even with no other exacerbating conditions. This
is to say that, if a shell is used to run `git`, the full effect of
CVE-2023-40590 is still present. Furthermore, as noted above, running
the application itself from a trusted directory is not a sufficient
mitigation.

An application that does not direct GitPython to use a shell to run
`git` subprocesses thus avoids most of the risk. However, there is no
such straightforward way to prevent GitPython from running `bash.exe` to
interpret hooks. So while the conditions needed for that to be exploited
are more involved, it may be harder to mitigate decisively prior to
patching.

### Possible solutions

A straightforward approach would be to address each bug directly:

- When a shell is used, pass `NoDefaultCurrentDirectoryInExePath` into
the subprocess environment, because in that scenario the subprocess is
the `cmd.exe` shell that itself performs the path search.
- Set `NoDefaultCurrentDirectoryInExePath` in the GitPython process
environment during the `Popen` call made to run hooks with a `bash.exe`
subprocess.

These need only be done on Windows.

---

### Release Notes

<details>
<summary>gitpython-developers/GitPython (GitPython)</summary>

###
[`v3.1.41`](https://github.com/gitpython-developers/GitPython/releases/tag/3.1.41):
- fix Windows security issue

[Compare
Source](https://github.com/gitpython-developers/GitPython/compare/3.1.40...3.1.41)

The details about the Windows security issue [can be found in this
advisory](https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-2mqj-m65w-jghx).

Special thanks go to
[@&#8203;EliahKagan](https://github.com/EliahKagan) who reported the
issue and fixed it in a single stroke, while being responsible for an
incredible amount of improvements that he contributed over the last
couple of months ❤️.

#### What's Changed

- Add `__all__` in git.exc by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1719](https://github.com/gitpython-developers/GitPython/pull/1719)
- Set submodule update cadence to weekly by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1721](https://github.com/gitpython-developers/GitPython/pull/1721)
- Never modify sys.path by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1720](https://github.com/gitpython-developers/GitPython/pull/1720)
- Bump git/ext/gitdb from `8ec2390` to `ec58b7e` by
[@&#8203;dependabot](https://github.com/dependabot) in
[https://github.com/gitpython-developers/GitPython/pull/1722](https://github.com/gitpython-developers/GitPython/pull/1722)
- Revise comments, docstrings, some messages, and a bit of code by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1725](https://github.com/gitpython-developers/GitPython/pull/1725)
- Use zero-argument super() by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1726](https://github.com/gitpython-developers/GitPython/pull/1726)
- Remove obsolete note in \_iter_packed_refs by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1727](https://github.com/gitpython-developers/GitPython/pull/1727)
- Reorganize test_util and make xfail marks precise by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1729](https://github.com/gitpython-developers/GitPython/pull/1729)
- Clarify license and make module top comments more consistent by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1730](https://github.com/gitpython-developers/GitPython/pull/1730)
- Deprecate compat.is\_<platform>, rewriting all uses by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1732](https://github.com/gitpython-developers/GitPython/pull/1732)
- Revise and restore some module docstrings by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1735](https://github.com/gitpython-developers/GitPython/pull/1735)
- Make the rmtree callback Windows-only by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1739](https://github.com/gitpython-developers/GitPython/pull/1739)
- List all non-passing tests in test summaries by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1740](https://github.com/gitpython-developers/GitPython/pull/1740)
- Document some minor subtleties in test_util.py by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1749](https://github.com/gitpython-developers/GitPython/pull/1749)
- Always read metadata files as UTF-8 in setup.py by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1748](https://github.com/gitpython-developers/GitPython/pull/1748)
- Test native Windows on CI by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1745](https://github.com/gitpython-developers/GitPython/pull/1745)
- Test macOS on CI by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1752](https://github.com/gitpython-developers/GitPython/pull/1752)
- Let close_fds be True on all platforms by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1753](https://github.com/gitpython-developers/GitPython/pull/1753)
- Fix IndexFile.from_tree on Windows by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1751](https://github.com/gitpython-developers/GitPython/pull/1751)
- Remove unused TASKKILL fallback in AutoInterrupt by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1754](https://github.com/gitpython-developers/GitPython/pull/1754)
- Don't return with operand when conceptually void by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1755](https://github.com/gitpython-developers/GitPython/pull/1755)
- Group .gitignore entries by purpose by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1758](https://github.com/gitpython-developers/GitPython/pull/1758)
- Adding dubious ownership handling by
[@&#8203;marioaag](https://github.com/marioaag) in
[https://github.com/gitpython-developers/GitPython/pull/1746](https://github.com/gitpython-developers/GitPython/pull/1746)
- Avoid brittle assumptions about preexisting temporary files in tests
by [@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1759](https://github.com/gitpython-developers/GitPython/pull/1759)
- Overhaul noqa directives by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1760](https://github.com/gitpython-developers/GitPython/pull/1760)
- Clarify some Git.execute kill_after_timeout limitations by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1761](https://github.com/gitpython-developers/GitPython/pull/1761)
- Bump actions/setup-python from 4 to 5 by
[@&#8203;dependabot](https://github.com/dependabot) in
[https://github.com/gitpython-developers/GitPython/pull/1763](https://github.com/gitpython-developers/GitPython/pull/1763)
- Don't install black on Cygwin by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1766](https://github.com/gitpython-developers/GitPython/pull/1766)
- Extract all "import gc" to module level by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1765](https://github.com/gitpython-developers/GitPython/pull/1765)
- Extract remaining local "import gc" to module level by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1768](https://github.com/gitpython-developers/GitPython/pull/1768)
- Replace xfail with gc.collect in TestSubmodule.test_rename by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1767](https://github.com/gitpython-developers/GitPython/pull/1767)
- Enable CodeQL by [@&#8203;EliahKagan](https://github.com/EliahKagan)
in
[https://github.com/gitpython-developers/GitPython/pull/1769](https://github.com/gitpython-developers/GitPython/pull/1769)
- Replace some uses of the deprecated mktemp function by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1770](https://github.com/gitpython-developers/GitPython/pull/1770)
- Bump github/codeql-action from 2 to 3 by
[@&#8203;dependabot](https://github.com/dependabot) in
[https://github.com/gitpython-developers/GitPython/pull/1773](https://github.com/gitpython-developers/GitPython/pull/1773)
- Run some Windows environment variable tests only on Windows by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1774](https://github.com/gitpython-developers/GitPython/pull/1774)
- Fix TemporaryFileSwap regression where file_path could not be Path by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1776](https://github.com/gitpython-developers/GitPython/pull/1776)
- Improve hooks tests by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1777](https://github.com/gitpython-developers/GitPython/pull/1777)
- Fix if items of Index is of type PathLike by
[@&#8203;stegm](https://github.com/stegm) in
[https://github.com/gitpython-developers/GitPython/pull/1778](https://github.com/gitpython-developers/GitPython/pull/1778)
- Better document IterableObj.iter_items and improve some subclasses by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1780](https://github.com/gitpython-developers/GitPython/pull/1780)
- Revert "Don't install black on Cygwin" by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1783](https://github.com/gitpython-developers/GitPython/pull/1783)
- Add missing pip in $PATH on Cygwin CI by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1784](https://github.com/gitpython-developers/GitPython/pull/1784)
- Shorten Iterable docstrings and put IterableObj first by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1785](https://github.com/gitpython-developers/GitPython/pull/1785)
- Fix incompletely revised Iterable/IterableObj docstrings by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1786](https://github.com/gitpython-developers/GitPython/pull/1786)
- Pre-deprecate setting Git.USE_SHELL by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1782](https://github.com/gitpython-developers/GitPython/pull/1782)
- Deprecate Git.USE_SHELL by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1787](https://github.com/gitpython-developers/GitPython/pull/1787)
- In handle_process_output don't forward finalizer result by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1788](https://github.com/gitpython-developers/GitPython/pull/1788)
- Fix mypy warning "Missing return statement" by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1789](https://github.com/gitpython-developers/GitPython/pull/1789)
- Fix two remaining Windows untrusted search path cases by
[@&#8203;EliahKagan](https://github.com/EliahKagan) in
[https://github.com/gitpython-developers/GitPython/pull/1792](https://github.com/gitpython-developers/GitPython/pull/1792)

#### New Contributors

- [@&#8203;marioaag](https://github.com/marioaag) made their first
contribution in
[https://github.com/gitpython-developers/GitPython/pull/1746](https://github.com/gitpython-developers/GitPython/pull/1746)
- [@&#8203;stegm](https://github.com/stegm) made their first
contribution in
[https://github.com/gitpython-developers/GitPython/pull/1778](https://github.com/gitpython-developers/GitPython/pull/1778)

**Full Changelog**:
gitpython-developers/GitPython@3.1.40...3.1.41

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "" (UTC), Automerge - At any time (no
schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/googleapis/sdk-platform-java).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yNjkuMiIsInVwZGF0ZWRJblZlciI6IjM3LjI2OS4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants