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

proposal: modules: extend syntax go.mod to allow overriding fetch protocol #39536

Open
narqo opened this issue Jun 11, 2020 · 17 comments
Open

proposal: modules: extend syntax go.mod to allow overriding fetch protocol #39536

narqo opened this issue Jun 11, 2020 · 17 comments
Labels
Projects
Milestone

Comments

@narqo
Copy link
Contributor

@narqo narqo commented Jun 11, 2020

Update

The original proposal was build on the idea that we could extend replace directive (see the original text below).

After several rounds of discussions here in the comments, the current semantics of replace isn't about fetching the dependency but about local substitution of the source path and version (see this comment from @bcmills).

I have re-title the proposal and suggested the new syntax to solve the described problem.

Original Proposal

I propose to extend the syntax of go.mod's replace directive to support overwriting the protocol, which Go will use to fetch the module. That is

replace example.com/foo/bar => git+ssh://private.example.com/foo/bar

Background

As it’s for Go 1.14, the syntax of replace directive in go.mod allows overwriting module’s import path by defining an alternative version, alternative import path, or a path on the local FS (refer to [1]).

For non-FS-based replaces, Go fetches the replaced modules from modules proxy server as it does for other dependencies.

If a project uses private dependencies or private forks to replace a dependency, managing the modules become an extra burden for the developers, who work on the project. Even though the dependency is an attribute of the project itself, it’s the responsibility of every developer of the project to define the correct values for GOPRIVATE, GONOPROXY, etc, which alters their "normal" development workflow.

With private dependencies explicitly listed as the ones, that can't be fetched from any modules-proxy server, the project's workflow wouldn't be any different from a project, that only uses public modules (providing that a developer has the required accesses).

Specifying the protocol explicitly would also solve the issue where “go get” chooses to fetch a private dependency via HTTPS, which usually forces developers to manipulate their host’s .gitconfig, adding custom insteadOf rules to force go-get to use SSH when downloading modules from GitHub repositories.

Prior Experience

Dependency-management systems in other programming languages have similar mechanisms. For example:

  • Node.js + NPM
    package.json "dependency" syntax accepts URLs, Git URLs and some other [2].

  • Rust + Cargo
    Cargo.toml allows overwriting the source of the dependent crate, passing project’s Git URL [3].

  • Go + dep
    Gopkg.toml allows specifying dependencies' source as an alternate location, passing the URL [4].

@gopherbot gopherbot added this to the Proposal milestone Jun 11, 2020
@gopherbot gopherbot added the Proposal label Jun 11, 2020
@mvdan
Copy link
Member

@mvdan mvdan commented Jun 11, 2020

I'm a bit confused by the proposal - go.mod only deals with modules and their paths and versions. How go get then fetches those modules is an entirely separate concern.

If we start allowing full URLs in replace directives, should we also allow them in other places that usually get import paths, such as go get commands?

What should happen if I tell go.mod to use git+ssh://, but I have also set up GOPROXY=https://proxy.my.corp? What if go.mod uses https://?

Have you thought about modules where the import path is not a git server? For example, https://mvdan.cc/sh works, but git+ssh://mvdan.cc/sh will not. Conversely, git+ssh://github.com/mvdan/sh will work, since that's the repository URL. But of course, the two URLs are not the same, and go.mod is meant to contain module paths, not source code repository URLs.

@narqo
Copy link
Contributor Author

@narqo narqo commented Jun 11, 2020

Have you thought about modules where the import path is not a git server? For example, https://mvdan.cc/sh works, but git+ssh://mvdan.cc/sh will not. Conversely, git+ssh://github.com/mvdan/sh will work, since that's the repository URL. But of course, the two URLs are not the same, and go.mod is meant to contain module paths, not source code repository URLs.

I'm not sure I got this part right, please, correct me if not. The current restriction is that replace — if used with FS-based paths — must point to a location where go.mod is located (as per Modules wiki).
If you used git+ssh://github.com/mvdan/sh as the replacement for mvdan.cc/sh/v3 it would work because that replacement point to a target with go.mod file, which defines the proper module.


I didn't explicitly (or implicitly) proposed to support the whole URL. More to that, I think w/o a formal definition of what is the difference between "package", "module" and URL, the discussion might become very confusing.

We can consider adding the protocol to the existing syntax of go get, when in the module's mode. I.e. go get proto://foo/bar@version.

Regarding the conflicting cases, we should come up with the priorities, yes. I expect that a package, which is replaced with FS-based replace, isn't affected by GOPROXY. I would expect the similar behaviour for packages, whose fetch-protocol is explicitly defined.

@mvdan
Copy link
Member

@mvdan mvdan commented Jun 12, 2020

So, in a way, these URL-based replaces would be like local directory replaces - just pointing at a remote location/machine. That makes more sense to me.

We can consider adding the protocol to the existing syntax of go get, when in the module's mode. I.e. go get proto://foo/bar@version.

I would certainly expect go get to not accept URLs, because it similarly does not accept paths to modules on disk like replace directives do.

Similarly, I think supporting proto://foo/bar@version is a mistake. We don't support replace example.com/foo/bar => ../bar@v2.0.0, precisely because there is no protocol at play that would let us figure out exactly what v2.0.0 is. We just support using whatever is found at ../bar, as-is.

You can't assume that the other end will be a git repository or clone, either. This applies for local directory paths, and by extension, your URLs too I would say.

I expect that a package, which is replaced with FS-based replace, isn't affected by GOPROXY. I would expect the similar behaviour for packages, whose fetch-protocol is explicitly defined.

Assuming we piggyback off of local directory replace directives, I agree that these rules become clearer.

@narqo
Copy link
Contributor Author

@narqo narqo commented Jun 12, 2020

So, in a way, these URL-based replaces would be like local directory replaces - just pointing at a remote location/machine.

Exactly. I don't know yet the deeps of the current implementation. But when I wrote the proposal, this was exactly how though about it 👍

@narqo
Copy link
Contributor Author

@narqo narqo commented Aug 6, 2020

@mvdan sorry for the ping but, based on the discussion above, do you have any suggestions to how to move this forward?
I know @rsc and the team look through the "Proposal" issues weekly, although I expect the overall number of those issues is too big to observe.

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals Aug 6, 2020
@ianlancetaylor ianlancetaylor changed the title Proposal: modules: extend syntax of “replace” allowing to overwrite protocol proposal: modules: extend syntax of “replace” allowing to overwrite protocol Aug 6, 2020
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Aug 6, 2020

@bcmills
Copy link
Member

@bcmills bcmills commented Aug 6, 2020

The nice property of filesystem-local replace directives is that the files are already in a location where they are accessible to cmd/compile. In contrast, cmd/compile does not know how to read a git+ssh:// URL, so the go command would have to clone it somewhere — and where would it put that? (It presumably can't go in the module cache, because git+ssh:// is not a well-formed module path component and is not immutable in the same way as a versioned module anyway.)

@narqo
Copy link
Contributor Author

@narqo narqo commented Aug 7, 2020

@bcmills I feel there is a confusion or maybe I don't follow this part:

so the go command would have to clone it somewhere — and where would it put that? (It presumably can't go in the module cache, because git+ssh:// is not a well-formed module path component and is not immutable in the same way as a versioned module anyway

It could be, that the description didn't show the idea clearly — I will appreciate suggestions to improve the wording — but I expected the same behaviour for GOPRIVATE=foo/bar go build and a replace foo/bar => git+ssh://foo/bar. In the replace case though go command would bail out if foo/bar couldn't be fetched using the specified protocol.

@bcmills
Copy link
Member

@bcmills bcmills commented Aug 7, 2020

@narqo, the replace directive performs a local substitution of the source code for the given module path (and possibly version), so it changes the meaning of a path@version pair (and may change the content to have a different checksum). In contrast, GOPRIVATE does not change the meaning of a specific version — it merely instructs the go command not to fetch that version directly from the origin, but expects the content to have the same (consistent, immutable) checksum.

If what you have in mind is analogous to the behavior of GOPRIVATE, then the replace keyword is not a good semantic fit.

@narqo
Copy link
Contributor Author

@narqo narqo commented Aug 7, 2020

If what you have in mind is analogous to the behaviour of GOPRIVATE, then the replace keyword is not a good semantic fit.

Good point. My initial hopes were to piggyback the existing replace to get the behaviour of GOPRIVATE, but allowing to explicitly require this behaviour in a project's go.mod. Providing that, this goal makes sense, maybe a new explicit source directive would solve this better:

// go.mod

require foo/bar 1.0.0

source foo/bar => git+ssh://git.corp.com/user/repo

That is, source directive instructs go, that module foo/bar must be fetched from the origin git.corp.com/user/repo using git over SSH protocol.

What do you think?

@narqo narqo changed the title proposal: modules: extend syntax of “replace” allowing to overwrite protocol proposal: modules: extend syntax go.mod to allow to overwriding fetch protocol Sep 24, 2020
@narqo narqo changed the title proposal: modules: extend syntax go.mod to allow to overwriding fetch protocol proposal: modules: extend syntax go.mod to allow overwriding fetch protocol Sep 24, 2020
@narqo
Copy link
Contributor Author

@narqo narqo commented Sep 24, 2020

@bcmills @rsc I've updated the title and description pointing to latest state of the proposal. Is there something we can help with to move this forward?

@matloob matloob changed the title proposal: modules: extend syntax go.mod to allow overwriding fetch protocol proposal: modules: extend syntax go.mod to allow overriding fetch protocol Sep 24, 2020
@bcmills
Copy link
Member

@bcmills bcmills commented Sep 24, 2020

#33985 is closely related. See in particular #33985 (comment): if you have N repositories, then you potentially have N² source directives to maintain, where you could have had only O(N) .gitconfig directives.

@bcmills
Copy link
Member

@bcmills bcmills commented Sep 24, 2020

If the purpose of this proposal is to avoid needing to run an HTTPS server serving go-import directives, then it seems like instead of adding the directive to the go.mod file, you could add a .git suffix to the import path (see https://golang.org/cmd/go/#hdr-Remote_import_paths).

So I think first we need to understand where the current options are falling short: what happens if you use a .git suffix for the import paths from this repo?

@narqo
Copy link
Contributor Author

@narqo narqo commented Sep 24, 2020

I don't think I understand how .git suffix can help. The issue the proposal tries to solve is stated in the description

If a project uses private dependencies or private forks to replace a dependency, managing the modules become an extra burden for the developers, who work on the project. Even though the dependency is an attribute of the project itself, it’s the responsibility of every developer of the project to define the correct values for GOPRIVATE, GONOPROXY, etc, which alters their "normal" development workflow.

My company uses private github repositories. In order to use Go modules in our projects, every developer or a contributer, has go through the additional onboarding, where they tune their .gitconfig and asked to "properly" set environment variables in order to work with the projects.

@bcmills
Copy link
Member

@bcmills bcmills commented Sep 24, 2020

Does replace example.com/foo/bar vX.Y.Z => private.example.com/foo/bar.git vX.Y.Z not work?

If it does work, and the problem is that the versions don't track, then it seems like this could be solved less invasively using #28176.

@narqo
Copy link
Contributor Author

@narqo narqo commented Sep 25, 2020

Does replace example.com/foo/bar vX.Y.Z => private.example.com/foo/bar.git vX.Y.Z not work?

As far as I can tell it does not work with github.com and other "commonly-used VCS hosting sites" (refer to noVSCSuffix in cmd/go/internal/vcs/vcs.go). Go 1.15 gives you "invalid version control suffix in github.com/ path".

@bcmills, if a user could do

replace (
  github.com/foo/bar <version> => github.com/foo/bar.git <version>
)

forcing the use of git instead of https, that could solve it.

@bcmills
Copy link
Member

@bcmills bcmills commented Sep 25, 2020

Wait, you initially said example.com!

github.com has its own special problems because cmd/go hard-codes the protocol for that domain — that's #26134, and shouldn't require a replace directive at all to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Proposals
Incoming
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.