FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix files list on file rename by teknoraver · Pull Request #1537 · gitpython-developers/GitPython · GitHub

Repository navigation

fix files list on file rename - #1537

Merged
Byron merged 1 commit into
gitpython-developers:mainfrom
teknoraver:main
Jan 14, 2023
Merged

Byron merged 1 commit into
gitpython-developers:mainfrom
teknoraver:main

Conversation

teknoraver commented Jan 12, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

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:

$ git diff --numstat 6ab9810cfe6c^ 6ab9810cfe6c
5       2       drivers/clk/zynqmp/divider.c

And this a commit containing a rename:

$ git diff --numstat db401875f438^ db401875f438
1       1       tools/testing/selftests/drivers/net/mlxsw/{spectrum-2 => }/devlink_trap_tunnel_ipip6.sh

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:

6ab9810cfe6c8f3d8b8750c827d7870abd3751b9
'drivers/clk/zynqmp/divider.c'

And when there is a rename:

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 db401875f438^ db401875f438
250     0       tools/testing/selftests/drivers/net/mlxsw/devlink_trap_tunnel_ipip6.sh
0       250     tools/testing/selftests/drivers/net/mlxsw/spectrum-2/devlink_trap_tunnel_ipip6.sh

Byron added this to the v3.1.31 - Bugfixes milestone Jan 12, 2023

Byron left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Copy link
Copy Markdown
Contributor Author

@Byron done, and I rewrote the commit message using GitPython hashes, so is simpler for anyone to test

Byron left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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:

Copy link
Copy Markdown
Contributor Author

This is the output of git show, gitpython does git diff --numstat $commit^ $commit which gives:

3       1       .github/workflows/{test_pytest.yml => Future.yml}

And with --no-renames added:

57      0       .github/workflows/Future.yml
0       55      .github/workflows/test_pytest.yml

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

Byron left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks a million, that is what I had in mind.

Thanks again for contributing!

Byron merged commit 3901d4c into gitpython-developers:main Jan 14, 2023
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
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


Back | FazBrowse Home | New Git URL