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

salt.utils.gitfs: remove dulwich support, make refspecs configurable #39210

Merged
merged 12 commits into from Feb 9, 2017

Conversation

terminalmage
Copy link
Contributor

Dulwich still lacks important features, including (but not limited to) the
following:

  • Lack of the necessary support for checking out a ref needed for
    git_pillar/winrepo support

  • No support in its config objects for multivar git config items, making it
    impossible to detect when repos have multiple refspecs set for a given git
    remote

Given this information, and the fact that it trails as a distant third to
Pygit2 and GitPython, Salt will cease to support Dulwich as a git interface
moving forward.

This pull request also resolves #36913 by making refspecs a configurable
parameter for the salt.utils.gitfs.GitBase subclasses.

@cachedout
Copy link
Contributor

Some tests didn't pass here: https://jenkins.saltstack.com/job/PR/job/salt-pr-rs-cent7-n/8490/

@terminalmage
Copy link
Contributor Author

@cachedout I needed to add the new gitfs_refspecs option to the mocked opts dict in the integration tests. I also added a CLI example to satisfy the test_valid_docs failure, even though it's unrelated to this PR.

Dulwich still lacks important features, including (but not limited to)
the following:

- Lack of the necessary support for checking out a ref needed for
  git_pillar/winrepo support

- No support in its config objects for multivar git config items, making
  it impossible to detect when repos have multiple refspecs set for a
  given git remote

Given this information, and the fact that it trails as a distant third
to Pygit2 and GitPython, Salt will cease to support Dulwich as a git
interface moving forward.
@terminalmage
Copy link
Contributor Author

Still have some failing tests, will address them today.

@terminalmage
Copy link
Contributor Author

As far as I can tell, tests are passing. The failures I noted yesterday turned out to be caused by the same VM memory issues, as the test VM was unable to shell out to git to do various tasks that can't be handled by GitPython/Pygit2.

The SDB test failures were caused by 87fd95d and fixed by @rallytime in #39261. The schema tests are also unrelated and fixed in #39260 (which is pending a merge-forward). The remaining failures are also unrelated.

@rallytime
Copy link
Contributor

Yeah, those test failures listed on the Ubuntu build are already present on the branch. I think this looks good.

@terminalmage Just to double check: did you put warn_util notices on earlier release branches already to warn people about dulwich being removed? I didn't see a PR for that yet, but I didn't do a very exhaustive search. We should make sure those are added in 2016.3.

@terminalmage
Copy link
Contributor Author

I did not, I was planning this for a separate PR. I'll have that in shortly.

@terminalmage
Copy link
Contributor Author

@rallytime #39280

@rallytime
Copy link
Contributor

Awesome! Thanks @terminalmage!

terminalmage added a commit to terminalmage/salt that referenced this pull request Feb 9, 2017
Dulwich support is being removed for the Oxygen release (see saltstack#39210).
@rallytime rallytime merged commit d35c2f8 into saltstack:develop Feb 9, 2017
rallytime pushed a commit that referenced this pull request Feb 9, 2017
Dulwich support is being removed for the Oxygen release (see #39210).
gitebra pushed a commit to gitebra/salt that referenced this pull request Feb 10, 2017
* upstream/develop: (128 commits)
  Fix up bad merge
  nix: Correct documentation for gc
  nix: Check for nix-collect-garbage
  Add nix module to Salt
  Inform on which python version we are running on
  Restore full user permissions when failing to delete
  Use six.moves to import cStringIO
  Remove unused imports
  salt.utils.gitfs: remove dulwich support, make refspecs configurable (saltstack#39210)
  Purged trailing whitespace
  Fix the new jinja_test failures (saltstack#39262)
  provide the possibility to put extra modules into the thin
  Add __utils__ to sdb unit test (saltstack#39261)
  support dryrun for dockerng.sls_build
  Pylint fix
  _device_mismatch_ignored will never be True
  Do boolean check instead len() to test whether string is empty
  salt/states/blockdev.py
  Remove blank line
  Add safety mechanism in shutdown fucntion and add passwd as valid argument
  ...
@terminalmage terminalmage deleted the issue36913 branch April 25, 2017 19:26
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

Successfully merging this pull request may close these issues.

Support custom refspecs in GitFS
3 participants