Update and adjust pre-commit hooks by EliahKagan · Pull Request #1953 · gitpython-developers/GitPython
added 10 commits
August 15, 2024 18:08On Windows, when `core.symlinks` is `false` or unset (since it defaults to `false` on Windows), Git checks out symbolic links as regular files whose contents are symlinks' target paths. Modifying those regular files and committing the changes alters the symlink target in the repository, and when they are checked out as actual symlinks, the targets are different. But the `end-of-file-fixer` pre-commit hook automatically adds newlines to the end of regular files that lack them. It doesn't do this on actual symlinks, but it does do it to regular files that stand in for symlinks. This causes it to carry a risk of breaking symlinks if it is run on Windows and the changes committed, and it is easy to miss that this will happen because `git diff` output shows it the same way as other additions of absent newlines. This deliberately commits the change that end-of-file-fixer makes to the `LICENSE-BSD` symlink, in order to allow a mitigation beyond just excluding that symlink (or replacing it with a regular file) to be tested. This change must be undone, of course.
Rationale: - Small but likely benefit in general, since there are no currently foreseen intentional use cases of committing of broken/dangling symlinks in this project. So such symlinks that arise are likely unintentional. - If the end-of-file-fixer hook has run on a Windows system where `core.symlinks` has *not* been set to `true`, and symlinks' paths have not been excluded, then a newline character is added to the end of the path held in the regular file Git checks out to stand in for the symlink. Because it is not actually a symlink, this will not detect the problem at that time (regardless of the order in which this and that hook run relative to each other). But when it is then run on CI on a system where symlinks are checked out, it will detect the problem.
The unanchored `LICENSE` and `COPYING` alternatives match the pattern anywhere, and therefore exclude the currently used path `fuzzing/LICENSE-BSD`. License files are more likely than other files in this project to be introduced as symlinks, and less likely to be noticed immediately if they break. Symlinks can be checked out as regular files when `core.symlinks` is set to `false`, which is rare outside of Windows but is the default behavior when unset on Windows. This exclusion fixes the current problem that end-of-file-fixer breaks those links by adding a newline character to the end (the symlinks are checked out broken if that is committed). It also guards against most future cases involving licenses, though possibly not all, and not other unrelated cases where symlinks may be used for other purposes. Although the pre-commit-hooks repository also provides a destroyed-symlinks hook that detects the situation of a symlink that has been replaced by a regular file, this does not add that hook, because this situation is not inherently a problem. The code here does not require symlinks to be checked out to work, and adding that would break significant uses of the repository on Windows. Note that this leaves the situation where a license file may be a symlink to another license file and may thus be checked out as a regular file containing that file's path. However, it is easy to understand that situation and manually follow the path. That differs from the scenario where a symlink is created but broken, because attempting to open it gives an error, and the error message is often non-obvious, reporting that a file is not found but giving the name of the symlink rather than its target.
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request
Jun 8, 2025This resolves two warnings about Ruff configuration, by: - No longer setting `ignore-init-module-imports = true` explicitly, which was deprecated since `ruff` 0.4.4. We primarily use `ruff` via `pre-commit`, for which this deprecation has applied since we upgraded the version in `.pre-commit-config.yaml` from 0.4.3 to 0.6.0 in d1582d1 (gitpython-developers#1953). We continue to list `F401` ("Module imported but unused") as not automatically fixable, to avoid inadvertently removing imports that may be needed. See also: https://docs.astral.sh/ruff/settings/#lint_ignore-init-module-imports - Rename the rule `TCH004` to `TC004`, since `TCH004` is the old name that may eventually be removed and that is deprecated since 0.8.0. We upgraded `ruff` in `.pre-commit-config.yml` again in b7ce712 (gitpython-developers#2031), from 0.6.0 to 0.11.12, at which point this deprecation applied. See also https://astral.sh/blog/ruff-v0.8.0. These changes make those configuration-related warnings go away, and no new diagnostics (errors/warnings) are produced when running `ruff check` or `pre-commit --all-files`. No F401-related diagnostics are triggered when testing with explicit `ignore-init-module-imports = false`, in preview mode or otherwise. This commit also adds the version lower bound `>=0.8` for `ruff` in `requirements-dev.txt`. (That file is rarely used, as noted in a8a73ff (gitpython-developers#1871), but as long as we have it, there may be a benefit to excluding dependency versions for which our configuration is no longer compatible.)
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request
Jun 8, 2025This resolves two warnings about Ruff configuration, by: - No longer setting `ignore-init-module-imports = true` explicitly, which was deprecated since `ruff` 0.4.4. We primarily use `ruff` via `pre-commit`, for which this deprecation has applied since we upgraded the version in `.pre-commit-config.yaml` from 0.4.3 to 0.6.0 in d1582d1 (gitpython-developers#1953). We continue to list `F401` ("Module imported but unused") as not automatically fixable, to avoid inadvertently removing imports that may be needed. See also: https://docs.astral.sh/ruff/settings/#lint_ignore-init-module-imports - Rename the rule `TCH004` to `TC004`, since `TCH004` is the old name that may eventually be removed and that is deprecated since 0.8.0. We upgraded `ruff` in `.pre-commit-config.yml` again in b7ce712 (gitpython-developers#2031), from 0.6.0 to 0.11.12, at which point this deprecation applied. See also https://astral.sh/blog/ruff-v0.8.0. These changes make those configuration-related warnings go away, and no new diagnostics (errors/warnings) are produced when running `ruff check` or `pre-commit run --all-files`. No F401-related diagnostics are triggered when testing with explicit `ignore-init-module-imports = false`, in preview mode or otherwise. In addition, this commit makes two changes that are not needed to resolve warnings: - Stop excluding `E203` ("Whitespace before ':'"). That diagnostic is no longer failing with the current code here in the current version of `ruff`, and code changes that would cause it to fail would likely be accidentally mis-st - Add the version lower bound `>=0.8` for `ruff` in `requirements-dev.txt`. That file is rarely used, as noted in a8a73ff (gitpython-developers#1871), but as long as we have it, there may be a benefit to excluding dependency versions for which our configuration is no longer compatible. This is the only change in this commit outside of `pyproject.toml`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters