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

gitfs: Fix use of deprecated pygit2 function #51304

Merged
merged 2 commits into from Jan 25, 2019

Conversation

@terminalmage
Copy link
Contributor

commented Jan 24, 2019

0.27.4 (released 5 days ago) removed pygit2.Reference.get_object(). This fixes it so that older pygit2 still works, while using .peel() where applicable.

Resolves #51270.

0.27.4 (released 5 days ago) removed pygit2.Reference.get_object()
@terminalmage terminalmage force-pushed the terminalmage:issue51270 branch from 3eb5e74 to c02757d Jan 24, 2019
@s0undt3ch s0undt3ch merged commit 31b921f into saltstack:2018.3 Jan 25, 2019
1 check passed
1 check passed
WIP Ready for review
Details
@terminalmage terminalmage deleted the terminalmage:issue51270 branch Jan 28, 2019
@toanju

This comment has been minimized.

Copy link
Contributor

commented Mar 1, 2019

@terminalmage what is the best way to backport this to Fedora since this is now broken there?

@terminalmage

This comment has been minimized.

Copy link
Contributor Author

commented Mar 26, 2019

@toanju What Fedora are you using where 0.27.4 is available in the repositories? I think only F30 is affected but it's still in beta.

In the meantime, you'd have to either install pygit2 version 0.27.3 or apply the patch from this pull request. If you decide to do the latter, you can get the patch by adding .patch to this pull request's URL.

@toanju

This comment has been minimized.

Copy link
Contributor

commented Mar 26, 2019

@terminalmage F29 got this as well https://apps.fedoraproject.org/packages/python-pygit2

In any case what is the best workflow to bring this upstream? A new issue here or in the Bugzilla of Red Hat or directly a PR to src.fedoraproject.org? Thanks for the feedback :)

@terminalmage

This comment has been minimized.

Copy link
Contributor Author

commented Mar 27, 2019

The RedHat Bugzilla is probably a good place to start. Just mention that the fix is https://github.com/saltstack/salt/pull/51304.patch.

@Poil

This comment has been minimized.

Copy link

commented Jun 21, 2019

+1 (need to use pip on EL7 when using your py3 packages or you should provide the package python3.6-pygit2)

@ptitdoc

This comment has been minimized.

Copy link

commented Jul 15, 2019

For some reason, this pull request has been merged into saltstack:2018.3, but I use a 2019.2.0 install on archlinux and the commit is not applied. Is it normal ?

@terminalmage

This comment has been minimized.

Copy link
Contributor Author

commented Jul 15, 2019

This bugfix was merged after the 2019.2 release branch was frozen for the 2019.2.0 release. The fix will be part of the 2019.2.1 release.

@ptitdoc

This comment has been minimized.

Copy link

commented Jul 16, 2019

Thanks for the information. I manually applied the patch until 2019.2.1 comes out.

@xenadmin

This comment has been minimized.

Copy link

commented Aug 9, 2019

If this Bugfix was merged into 2018.3 why do I encounter this bug in 2018.3.4 with pygit2: 0.28.2 @terminalmage ?

root@salt:~# salt --versions-report
Salt Version:
           Salt: 2018.3.4

Dependency Versions:
           cffi: 1.12.3
       cherrypy: Not Installed
       dateutil: 2.5.3
      docker-py: Not Installed
          gitdb: 2.0.0
      gitpython: 2.1.1
          ioflo: Not Installed
         Jinja2: 2.9.4
        libgit2: 0.28.2
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: Not Installed
      pycparser: 2.19
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: 0.28.2
         Python: 3.5.3 (default, Sep 27 2018, 17:25:39)
   python-gnupg: Not Installed
         PyYAML: 3.12
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: 2.0.1
        timelib: Not Installed
        Tornado: 4.4.3
            ZMQ: 4.2.1

System Versions:
           dist: debian 9.9
         locale: UTF-8
        machine: x86_64
        release: 4.9.0-9-amd64
         system: Linux
        version: debian 9.9

root@salt:~# salt-run fileserver.update
Exception occurred in runner fileserver.update: Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/salt/client/mixins.py", line 387, in _low
    data['return'] = self.functions[fun](*args, **kwargs)
  File "/usr/lib/python3/dist-packages/salt/runners/fileserver.py", line 352, in update
    fileserver.update(back=backend)
  File "/usr/lib/python3/dist-packages/salt/fileserver/__init__.py", line 500, in update
    self.servers[fstr]()
  File "/usr/lib/python3/dist-packages/salt/fileserver/gitfs.py", line 133, in update
    _gitfs().update(remotes)
  File "/usr/lib/python3/dist-packages/salt/utils/gitfs.py", line 2421, in update
    self.find_file
  File "/usr/lib/python3/dist-packages/salt/fileserver/__init__.py", line 277, in reap_fileserver_cache_dir
    ret = find_func(filename, saltenv=saltenv)
  File "/usr/lib/python3/dist-packages/salt/utils/gitfs.py", line 2764, in find_file
    blob, blob_hexsha, blob_mode = repo.find_file(repo_path, tgt_env)
  File "/usr/lib/python3/dist-packages/salt/utils/gitfs.py", line 1818, in find_file
    tree = self.get_tree(tgt_env)
  File "/usr/lib/python3/dist-packages/salt/utils/gitfs.py", line 1031, in get_tree
    candidate = func(tgt_ref)
  File "/usr/lib/python3/dist-packages/salt/utils/gitfs.py", line 1861, in get_tree_from_branch
    'refs/remotes/origin/{0}'.format(ref)).get_object().tree
AttributeError: '_pygit2.Reference' object has no attribute 'get_object'
@terminalmage

This comment has been minimized.

Copy link
Contributor Author

commented Aug 9, 2019

Because they'd already branched 2018.3.4 from the 2018.3 release branch in preparation for the 2018.3.4 release, is my guess.

@xenadmin

This comment has been minimized.

Copy link

commented Aug 9, 2019

Ok, I understand that. But which is the combination of software and libraries I have to use to get gitfs back to work? #54165
(Besides that fact that it seems really strange to me that I have to solve this by manually playing around with pygit2 versions to get this to work. I wonder if I should just checkout the git repos to /srv/salt manually?)

@terminalmage

This comment has been minimized.

Copy link
Contributor Author

commented Aug 9, 2019

Pygit2 0.27.3 and any libgit2 0.27.x release should work.

The OP at the top of this pull request states that this PR is in reaction to a change that was made in Pygit2 0.27.4, so I'm not sure why this seems strange to you. You can:

  1. Use a version of Pygit2 from before 0.27.4.
  2. If you installed from upstream Debian repos, send this patch and ask them to update the package. If you installed from Salt's own repository, then maybe @dmurphy18 can patch the 2018.3.4 Debian package(s).
  3. Wait for a new Salt release with the fix.
  4. Work around temporarily by checking out into /srv/salt.
@xenadmin

This comment has been minimized.

Copy link

commented Aug 9, 2019

Thank you very much for your patience!

  1. I understand that, and I'm quite OK with Debian, but I'm not familiar with the whole Python/pip stuff. I tried installing pip3 install pygit2==0.27.3 several times, but it just fails :-/ pip3 install pygit2 on the other hand works, brings me version 0.28 which won't help me.
  2. I use the offical salt repo and would love to see 2018.3.4 patched or 2018.3.5 released asap as I can't work with the formulas which are on Git right now.
  3. I bet there are no ETAs ;-)
  4. That might be the best option right now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.