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

3.0.0 regression #907

Closed
yarikoptic opened this issue Aug 12, 2019 · 2 comments
Closed

3.0.0 regression #907

yarikoptic opened this issue Aug 12, 2019 · 2 comments

Comments

@yarikoptic
Copy link
Contributor

bisected to 0b6b90f which caused our tests to start failing with 3.0.0 release: datalad/datalad#3598 . Unfortunately I do not have yet a good concise example to reproduce.

here is our failing test traceback on travis:

# from https://travis-ci.org/datalad/datalad/jobs/570888123#L1125
======================================================================
ERROR: datalad.support.tests.test_annexrepo.test_AnnexRepo_flyweight_monitoring_inode
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/datalad/tests/utils.py", line 615, in newfunc
    return t(*(arg + (filename,)), **kw)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/datalad/tests/utils.py", line 615, in newfunc
    return t(*(arg + (filename,)), **kw)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/datalad/support/tests/test_annexrepo.py", line 2399, in test_AnnexRepo_flyweight_monitoring_inode
    check_repo_deals_with_inode_change(AnnexRepo, path, store)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/datalad/support/tests/utils.py", line 53, in check_repo_deals_with_inode_change
    hexsha = repo.repo.head.object.hexsha
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/git/refs/symbolic.py", line 193, in _get_object
    return Object.new_from_sha(self.repo, hex_to_bin(self.dereference_recursive(self.repo, self.path)))
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/git/objects/base.py", line 64, in new_from_sha
    oinfo = repo.odb.info(sha1)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/git/db.py", line 37, in info
    hexsha, typename, size = self._git.get_object_header(bin_to_hex(sha))
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/git/cmd.py", line 1077, in get_object_header
    return self.__get_object_header(cmd, ref)
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/git/cmd.py", line 1066, in __get_object_header
    return self._parse_object_header(cmd.stdout.readline())
  File "/home/travis/virtualenv/python3.5.6/lib/python3.5/site-packages/git/cmd.py", line 1030, in _parse_object_header
    raise ValueError("SHA %s could not be resolved, git returned: %r" % (tokens[0], header_line.strip()))
ValueError: SHA b'8097085c9fb022ad18cea2ff099fbcd13ae953be' could not be resolved, git returned: b'8097085c9fb022ad18cea2ff099fbcd13ae953be missing'
@kyleam
Copy link

kyleam commented Aug 13, 2019

As mentioned at datalad/datalad#3598, .close() resetting the git.cmd.Git instance seems to be new with 0b6b90f. Reverting that bit seems to resolve the failure on our end

diff --git a/git/repo/base.py b/git/repo/base.py
index 31b57a3..348474e 100644
--- a/git/repo/base.py
+++ b/git/repo/base.py
@@ -224,7 +224,6 @@ def _get_git(self):
     def _del_git(self):
         if self._git:
             self._git.clear_cache()
-            self._git = None
             # Tempfiles objects on Windows are holding references to
             # open files until they are collected by the garbage
             # collector, thus preventing deletion.

0b6b90f doesn't mention any change in behavior in this regard. Is setting _git to None necessary for the change in 0b6b90f to function properly (or to avoid some other issue)?

@Byron
Copy link
Member

Byron commented Aug 14, 2019

Also related to #906, which is a performance regression caused by the same commit 0b6b90f. The latter was introduced to fix #719 , and has since been reverted.
It's probably worth creating an intermediate 3.0.1 release to fix the breakage, and hope for another attempt to fix #719.

Also CC @bdauvergne who authored the fix and who might know why setting _git to None was required, and possible alternatives.

Thank you all for your help!

@Byron Byron closed this as completed Aug 14, 2019
@Byron Byron added this to the v3.0.1 - Bugfixes milestone Aug 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants