Skip to content

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

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

Closed
wants to merge 1 commit into from

Conversation

tomrittervg
Copy link

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
Copy link
Member

Byron commented Jun 7, 2012

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
Copy link
Contributor

maxyz commented Sep 23, 2014

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
Copy link
Member

Byron commented Nov 14, 2014

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
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.

3 participants