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

allow option for using git CLI for git downloads #2448

Merged
merged 3 commits into from
Mar 29, 2021
Merged

Conversation

KristofferC
Copy link
Sponsor Member

@KristofferC KristofferC commented Mar 24, 2021

No description provided.

@KristofferC KristofferC changed the title WIP: allow option for using git CLI for git downloads allow option for using git CLI for git downloads Mar 24, 2021
Copy link
Member

@DilumAluthge DilumAluthge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@StefanKarpinski
Copy link
Sponsor Member

This is great. Now if we delete all the stuff that uses LibGit2, we'll really be getting somewhere!

@KristofferC
Copy link
Sponsor Member Author

Hehe, work hard to remove all external download commands in favor of a library when it comes to downloading via HTTP, and then work hard to remove a library for an external command when it comes to downloading via Git :P.

But seriously, I don't think we want to be bundling the Git executable and I think it is nice that the package manager doesn't require git to fully work. Using a library gives us nice progress bars and a nice API, and it works well in the vast majority of cases. Having an escape hatch to the CLI is probably enough.

@DilumAluthge
Copy link
Member

DilumAluthge commented Mar 25, 2021

I think it is nice that the package manager doesn't require git to fully work

Yeah, I definitely agree that Pkg shouldn't require that the user has Git installed on their system for normal usage.

I don't think we want to be bundling the Git executable

Personally, I would like us to bundle Git_jll with Julia. But as @giordano has pointed out in a different thread, Git_jll is not small (e.g. approximately 50 MiB on Windows).

@StefanKarpinski
Copy link
Sponsor Member

Unlike libcurl, which is complex but ultimately well designed and capable, the more I've dug into libgit2, the worse my opinion of it has become. It is, for example, impossible to use it securely without patching it. I think that we should preferentially use a git binary if one exists. Yes, that means we lose a progress bar for the rare operations where we still end up downloading something via git, but it seems way better than dealing with libgit2. As to the API, well, opinions are subjective, but I'm not sure that I would agree that the LibGit2 API is nice.

@DilumAluthge
Copy link
Member

Could we merge this PR now, and then we can make a separate PR that implements this:

preferentially use a git binary if one exists

Then, in that PR, we can continue the discussion about whether or not we want to default to the git binary (if one exists) or default to libgit2.

But it would be good to get this PR merged so at least people have the ability to opt-in to the Git CLI.

@KristofferC
Copy link
Sponsor Member Author

ENV var should be added to docs when #2453 is fixed.

@KristofferC KristofferC merged commit 623625b into master Mar 29, 2021
@KristofferC KristofferC deleted the kc/git_cli_option branch March 29, 2021 15:30
@fredrikekre fredrikekre added the backport 1.6 Change should be backported to release-1.6 label Mar 18, 2022
fredrikekre pushed a commit that referenced this pull request Mar 18, 2022
fredrikekre pushed a commit that referenced this pull request Mar 18, 2022
@fredrikekre fredrikekre removed the backport 1.6 Change should be backported to release-1.6 label Mar 18, 2022
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.

4 participants