-
-
Notifications
You must be signed in to change notification settings - Fork 923
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
fix files list on file rename #1537
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 what git
thinks about the commit being used in the test:
commit 185d847ec7647fd2642a82d9205fb3d07ea71715
Author: Dominic <<redacted>@gmail.com>
Date: Mon Jul 12 17:02:21 2021 +0100
Update and rename test_pytest.yml to Future.yml
diff --git a/.github/workflows/test_pytest.yml b/.github/workflows/Future.yml
similarity index 92%
rename from .github/workflows/test_pytest.yml
rename to .github/workflows/Future.yml
index 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 the files changed in a commit. This breaks when a commit contains a file rename, because the output of `git diff` is different than expected. This is the output of a normal commit: $ git diff --numstat 8f41a39^ 8f41a39 30 5 test/test_repo.py And this a commit containing a rename: $ git diff --numstat 185d847^ 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 become a list of strings, breaking even more the caller. This is in the Linux kernel tree: $ git diff --numstat db401875f438^ db401875f438 1 1 tools/testing/selftests/drivers/net/mlxsw/{spectrum-2 => }/devlink_trap_tunnel_ipip6.sh and 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 ignores renames and print the same output as if the file was deleted from the old path and created in the new one: $ git diff --numstat --no-renames 185d847^ 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!
GitPython parses the output of
git diff --no-renames
to get the files changed in a commit.This breaks when a commit contains a file rename, because the output of
git diff
is 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 diff
which ignores renames and print the same output as if the file was deleted from the old path and created in the new one: