-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
cmd/go: support easy way to install a remote binary while respecting remote 'replace' and 'exclude' directives #31173
Comments
I'm still not convinced that long-term use of (Note that we do support it, in the sense that it remains possible to check out the repository and run In particular, the compatibility argument for using So I would like some more detail: @rogpeppe, why does |
OK, here's some detail on the replacements used by Juju currently. I speak at some remove, because I haven't been directly involved in the Juju code base for a little while now, but I have been historically familiar with some of the issues. SummaryIt might be possible to move away from replace statements in the long term, but it's not necessarily possible in practice without hard-forking multiple external repositories, which has its own on-going cost. It's certainly not going to happen in the short to medium term, which means that replace statements are going to be a necessity for building juju for a while to come. And even the need for these particular replace statements is dropped eventually, another similar issue might well arise in the meantime. The open source world is not always clean. Some kind of replace statement is a crucial band-aid at times, and one which anyone installing the juju commands, not just developers, will need to apply. github.com/altoros/gosigma => github.com/juju/gosigmaFixes races in tests and code. https://github.com/Altoros/gosigma/pull/1/files. There's been a PR open since 2017-05-23 without response. Making a hard fork of this probably wouldn't be that hard to do, although might gopkg.in/yaml.v2 => github.com/juju/yamlA tweak to the accepted floating point syntax. No new API. A small but
holds a floating point number or a string. Changing this behaviour might Changing this unilaterally inside juju could break other code. For Changing this across the entire code base would be hard, as many external gopkg.in/mgo.v2 => github.com/juju/mgo19 commits. Some new API. The parent repository is no longer maintained, and was not generally Changing this dependency to use github.com/juju/mgo throughout |
Hi @bcmills, briefly cherry picking a couple points you made:
...unless of course part of the way you selected the module versions as the binary author is via a
That is not my concern -- modules provide 100% reproducible builds, etc. The compatibility argument is not about future breaking changes causing problems for consumers, and is more about incompatibilities that exist when a binary is being published given for example (a) semver is not always perfectly followed in practice and (b) people use v0 dependencies from the "compatibility free zone". Either of those mean MVS by itself can select problematic versions of |
That's a bit of a circular argument, though. 🙂 The central question here, as I see it, is: should |
@rogpeppe, thanks for the detail. It seems to me that since Specifically in relation to
Could you give some more detail about why changing the import path would require a major version change? (Do packages pass through |
The issue with That is, I suspect you'll want to maintain a fork of the package for compatibility with existing YAML files, but you'll want to use that fork only for the parts of the code that consume existing files. On the other hand, if you replacement really is benign for your dependencies, then perhaps the |
Yes. For example see this package. The forked mgo package can't type-alias the original Changing the I agree that replacements are not desirable and can usually be worked around in time (with negotiation with upstream maintainers, hard forks etc), but the fact remains that they are important at the time for making working products. Even if the replacement is just a "temporary" fix, it can be an important one, and I consider it important that it's possible to publish a predictably-versioned Go binary with the fix in place until the replacement can be removed. Telling the user to do their own git clone (working out the correct incantation for that is non-obvious with vanity import paths) or similar is not a good solution. I think Russ's words, aptly quoted above, bear repeating once again:
Please, let's provide that flexibility for all users of Go modules. BTW I would be perfectly happy if file-path replacements yielded an error in this case. I do consider file-path replacements suitable for developers only (and they're not a fundamental part of VCS, which replacements and exclusions in general are). |
This has an interesting interaction with #33848. #33848 proposes to use the main module's Also like a |
Perhaps #26904 provides a better way forward: one option we're considering there (from among many alternatives) is adding some new keyword to the Those paths are necessarily always import paths, not filesystem paths, which obviates the concern about treating filesystem paths and module paths differently. Unfortunately, applying only a subset of the main module's directives still poses a testing problem: it would still mean that the module could be built — and would need to be tested in — a third configuration that is neither the same as the view from inside that module, nor from an external module. |
The more I think about it, though, the more convinced I am that an “install from semi-inside” command should not be
You might even imagine that a build in this mode should not resolve missing imports: if the point is to exactly reproduce what the developer would have built, then the developer necessarily needs to record all of the versions that they intended to use. |
This seems like a good idea to me. |
See issue: golang/go#31173.
See modules issue golang/go#31173. Fixes #28
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes, including tip.
What did you do?
where:
foo
is a module withreplace
directives chosen by the module author, andfoo
is a binary command, andfoo
to build as specified by the module authorWhat did you expect to see?
The
replace
directives in thego.mod
forfoo
respected in my resultingfoo
binary.What did you see instead?
The
replace
directives in thego.mod
forfoo
are currently ignored.Summary
The suggestion in this issue is to provide some easy way to install a remote binary while respecting
replace
andexclude
directives in the remote module.This seems necessary based on observed usage, as well as this seems part of the promise from the proposal and elsewhere that "a module author is in complete control of that module's build when it is the main program being built". That promise of control by the module author seems especially important when the author is publishing a binary for use by others.
Background
As far I as I was able to follow the design discussions:
replace
andexclude
directives was important aspect of balancing out that reduction in expressiveness.For example, the "Build Control" section in the official proposal has a discussion about this, including (with emphasis added):
There are similar sentiments expressed in the initial vgo blog series, such as:
Those arguments seem reasonably compelling. It seems, however, the current Go modules system does not quite deliver on that promise when, for example:
foo
decided to use areplace
orexclude
, andfoo
is "the main program being built" via ago get foo
executed outside another module.In that scenario, the current modules system ignores the remote
replace
orexclude
infoo
. In other words, that scenario seems to illustrate not delivering on the promise of "A module author is therefore in complete control of that module's build when it is the main program being built"."Don't use replace" as an alternative solution
When the concern in this issue has been raised in the past, sometimes the response has been something like "People shouldn't really use
replace
when releasing a module". However, I think that falls short as a solution.@rogpeppe has stated for example that
juju
cannot currently be built withoutreplace
directives.In general, one could imagine that the need for
replace
directives going down over time as the ecosystem adapts modules and semver more faithfully, but it is hard to imagine the need for need forreplace
directives going so low that the need forreplace
could be approximated by zero. For example:v0
is a "compatibility free zone", yet people in practice and by necessity still usev0
dependencies. MVS can deliver incompatible results in the face of multiplerequire
directives for av0
dependency.I think the on-going need for
replace
is especially true given the purposeful reduction in expressivity elsewhere in the modules system."git clone" as an alternative solution
In discussions on this topic, a response is sometimes made along the lines of "If an author of a binary needs to use
replace
directives, they can always just update their readme to ask users to not dogo get
orgo install
and instead do agit clone
followed bygo install
". A readme in theory could also includegit clone --branch
. However, I think agit clone
solution falls short given:go
command automatically picking a good semver tag for you, with a default of@latest
(e.g., the semver-aware logic described in "Module aware go get").git
, or changing the readme if it normally specifies a more standardgo get
variation but then is only temporarily updated to specify usinggit clone
in order to respect areplace
directive for a few months while waiting for resolution of an upstream problem, etc.).go get some/cool/cmd
has proven to be popular within the Go community, including as a concrete "gateway" to Go for people who are not yet developing Go themselves.go get
nicely hides most VCS differences.Hugo is an example of an early modules adopter that currently has a
replace
directive in itsgo.mod
. It has the following installation instructions:That might be the right choice for Hugo based on its needs and the current state of modules in Go 1.12, but those instructions for example ignore the benefit of semver tags.
Personally, I would view it as a step backwards if something along those lines became the recommended solution if you have a
replace
directive.Other possible solutions
There are likely many possible solutions, but here are three sample solutions to help start conversation here.
Under all three of these options, it could be reasonable reject any filesystem-based replace directives in a remote go.mod that reach outside the module (as suggested by Bryan in #24250 (comment)). To help avoid placing a testing burden on authors to check for that,
go release
could warn if that condition is found, which is likely a good thing to do anyway as suggested in thego release
issue in #26420 (comment).Option 1
If #30515 is adopted with a new
-b
flag (for binary or bare), andgo get -b foo
ends up meaning "install foo while ignoring any current module'sgo.mod
and without altering the current module'sgo.mod
", it could be natural forgo get -b foo
to also respect anyreplace
orexclude
directives infoo
. The rationale could be that there is no other top-level module being considered aside fromfoo
when-b
is specified.The same could apply if a different spelling than
-b
is selected for #30515 (e.g., perhaps #30515 is resolved via ago get -global
,go get -clone
,go install
, etc.).Option 2
If #30515 is not adopted, then
go get foo
when run outside of a module could be redefined in 1.13 to respect anyreplace
orexclude
directives infoo
. The rationale could be that there is no other top-level module being considered aside fromfoo
when doinggo get foo
outside of a module.This behavior was suggested by multiple people for Go 1.12 (including by Bryan in #24250 (comment)), but the ultimate Go 1.12 behavior was different, including Bryan commented in CL 148517 that his 1.12 change was "as minimal a change as I could comfortably make to enable 'go get' outside of a module for 1.12". (Part of the context for Bryan's comment in the CL is I think that minimal change might have been implemented after the 1.12 freeze).
Option 3
The behavior in Option 1 and Option 2 could both be the 1.13 behavior. In other words:
go get foo
when run outside of a module in 1.13 would now respect anyreplace
orexclude
directives infoo
(in addition to respecting the remote module'srequire
directives and not changing any localgo.mod
).go get -b foo
in general means "run as if in a directory outside any module". (Under this definition, it would implyreplace
orexclude
directives in a remotefoo
are respected).Relationship to other issues
This has been discussed in multiple issue such as #27643, #24250, #30515 and others, but usually as a side topic. Perhaps this issue here can be closed as a duplicate of #30515, but @mvdan, the author of #30515, asked that this aspect be discussed in a different issue than #30515, so hence this issue is being opened now.
The text was updated successfully, but these errors were encountered: