Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork938
fix: treeNotSorted issue#1799
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
Merged
Merged
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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
Byron requested changesJan 12, 2024
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.
Thanks a lot for providing a fix!
I wonder if there was a way to assure the ordering is correct by looking at what git would do.
I assume thefiles_in_desired_order is from a repository, could you show agit cat-file <tree>? Alternatively, maybe there is a way to usegit to build a (somewhat) similar tree, and have the test compare the outcome of GitPython with the outcome of git itself - it's much more effort though.
Finally,here is a good tree for testing ordering, and you might be able create a test-case with this one as well.
Something I'd appreciate is to re-order the commits so that the test, whatever you come up with, first, and the fix later so it's clear how it fails first.
Thanks a lot for bearing with me, but I'd like to gain confidence that this is now indeed what git would do, and not just different but subtly wrong (I have been bitten by this more than once).
Uh oh!
There was an error while loading.Please reload this page.
Would this also fix#1771? |
Uh oh!
There was an error while loading.Please reload this page.
BTW, the test i provide before ishere fyi~ |
i think it's related |
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestJan 15, 2024
This fixes version incompatibilities with major version 4 ofSphinx, which GitPython is still using for the time being. Someplugins that previously had depended back on Sphinx have had thosedependencies removed to avoid dependency cycles, but the effect isthat `pip` no longer is aware of or able to enforce versioncompatibility, and newer plugin versions than can actually run withSphinx 4 are installed instead of older, usable versions.This fixes the problem by pinning each of the affected Sphinxplugins' latest actually compatible versions, i.e., latest versionsthat do not need Sphinx 5 or higher.The alternative of moving to Sphinx 5 or higher should probably bedone eventually, but will require addressing a cross-referenceambiguity in type annotations for the `Actor` class.For details on the bug, see:-gitpython-developers#1799 (comment)-gitpython-developers#1802
Byron approved these changesJan 16, 2024
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.
Thanks a lot for bearing with us for changes and the turbulent CI.
If this change has made the feature usable as originally intended and supersedes the advice in#369 (comment), then I think think also fixes#1771. If not, then I think#1771 should be modified accordingly, and any non-obvious limitations of the affected code documented in its docstrings (which would close#1771). |
I'd still lean towards rather removing it as it's deemed unfit, but that might be overly generalising and unnecessarily dismissive of the work of 'young' me. Since @et-repositories has had interest in fixing this, maybe they also have interest in investigating this matter? |
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestFeb 15, 2024
This fixes a test bug in TestTree.test_tree_modifier_orderingwhere:- A `tmp` subdirectory of the directory the tests are run from (almost always the root of the GitPython source tree) was created, instead of creating it in /tmp or elsewhere configured.- That directory was never deleted afterward, creating new untracked files in GitPython's own working tree, and also making it so running the tests twice would always fail the second time, unless a manual intervening deletion was performed. (This had not broken CI, since CI checks clone the repository anew each time.)- The directory was changed into to set up the test, but not deterministically changed back out of. It would typically be exited, but this was not guaranteed to occur if some subprocess commands were to fail unexpectedly.In addition to fixing all three aspects of that bug, this also:- Remains in the temporary directory only as long as necessary to execute the subprocesses in it that produce the expected value for the test (including not entering it until running them).- Deletes the temporary directory immediately after exiting it, since it is only used to produce a file list to compare to.- Avoids interleaving file creation and git subprocess commands, since doing so made it harder to understand what was being set up and since the difference is not relevant to the test.- Does the work in that temporary directory before performing any operations with the code under test, because it is conceptually an "arrange" step, and because doing so makes its limited purpose clearer on reading the tests.- Some other refactoring to support the fixes and accompanying changes, including extracting the temporary directory logic to a helper method.This deliberately does not change the order in which any files arecreated. It also keeps the approach of using subprocess functionsdirectly to operate on the temporary git repository (and changingdirectory while doing so, keeping each of the commands the same),since there might be good reasons to do this, such as to make veryclear that the file order being obtained to compare to is reallycoming from git itself. Even if this is to be changed, it seemsoutside the scope of this bugfix.The test still fails if the fix to git/objects/tree.py from365d44f(gitpython-developers#1799) is absent. This was verified by running: git revert --no-commit365d44f pytest --no-cov -vv test/test_tree.py git revert --abort
This was referencedFeb 15, 2024
lettuce-botbot referenced this pull request in lettuce-financial/github-bot-signed-commitFeb 15, 2024
[](https://renovatebot.com)This PR contains the following updates:| Package | Change | Age | Adoption | Passing | Confidence ||---|---|---|---|---|---|| [GitPython](https://togithub.com/gitpython-developers/GitPython) |`==3.1.41` -> `==3.1.42` |[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|---### Release Notes<details><summary>gitpython-developers/GitPython (GitPython)</summary>###[`v3.1.42`](https://togithub.com/gitpython-developers/GitPython/releases/tag/3.1.42)[CompareSource](https://togithub.com/gitpython-developers/GitPython/compare/3.1.41...3.1.42)#### What's Changed- Fix release link in changelog by[@​PeterJCLaw](https://togithub.com/PeterJCLaw) in[https://github.com/gitpython-developers/GitPython/pull/1795](https://togithub.com/gitpython-developers/GitPython/pull/1795)- Remove test dependency on sumtypes library by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1798](https://togithub.com/gitpython-developers/GitPython/pull/1798)- Pin Sphinx plugins to compatible versions by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1803](https://togithub.com/gitpython-developers/GitPython/pull/1803)- fix: treeNotSorted issue by[@​et-repositories](https://togithub.com/et-repositories) in[https://github.com/gitpython-developers/GitPython/pull/1799](https://togithub.com/gitpython-developers/GitPython/pull/1799)- Remove git.util.NullHandler by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1807](https://togithub.com/gitpython-developers/GitPython/pull/1807)- Clarify why GIT_PYTHON_GIT_EXECUTABLE may be set on failure by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1810](https://togithub.com/gitpython-developers/GitPython/pull/1810)- Report actual attempted Git command when Git.refresh fails by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1812](https://togithub.com/gitpython-developers/GitPython/pull/1812)- Don't suppress messages when logging is not configured by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1813](https://togithub.com/gitpython-developers/GitPython/pull/1813)- Pin Python 3.9.16 on Cygwin CI by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1814](https://togithub.com/gitpython-developers/GitPython/pull/1814)- Have initial refresh use a logger to warn by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1815](https://togithub.com/gitpython-developers/GitPython/pull/1815)- Omit warning prefix in "Bad git executable" message by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1816](https://togithub.com/gitpython-developers/GitPython/pull/1816)- Test with M1 macOS CI runner by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1817](https://togithub.com/gitpython-developers/GitPython/pull/1817)- Bump pre-commit/action from 3.0.0 to 3.0.1 by[@​dependabot](https://togithub.com/dependabot) in[https://github.com/gitpython-developers/GitPython/pull/1818](https://togithub.com/gitpython-developers/GitPython/pull/1818)- Bump Vampire/setup-wsl from 2.0.2 to 3.0.0 by[@​dependabot](https://togithub.com/dependabot) in[https://github.com/gitpython-developers/GitPython/pull/1819](https://togithub.com/gitpython-developers/GitPython/pull/1819)- Remove deprecated section in README.md by[@​marcm-ml](https://togithub.com/marcm-ml) in[https://github.com/gitpython-developers/GitPython/pull/1823](https://togithub.com/gitpython-developers/GitPython/pull/1823)- Keep temp files out of project dir and improve cleanup by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1825](https://togithub.com/gitpython-developers/GitPython/pull/1825)#### New Contributors- [@​PeterJCLaw](https://togithub.com/PeterJCLaw) made their firstcontribution in[https://github.com/gitpython-developers/GitPython/pull/1795](https://togithub.com/gitpython-developers/GitPython/pull/1795)- [@​et-repositories](https://togithub.com/et-repositories) madetheir first contribution in[https://github.com/gitpython-developers/GitPython/pull/1799](https://togithub.com/gitpython-developers/GitPython/pull/1799)- [@​marcm-ml](https://togithub.com/marcm-ml) made their firstcontribution in[https://github.com/gitpython-developers/GitPython/pull/1823](https://togithub.com/gitpython-developers/GitPython/pull/1823)**Full Changelog**:gitpython-developers/GitPython@3.1.41...3.1.42</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 youare satisfied.♻ **Rebasing**: Whenever PR becomes conflicted, or you tick therebase/retry checkbox.🔕 **Ignore**: Close this PR and you won't be reminded about theseupdates again.---- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, checkthis box---This PR has been generated by [MendRenovate](https://www.mend.io/free-developer-tools/renovate/). Viewrepository job log[here](https://developer.mend.io/github/lettuce-financial/github-bot-signed-commit).<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xNzMuMCIsInVwZGF0ZWRJblZlciI6IjM3LjE3My4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->
%5d(https%3a%2f%2frenovatebot.com){kind=link}
renovatebot referenced this pull request in allenporter/flux-localFeb 16, 2024
[](https://renovatebot.com)This PR contains the following updates:| Package | Change | Age | Adoption | Passing | Confidence ||---|---|---|---|---|---|| [GitPython](https://togithub.com/gitpython-developers/GitPython) |`==3.1.41` -> `==3.1.42` |[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|---### Release Notes<details><summary>gitpython-developers/GitPython (GitPython)</summary>###[`v3.1.42`](https://togithub.com/gitpython-developers/GitPython/releases/tag/3.1.42)[CompareSource](https://togithub.com/gitpython-developers/GitPython/compare/3.1.41...3.1.42)#### What's Changed- Fix release link in changelog by[@​PeterJCLaw](https://togithub.com/PeterJCLaw) in[https://github.com/gitpython-developers/GitPython/pull/1795](https://togithub.com/gitpython-developers/GitPython/pull/1795)- Remove test dependency on sumtypes library by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1798](https://togithub.com/gitpython-developers/GitPython/pull/1798)- Pin Sphinx plugins to compatible versions by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1803](https://togithub.com/gitpython-developers/GitPython/pull/1803)- fix: treeNotSorted issue by[@​et-repositories](https://togithub.com/et-repositories) in[https://github.com/gitpython-developers/GitPython/pull/1799](https://togithub.com/gitpython-developers/GitPython/pull/1799)- Remove git.util.NullHandler by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1807](https://togithub.com/gitpython-developers/GitPython/pull/1807)- Clarify why GIT_PYTHON_GIT_EXECUTABLE may be set on failure by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1810](https://togithub.com/gitpython-developers/GitPython/pull/1810)- Report actual attempted Git command when Git.refresh fails by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1812](https://togithub.com/gitpython-developers/GitPython/pull/1812)- Don't suppress messages when logging is not configured by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1813](https://togithub.com/gitpython-developers/GitPython/pull/1813)- Pin Python 3.9.16 on Cygwin CI by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1814](https://togithub.com/gitpython-developers/GitPython/pull/1814)- Have initial refresh use a logger to warn by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1815](https://togithub.com/gitpython-developers/GitPython/pull/1815)- Omit warning prefix in "Bad git executable" message by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1816](https://togithub.com/gitpython-developers/GitPython/pull/1816)- Test with M1 macOS CI runner by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1817](https://togithub.com/gitpython-developers/GitPython/pull/1817)- Bump pre-commit/action from 3.0.0 to 3.0.1 by[@​dependabot](https://togithub.com/dependabot) in[https://github.com/gitpython-developers/GitPython/pull/1818](https://togithub.com/gitpython-developers/GitPython/pull/1818)- Bump Vampire/setup-wsl from 2.0.2 to 3.0.0 by[@​dependabot](https://togithub.com/dependabot) in[https://github.com/gitpython-developers/GitPython/pull/1819](https://togithub.com/gitpython-developers/GitPython/pull/1819)- Remove deprecated section in README.md by[@​marcm-ml](https://togithub.com/marcm-ml) in[https://github.com/gitpython-developers/GitPython/pull/1823](https://togithub.com/gitpython-developers/GitPython/pull/1823)- Keep temp files out of project dir and improve cleanup by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1825](https://togithub.com/gitpython-developers/GitPython/pull/1825)#### New Contributors- [@​PeterJCLaw](https://togithub.com/PeterJCLaw) made their firstcontribution in[https://github.com/gitpython-developers/GitPython/pull/1795](https://togithub.com/gitpython-developers/GitPython/pull/1795)- [@​et-repositories](https://togithub.com/et-repositories) madetheir first contribution in[https://github.com/gitpython-developers/GitPython/pull/1799](https://togithub.com/gitpython-developers/GitPython/pull/1799)- [@​marcm-ml](https://togithub.com/marcm-ml) made their firstcontribution in[https://github.com/gitpython-developers/GitPython/pull/1823](https://togithub.com/gitpython-developers/GitPython/pull/1823)**Full Changelog**:gitpython-developers/GitPython@3.1.41...3.1.42</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 therebase/retry checkbox.🔕 **Ignore**: Close this PR and you won't be reminded about this updateagain.---- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, checkthis box---This PR has been generated by [MendRenovate](https://www.mend.io/free-developer-tools/renovate/). Viewrepository job log[here](https://developer.mend.io/github/allenporter/flux-local).<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xNzMuMCIsInVwZGF0ZWRJblZlciI6IjM3LjE3My4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Labels
None yet
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#369