Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Mar 30, 2020

What does this PR do?

This PR adds a fix for Issue #56451

What issues does this PR fix or reference?

#56451

Previous Behavior

Using annotated tags with PyGit2 as backend for the gitfs_remote feature just did not work as checkout failed.

New Behavior

Using annotated tags with PyGit2 as backend for the gitfs_remote feature is possible, checkouts work.

Tests written?

Yes

Commits signed with GPG?

Yes

@ghost ghost self-requested a review as a code owner March 30, 2020 15:49
@ghost ghost requested a review from DmitryKuzmenko March 30, 2020 15:49
@DmitryKuzmenko
Copy link
Contributor

@afischer-opentext-com thank you for PR. The code looks good for me. Thanks for the test. Could you please check my review comments?

@DmitryKuzmenko
Copy link
Contributor

DmitryKuzmenko commented Mar 31, 2020

@afischer-opentext-com the tests failures are related. Could you please protect the test to run in non-pygit2 supported environments? You can find an example here:

@skipIf(not HAS_PYGIT2, 'pygit2 >= {0} and libgit2 >= {1} required'.format(PYGIT2_MINVER, LIBGIT2_MINVER))
@skipIf(salt.utils.platform.is_windows(), 'Skip Pygit2 on windows, due to pygit2 access error on windows')

@ghost
Copy link
Author

ghost commented Mar 31, 2020

@DmitryKuzmenko , is everything okay with the Jenkins host? I tried to look at the failured jobs but found so far no evidence that the changes are responsible for this. I have to admit not every log was accessible and some jobs are still running (e.g. Windows 2019), even if reported as failure here.

@DmitryKuzmenko
Copy link
Contributor

re-run full amazon2-py2
re-run full centos6-py2
re-run full centos7-py2-m2crypto
re-run full centos7-py2-pycryptodomex
re-run full centos7-py2-tcp
re-run full fedora30-py2
re-run full macosxmojave-py2
re-run full windows2016-py2
re-run full windows2019-py2
re-run full ubuntu1604-py3
re-run full ubuntu1604-py3-pycryptodomex
re-run full ubuntu1604-py3-tcp

@ghost
Copy link
Author

ghost commented Apr 1, 2020

Thank you! The left two jobs seem to be again timeout related.

DmitryKuzmenko
DmitryKuzmenko previously approved these changes Apr 1, 2020
@DmitryKuzmenko
Copy link
Contributor

@afischer-opentext-com could you please resolve merge conflicts?

@dwoz dwoz merged commit 65dd57a into saltstack:master Apr 12, 2020
@sagetherage sagetherage added the ZRelease-Sodium retired label label May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ZRelease-Sodium retired label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants