Skip to content
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

KeyError in blame_hunk #50

Open
bmwiedemann opened this issue Jun 3, 2015 · 7 comments
Open

KeyError in blame_hunk #50

bmwiedemann opened this issue Jun 3, 2015 · 7 comments

Comments

@bmwiedemann
Copy link
Contributor

I ran into some bug (on openSUSE-13.2 and Factory) using:

git clone git://github.com/openstack/python-keystoneclient.git
cd python-keystoneclient/
git-deps 3d6d749e6f0fef682a88758e1a2f6c9e8e7bd23c

this produced

7920899af119d1697c333d202ca3272f167c19b0
[...some more hashes...]
Traceback (most recent call last):
  File "/home/bernhard/bin/git-deps", line 790, in <module>
    main()
  File "/home/bernhard/bin/git-deps", line 784, in main
    cli(options, args)
  File "/home/bernhard/bin/git-deps", line 671, in cli
    detector.find_dependencies(dependent_rev)
  File "/home/bernhard/bin/git-deps", line 402, in find_dependencies
    self.find_dependencies_with_parent(dependent, parent)
  File "/home/bernhard/bin/git-deps", line 426, in find_dependencies_with_parent
    self.blame_hunk(dependent, parent, path, hunk)
  File "/home/bernhard/bin/git-deps", line 529, in blame_hunk
    rev = line_to_culprit[line_num]
KeyError: 2
@dwm1945
Copy link

dwm1945 commented Oct 5, 2016

Is there any feedback available on the status of this bug? My exception isn't quite identical and since I used --recurse the conditions aren't quite the same, but what I get is:

Traceback (most recent call last):
  File "/home/dmorris/Tools/git-deps/git-deps.py", line 794, in <module>
    main()
  File "/home/dmorris/Tools/git-deps/git-deps.py", line 788, in main
    cli(options, args)
  File "/home/dmorris/Tools/git-deps/git-deps.py", line 675, in cli
    detector.find_dependencies(dependent_rev)
  File "/home/dmorris/Tools/git-deps/git-deps.py", line 406, in find_dependencies
    self.find_dependencies_with_parent(dependent, parent)
  File "/home/dmorris/Tools/git-deps/git-deps.py", line 430, in find_dependencies_with_parent
    self.blame_hunk(dependent, parent, path, hunk)
  File "/home/dmorris/Tools/git-deps/git-deps.py", line 533, in blame_hunk
    rev = line_to_culprit[line_num]

My command:

git-deps.py --recurse   9a0cd44c4e9930c40b8ad128a12999061599dcf4 e5e4e280693b8fd9db389aa3defd7a2b1fee013f

from which I get about 20 pairs of commit IDs before the traceback. W/o the --recurse flag, I get a list of 5 commit IDs and no exception.

@dwm1945
Copy link

dwm1945 commented Oct 10, 2016

After adding a bit of debug logging, what I found is that hunk.lines has the gratuitous extra line added matching: "\n\ No newline at end of file" which I presume is because the file in git didn't end with "\n"

I added filtering for that string to the loop and that prevents the exception ... though now I'm getting what seems to be an impossibly long list of dependencies.

@aspiers
Copy link
Owner

aspiers commented Oct 10, 2016

Cool, thanks!!

Haven't had time to check yet but I would guess that if you are getting an impossibly long list then it's because you're using --recurse without also using --exclude-commits to set an upper bound on the set of commits. Commit dependency trees are often very thick, so if you recurse without bounds it's not uncommon to end up traversing the majority of the repository's history.

@dwm1945
Copy link

dwm1945 commented Oct 11, 2016

Long list ... yes I hacked in a change to read the excludes from a file where the excludes were obtained from "git log" on the branch I'm backporting to. The list now reflects what I tend to expect. Only a couple hundred commits.

@aspiers
Copy link
Owner

aspiers commented Oct 11, 2016

Sounds like an interesting change - feel free to submit a PR ;-)

@aspiers
Copy link
Owner

aspiers commented Oct 11, 2016

Also, would be very grateful if you could provide the patch which fixes the original KeyError issue!

@dwm1945
Copy link

dwm1945 commented Oct 12, 2016

will do both when I have completed this major backport ... I'll have more confidence then that what I've done is complete ....

The current patch for the keyerror:

        diff_format = '      |%8.8s %5s %s%s'
        hunk_header = '@@ %s %s @@' % (line_range_before, line_range_after)
        self.logger.debug(diff_format % ('--------', '-----', '', hunk_header))
        line_num = hunk.old_start
        for line in hunk.lines:
            if "\n\\ No newline at end of file" == line.content.rstrip() :
                break
            if line.origin == '+':

I inserted the two lines following "for line in hunk.lines. This is a hack and I didn't spend any time walking backwared to the creation of the hunk list where this would have been more appropriate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants