Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork939
fix files list on file rename#1537
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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!
It should be straightforward to add a test that motivates this changes. The test-suite uses fixture files which could be derived from an example repository.
With a test that fails without this change, the fix can be merged.
Thank you
@Byron done, and I rewrote the commit message using GitPython hashes, so is simpler for anyone to test |
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, this looks like the way to go!
Instead of checking what it shouldn't be (which is a huge set of possible values), could you instead check what it should be? How does a rename look like with the current interface, I'd be very interested to see that.
For posterity, here is whatgit thinks about the commit being used in the test:
commit 185d847ec7647fd2642a82d9205fb3d07ea71715Author: Dominic <<redacted>@gmail.com>Date: Mon Jul 12 17:02:21 2021 +0100 Update and rename test_pytest.yml to Future.ymldiff --git a/.github/workflows/test_pytest.yml b/.github/workflows/Future.ymlsimilarity index 92%rename from .github/workflows/test_pytest.ymlrename to .github/workflows/Future.ymlindex 627e720f..39146533 100644--- a/.github/workflows/test_pytest.yml+++ b/.github/workflows/Future.yml@@ -5,7 +5,9 @@ name: Future on: push:- branches: [main]+ branches: [ main ]+ pull_request:+ branches: [ main ] jobs: build:
This is the output of And with |
GitPython parses the output of `git diff --numstat` to get thefiles changed in a commit.This breaks when a commit contains a file rename, because the outputof `git diff` is different than expected.This is the output of a normal commit: $ git diff --numstat8f41a39^8f41a39 30 5 test/test_repo.pyAnd this a commit containing a rename: $ git diff --numstat185d847^185d847 3 1 .github/workflows/{test_pytest.yml => Future.yml}This can be triggered by this code: for commit in repo.iter_commits(): print(commit.hexsha) for file in commit.stats.files: print(file)Which will print for the normal commit:8f41a39 'test/test_repo.py'And when there is a rename:185d847 '.github/workflows/{test_pytest.yml => Future.yml}'Additionally, when a path member is removed, the file list becomea list of strings, breaking even more the caller. This is in theLinux kernel tree: $ git diff --numstat db401875f438^ db401875f438 1 1 tools/testing/selftests/drivers/net/mlxsw/{spectrum-2 => }/devlink_trap_tunnel_ipip6.shand GitPython parses it as: db401875f438168c5804b295b93a28c7730bb57a ('tools/testing/selftests/drivers/net/mlxsw/{spectrum-2 => ' '}/devlink_trap_tunnel_ipip6.sh')Fix this by pasing the --no-renames option to `git diff` which ignoresrenames and print the same output as if the file was deleted from theold path and created in the new one: $ git diff --numstat --no-renames185d847^185d847 57 0 .github/workflows/Future.yml 0 55 .github/workflows/test_pytest.yml
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 million, that is what I had in mind.
Thanks again for contributing!
Uh oh!
There was an error while loading.Please reload this page.
GitPython parses the output of
git diff --no-renamesto get the files changed in a commit.This breaks when a commit contains a file rename, because the output of
git diffis different than expected.This is the output of a normal commit:
And this a commit containing a rename:
This can be triggered by this code:
Which will print for the normal commit:
And when there is a rename:
Fix this by pasing the --no-renames option to
git diffwhich ignores renames and print the same output as if the file was deleted from the old path and created in the new one: