Skip to content

I think I found a bug in the diff('diff-against') function regarding reversing ordering... - #40

Closed
tomrittervg wants to merge 1 commit into
gitpython-developers:masterfrom
tomrittervg:master
Closed

I think I found a bug in the diff('diff-against') function regarding reversing ordering...#40
tomrittervg wants to merge 1 commit into
gitpython-developers:masterfrom
tomrittervg:master

Conversation

@tomrittervg

Copy link
Copy Markdown

Pull this repo: https://github.com/tomrittervg/Code-Audit-Feed-Test-Cases

Now do this:
$ python

import git as pygit
c = pygit.Repo('github.com-tomrittervg-Code-Audit-Feed-Test-Cases.git/')
x = c.commit('4aee3ae658c1320619432817d63ec7adabf0f43a')
p = [i for i in x.diff('HEAD~1')][0]

That is this commit: tomrittervg/Code-Audit-Feed-Test-Cases@4aee3ae
This is the addition of a single file.

Unless I'm misunderstanding git, as well as this documentation: http://packages.python.org/GitPython/0.3.1/tutorial.html#obtaining-diff-information

tdiff = hcommit.diff('HEAD~1') # diff tree against previous tree

x.diff('HEAD~1') ought to give a 'new_file' commit
But it was reversing the commit.

It ran this:
git diff 4aee3ae658c1320619432817d63ec7adabf0f43a HEAD~1 --abbrev=40 --full-index --raw
producing
:100644 000000 5f9b998a3e2916af6671a0f8d296ad7c1fe1490d 0000000000000000000000000000000000000000 D api-call-test.c

When it should be running this:
git diff HEAD~1 4aee3ae658c1320619432817d63ec7adabf0f43a --abbrev=40 --full-index --raw
producing this:
:000000 100644 0000000000000000000000000000000000000000 5f9b998a3e2916af6671a0f8d296ad7c1fe1490d A api-call-test.c

I changed the order of arguments to get it to give a file addition. This would also affect which diff the a_blob and b_blob in a 'M' and how a 'R' behaves... I think.

…ases

Now do this:
$ python
>>> import git as pygit
>>> c = pygit.Repo('github.com-tomrittervg-Code-Audit-Feed-Test-Cases.git/')
>>> x = c.commit('4aee3ae658c1320619432817d63ec7adabf0f43a')
>>> p = [i for i in x.diff('HEAD~1')][0]

That is this commit: tomrittervg/Code-Audit-Feed-Test-Cases@4aee3ae
This is the addition of a single file.

Unless I'm misunderstanding git, as well as this documentation: http://packages.python.org/GitPython/0.3.1/tutorial.html#obtaining-diff-information

   tdiff = hcommit.diff('HEAD~1')  # diff tree against previous tree

x.diff('HEAD~1') ought to give a 'new_file' commit
But it was reversing the commit.

It ran this:
  git diff 4aee3ae658c1320619432817d63ec7adabf0f43a HEAD~1 --abbrev=40 --full-index --raw
producing
  :100644 000000 5f9b998a3e2916af6671a0f8d296ad7c1fe1490d 0000000000000000000000000000000000000000 D      api-call-test.c

When it should be running this:
  git diff HEAD~1 4aee3ae658c1320619432817d63ec7adabf0f43a --abbrev=40 --full-index --raw
producing this:
  :000000 100644 0000000000000000000000000000000000000000 5f9b998a3e2916af6671a0f8d296ad7c1fe1490d A      api-call-test.c

I changed the order of arguments to get it to give a file addition.  This would also affect which diff the a_blob and b_blob in a 'M' and how a 'R' behaves... I think.
@Byron

Byron commented Jun 7, 2012

Copy link
Copy Markdown
Member

Hi,

Thanks for the patch.
However, I am afraid just removing this line might actually break one or more tests.
Please use nosetests to verify that it didn't break anything. Additionally, it would be great to have a test which specifically verifies your issue.

Thank you

@maxyz

maxyz commented Sep 23, 2014

Copy link
Copy Markdown
Contributor

Please do not revert the diff order

I think that the current behavior is the expected one:
commit_a.diff(commit_b) => diffs that get commit_a to commit_b

Reversing this, would only break any existing code that depends on this. Also,
commit_a.diff(commit_b, R=true) would produce the reversed diff.

So, I think this pull request could be closed.

@Byron

Byron commented Nov 14, 2014

Copy link
Copy Markdown
Member

I agree, reversing the order now would possibly break too much code. We can restart the discussion if needs be though.

@Byron Byron closed this Nov 14, 2014
govtech42 pushed a commit to mirror81/GitPython that referenced this pull request Jul 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants