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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add generic git package manager #69

Merged

Conversation

b4nst
Copy link
Contributor

@b4nst b4nst commented Jul 20, 2021

This PR add the ability to pull dependencies from another remote than github.com. It also manages private repositories (using ssh auth) and repository with a longer path (e.g. GitLab and its - infamous - subgroups).

We can obviously improve this PR (managing v0.0.0 tag, basic auth) but I wanted to first take the temperature 馃槉

If that looks OK to you, we can even drop the GitHub API + Zip stuff and use this generic function instead.

lib/mod/cache/fetch.go Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
lib/mod/cache/fetch.go Outdated Show resolved Hide resolved
lib/mod/cache/fetch.go Outdated Show resolved Hide resolved
lib/mod/cache/fetch.go Outdated Show resolved Hide resolved
lib/mod/modder/modder_vendor.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
@verdverm
Copy link
Member

Generally speaking, I tried to follow the Golang model which prefers to use host APIs over the git protocol. I believe they will fall back to git if there are no APIs specified. This is for security reasons.

lib/mod/cache/lookup.go Outdated Show resolved Hide resolved
lib/mod/cache/fetch.go Outdated Show resolved Hide resolved
lib/mod/cache/fetch.go Outdated Show resolved Hide resolved
@b4nst
Copy link
Contributor Author

b4nst commented Jul 21, 2021

Generally speaking, I tried to follow the Golang model which prefers to use host APIs over the git protocol. I believe they will fall back to git if there are no APIs specified. This is for security reasons.

I was not aware of that. We need to think about how authenticating the APIs against private repositories then.

@verdverm
Copy link
Member

Go falls back on git for private repos, there is special git config override for Go

@verdverm
Copy link
Member

Also, trying to keep this CUE Proposal in mind which will also be very Go like

cue-lang/cue#851

@b4nst
Copy link
Contributor Author

b4nst commented Jul 21, 2021

Yes, I'm actually using this for Cue projects so I'm also 馃挴 to follow cue-lang/cue#851

@b4nst
Copy link
Contributor Author

b4nst commented Jul 21, 2021

Go is using GOPRIVATE env var since 1.13 for private modules IIRC. Maybe we can use something similar here. For GitLab infinite nested level, I think you have to use the .git suffix. At least this is how I'm doing it, and the only way I know as for now.

@b4nst
Copy link
Contributor Author

b4nst commented Jul 27, 2021

@verdverm I reverted the split and send it to its own function. This function now manage the .git extension so that if you set something like:

-- cue.mod/module.cue --
module: "remote.my/owner/repo"
-- cue.mods --
module remote.my/owner/repo

cue v0.3.0

require (
  "gitlab.com/nested/insanity/repo" v0.0.0
)

replace gitlab.com/nested/insanity/repo => gitlab.com/nested/insanity/repo.git v0.0.0

hof mod vendor [cue] will vendor the right repository. It's a bit verbose but not more than my go repositories.

I've also added Zip download from API request for GitLab, in a way it has been done by you for GitHub.

Now if that changes are ok with you, what I would want to do is to clean that a little bit, rely on netrc (with a fallback on plain git ssh) instead of some magic env var for private repositories. And detect private repositories from the GOPRIVATE environment variable. We'll have to decide if we go for plain git if the remote is GitHub or GitLab, or stay with the APIs and use the password extracted from netrc as token. If we do the former, we can safely drop the bit of code extracting auth from os.Getenv in the yagu/repos/*. I think go is doing plain git for private repository, not 100% sure tho

@verdverm
Copy link
Member

I'm not keen to rely on a GO... env var for all modders. Perhaps some of this per language / code host settings can be put in the modder config / schema? What do you think?

lib/mod/cache/mod.go Outdated Show resolved Hide resolved
lib/mod/cache/mod.go Outdated Show resolved Hide resolved
lib/mod/cache/mod_test.go Outdated Show resolved Hide resolved
@verdverm
Copy link
Member

Thinking about auth, what are the various scenarios? How would this work in a CI system that might be different than local dev? (Do all the major CI systems rely on sshkeys to fetch private code?)

@b4nst
Copy link
Contributor Author

b4nst commented Jul 27, 2021

I think a netrc fallbacking to an ssh auth would give enough flexibility to tackle every systems without overthinking it. We could also implementing some sort of dynamic env, but in the end it's quite common to just echo the env in the netrc file for CI systems.

@verdverm
Copy link
Member

verdverm commented Jul 27, 2021

https://stackoverflow.com/questions/6031214/git-how-to-use-netrc-file-on-windows-to-save-user-and-password

I've typically seen CI systems using ENV VARS to expose credentials, typically through a secret system that prevents them from being printed

@verdverm
Copy link
Member

I'd say enabling ENV VARS like GITHUB_USER / GITHUB_PASS / GITHUB_APIKEY isn't pressing for this and is an easy addition if someone wants it

@verdverm
Copy link
Member

Is there a particular netrc lib to use, or will the underlying http / git.v5 packages take care of this automagically?

@b4nst
Copy link
Contributor Author

b4nst commented Jul 27, 2021

We can copy pasta the official netrc.go since it's sadly not publicly exposed

@b4nst b4nst force-pushed the feat/add-generic-git-package-support branch from 1c0b1b0 to 8dc3a67 Compare August 2, 2021 17:25
@b4nst
Copy link
Contributor Author

b4nst commented Aug 2, 2021

@verdverm some news for you. netrc usage done, *PRIVATE env set dynamically on mod. The resolving model is not so dynamic tho, at it uses the go glob matching technique. That being said it works for both go and CUE (with GOPRIVATE and CUEPRIVATE)

I've started to look at cuelang.org/go type repository. I think using an empty owner is fine. I just need to refactor URL building to always use some sort of filepath or path join function.

@b4nst
Copy link
Contributor Author

b4nst commented Aug 2, 2021

Ok should be gtg now

@verdverm
Copy link
Member

verdverm commented Aug 2, 2021

@b4nst nice

Can you merge develop? I've setup GitHub Actions which will build and "test" (most tests are failing... which we won't worry about here) This is mostly so I can see if my PR setup is working, I recall there GH introducing something about first-time permissions due to abuse

This should show it at least building and running across the various platforms

I'll give windows a manual try later tonight

@b4nst
Copy link
Contributor Author

b4nst commented Aug 2, 2021

@verdverm I did rebase _dev. I'm currently facing 1 workflow awaiting approval so I guess there is something to do on your side to approve it, right?

@verdverm
Copy link
Member

verdverm commented Aug 2, 2021

@b4nst indeed, I had missed that and we should see it running now. Thanks!

@verdverm
Copy link
Member

verdverm commented Aug 2, 2021

@b4nst This looks really good, thanks for the contribution!

I'm going to merge without manually testing on Windows and any issues I find I will open a separate ticket for.

Would you like to open a separate PR for updating the docs? We could release a new patch version after that

@verdverm verdverm merged commit 7e12c32 into hofstadter-io:_dev Aug 2, 2021
@verdverm
Copy link
Member

verdverm commented Aug 2, 2021

It should only require

@b4nst
Copy link
Contributor Author

b4nst commented Aug 2, 2021

@verdverm sure! I'll tackle that asap

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.

None yet

2 participants