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

WIP: LibGit2 support for Git credential helpers #20725

Closed
wants to merge 8 commits into from

Conversation

omus
Copy link
Member

@omus omus commented Feb 21, 2017

Refactored the LibGit2 credential callback to add support for Git credential helpers. Since the credential callback code was mostly untested I also wrote tests for this code and fixed several issues that were discovered by the new tests. Changes include:

  • Support LibGit2 configuration iterators
  • Use LibGit2 callback payload to keep track of state
  • Add Git credential helper support to HTTPS authentication
  • Avoid sending invalid credentials multiple times to remote servers
  • Support use of EOF character (^D) to abort credential prompt
  • Limit amount of re-prompts
  • Revise when cached credentials are accepted or rejected
  • Testing framework which automates credential prompt responses and does not require external servers

Additional work to do:

  • Make tests compatible with Windows Disable tests for Windows
  • Disable git credential helper tests on systems where command-line git isn't installed
  • Add more tests for credential approval/rejection
  • Add more git credential tests
  • Document credential prompt abort using ^D

@omus omus added libgit2 The libgit2 library or the LibGit2 stdlib module test This change adds or pertains to unit tests labels Feb 21, 2017
@omus omus changed the title LibGit2 support for Git credential helpers WIP: LibGit2 support for Git credential helpers Feb 21, 2017
@omus omus added this to the 0.6.0 milestone Feb 21, 2017
@ararslan
Copy link
Member

Is this actually release blocking for 0.6? If not, 1.0 might be the more appropriate milestone. (cc @tkelman)

@omus
Copy link
Member Author

omus commented Feb 21, 2017

I can see the argument for not having this be blocking for 0.6. However, this work does include numerous fixes and tests I would like to see included in a 0.6 release.

Additionally, I require the git credential helper feature for running a GitLab CI with support private package dependencies on Julia 0.6

@tkelman
Copy link
Contributor

tkelman commented Feb 21, 2017

If the git credential helper relies on command-line git, then I don't think it should live in base. Can you separate the fixes, tests, and features here? This is a pretty big PR to be opening as we're basically one build-fix away from feature freeze.

omus added 8 commits February 21, 2017 16:25
Found a bug where scp-like syntax would interpret a path as a port.
Changes include:
- Use LibGit2 callback payload to keep track of state
- Avoid sending invalid credentials multiple times to remote servers
- Allowing accept or reject of cached credentials
- Testing framework which automates credential prompt responses and
  does not require external servers
@omus omus force-pushed the cv/libgit2-cred-refactor branch from f3bf5b0 to b0bf68d Compare February 21, 2017 23:15
@omus
Copy link
Member Author

omus commented Feb 21, 2017

I broke up the PR into smaller commits. Unfortunately GitHub isn't showing the commits in the right order. Here's a listing in the correct order:

  1. Improve scp-like syntax parsing (1f3b39b) (bugfix)
  2. Refactored LibGit2 credential callback (6ea2a68) (bugfix)
  3. Limit amount of re-prompts (73dc088) (feature)
  4. Use of EOF character (^D) to abort cred prompt (c661d60) (feature)
  5. Add LibGit2 support for config iterator (a462aeb) (git credential helper)
  6. Add GitConfig into RemotePayload (4ac043d) (git credential helper)
  7. Add framework for Git credential helper support (409bb3b) (git credential helper)
  8. Add Git credential helper support for HTTPS (b0bf68d) (git credential helper)

@omus
Copy link
Member Author

omus commented Feb 22, 2017

I'll be breaking this PR up into multiple PRs to make things faster and easier to review.

@omus
Copy link
Member Author

omus commented Feb 24, 2017

Removing 0.6.0 milestone as this is no longer critical. Details in #20769

@omus
Copy link
Member Author

omus commented Oct 26, 2017

With the merger of #23824 this PR can now be retired.

@omus omus closed this Oct 26, 2017
@omus omus deleted the cv/libgit2-cred-refactor branch October 26, 2017 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libgit2 The libgit2 library or the LibGit2 stdlib module test This change adds or pertains to unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants