-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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: deprecate a major module version #40357
Comments
If both #40323 and this proposal were accepted:
|
To the best of my knowledge, yes: when a module consumer first adds a version of a module to their project which meets the respective criteria.
Yes. |
To me, this has the same issue as proposal #40323: a one-time warning when the dependency is added is easy to miss. Module deprecation seems like a good thing to surface in |
I don't really understand these points. When I add a new module dependency to my project, and then run whichever Maybe we have different workflows, that's possible. I'd love to understand yours. |
If I make a change to a file and then run |
We're not talking about just making a change, we're talking about the first time you add a new dependency to a project, which is much less routine.
OK, I think it's clear that the changes in this proposal are likely to be missed by your workflow. I'm not convinced that (a) your workflow is typical, or (b) that in any case this should impact the proposal. |
FWIW my workflow is similar. When making changes to a project that might be using new dependencies, the test and/or build output can be verbose and I fear that these warnings would be lost in the noise. The warnings wouldn't be repeated AIUI, so I'm not convinced that this feature is as useful as it could be. As usual, warnings are often ignored. I think I'd prefer to have a feature that made it an error to add a new direct dependency that's deprecated (either opt-in, or as default behaviour with a way to opt out of it). |
@peterbourgon can you provide more detail on your workflow? I ask because from my perspective and experience of using That's not to say we shouldn't support a notification when invoking
Agreed. That said, getting UX for cc @stamblerre for |
I was quite literally just typing this out! 🙂 My workflow when it comes to new libs is to copy out of I'll also bring up that I think some users (at least with gopls in VS Code) may be running tests in the editor, be it by running the handy "run tests with coverage" commands or a code lens to run a specific test, in which case they may never see |
Most of the time my workflow is like this, too. I'd say about 30% of the time I manually To the broader point, editor integration is important, and I would indeed hope/expect the warning message to be parsed and surfaced in a more visible way in the editor by gopls. I'm not sure if that's a little warning dialog, like when you need to recompile your tools; or a prominent message postfixed to the output pane; or what, exactly. But it seemed like an over-reach for the proposals to dictate how that should work. Once the message is in the output, tooling can surface it however makes sense. |
To zoom out a bit, I think we'd be happy to amend the proposal(s) to describe additional places or ways to emit the warning. The mechanisms listed were simply the most obvious to us at time. We agree that editor integration and pkg.go.dev are important. I'm not sure about |
Thanks.
I suspect you are rather more diligent therefore than most, myself included! If a command succeeds, I don't care much for the output unless that command's principal job is to show me output (e.g. If
I would be surprised if a
I suspect @bcmills did indeed mean |
Interesting, I've never heard of this tool before. Seems like a kind of release linter? If so I agree that it makes sense to surface there.
I didn't get that from @bcmills comments, but with the point clarified, it seems reasonable to me, too. My intuition is that a dialog generated when the deprecated module is first added to the file would be best, if gopls can edge-trigger on that condition. And/or squiggly-underlining the deprecated modules in the require block of the go.mod at a warning (yellow) level? |
Just a thought: How about adding a (In the question of "when to show a warning", I kinda agree with everyone here, I feel that when initially adding it would be the semantically correct point in time, but I also would almost certainly miss it if that was the only place. So I don't really have an opinion, but as long as the warning is exposed to me in |
@jayconrod announced it some time ago. We've discussed it a number of times on the golang-tools calls in various contexts.
This is exactly what I was referring to. The challenge comes in storing the state of a dismissed warning. Should that also be dismissed for other authors of the package (c.f. @Merovius's comment) or is that something more personal? I appreciate this is detail that one might consider beyond the scope of the proposal as it stands, but to my mind in this instance we need to work backwards from the UI/UX of various tools where this detail is surfaced and then derive the scheme of changes to |
In my mind, the dialog on first add is naturally dismiss-able, but the squiggly should be perpetual.
Interesting. Where would that logic live? I guess in the go tool? |
I'd assume so - my understanding is, that the best way to programmatically edit |
It seems like deprecation notices should be shown in the same situations as retraction notices #24031, namely with I'd advocate for showing both notices in the same situations: when the user checks for upgrades, and when the user performs an upgrade, regardless of what's in the cache. ( |
Perhaps also plain |
@peterbourgon Sure. The only reason I was conservative in my suggestion was that I know it is controversial that |
Thanks everyone for your feedback so far. It has definitely helped me to better understand the various ways in which people manage their module dependencies. @rogpeppe wrote:
It is possible to make this an error, if the package maintainer wants it to be. They'd do this by breaking the build of (or perhaps retracting?) the package at the version that is deprecated. I think it's desirable that we leave this judgment—of whether it's worth breaking new users—to the upstream package maintainer. Note that this proposal doesn't preclude gopls from doing something useful here. This proposal is about adding a deprecation mechanism for modules that gopls may (and likely will) do something useful with. If we assume that we do want module deprecations, then IMO the first step of incorporating this new feature is to put it in the go tool, and only then broaden the scope to gopls and other tools once we have confidently taken that first step. @Merovius wrote:
I quite like this idea. In my workflow I always scrutinize changes to In that case, perhaps there needs to be a mechanism to look up a deprecation message for a given module. Right now it's not straightforward to examine a @jayconrod wrote:
This seems reasonable to me, and from an implementation POV may be the most elegant approach (check for both retractions and deprecations at the same time). If we mostly agree both that 1) retractions should be surfaced at all and 2) that deprecations are a feature we want to provide, then we should all be on board with surfacing them to the user in the same way. |
@bcmills @jayconrod @matloob, what are your opinions about whether we should move forward with this? I can't quite tell from the discussion above. |
I'm in favor of the I'm also in favor of @Merovius's suggestion to have the I'm ambivalent on printing warnings to the console. I certainly don't think that should be the only means of surfacing deprecation, but it seems unobjectionable if accompanied by something more durable (such as the aforementioned consumer-side |
Does this issue cover this particular use case, or should I open another one? If someone publishes a broken version of a module, it seems at the moment there's no current way to mark it broken or deprecated. All the other package managers I'm working with (NuGet, NPM and PyPI) currently have this support |
@jaxxstorm Module version retraction (#24031) will cover that use case. It will ship in Go 1.16. |
Change https://golang.org/cl/301089 mentions this issue: |
Change https://golang.org/cl/306331 mentions this issue: |
Change https://golang.org/cl/306332 mentions this issue: |
Change https://golang.org/cl/306334 mentions this issue: |
Change https://golang.org/cl/306333 mentions this issue: |
Instead of accepting bool flags, ListModules now accepts ListMode, a set of bit flags. Four flags are defined. listRetracted is split into ListRetracted and ListRetractedVersion to avoid ambiguity with -u, -retracted, and -versions. For #40357 Change-Id: Ibbbe44dc1e285ed17f27a6581f3392679f2124fb Reviewed-on: https://go-review.googlesource.com/c/go/+/306331 Trust: Jay Conrod <[email protected]> Reviewed-by: Michael Matloob <[email protected]>
Extract queryLatestVersionIgnoringRetractions, which returns the version we should load retractions and deprecations from. This will be shared with CheckDeprecations. Rename ShortRetractionRationale to ShortMessage. This will be used to shorten deprecation warnings as well. For #40357 Change-Id: Ic1e0c670396bdb3bd87c7a97cf2b14ca58ea1d80 Reviewed-on: https://go-review.googlesource.com/c/go/+/306332 Trust: Jay Conrod <[email protected]> Run-TryBot: Jay Conrod <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Michael Matloob <[email protected]>
Deprecation notices start with "Deprecated:" at the beginning of a line and run until the end of the paragraph. This CL reuses text extraction code for retraction rationale, so the same rules apply: comment text may be from the comments above a "module" directive or as a suffix on the same line. For golang/go#40357 Change-Id: Id5524149c6bbda3effc64c6b668b701b5cf428af Reviewed-on: https://go-review.googlesource.com/c/mod/+/301089 Trust: Jay Conrod <[email protected]> Run-TryBot: Jay Conrod <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Bryan C. Mills <[email protected]> Reviewed-by: Michael Matloob <[email protected]>
To pull in CL 301089. For #40357 Change-Id: I8aa9bc8d7a75f804adc7982adaaa1c926b43d0ef Reviewed-on: https://go-review.googlesource.com/c/go/+/306333 Trust: Jay Conrod <[email protected]> Run-TryBot: Jay Conrod <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Bryan C. Mills <[email protected]>
A module is deprecated if its author adds a comment containing a paragraph starting with "Deprecated:" to its go.mod file. The comment must appear immediately before the "module" directive or as a suffix on the same line. The deprecation message runs from just after "Deprecated:" to the end of the paragraph. This is implemented in CL 301089. 'go list -m -u' loads deprecation messages from the latest version of each module, not considering retractions (i.e., deprecations and retractions are loaded from the same version). By default, deprecated modules are printed with a "(deprecated)" suffix. The full deprecation message is available in the -f and -json output. 'go get' prints deprecation warnings for modules named on the command line. It also prints warnings for modules needed to build packages named on the command line if those modules are direct dependencies of the main module. For #40357 Change-Id: Id81fb2b24710681b025becd6cd74f746f4378e78 Reviewed-on: https://go-review.googlesource.com/c/go/+/306334 Trust: Jay Conrod <[email protected]> Reviewed-by: Bryan C. Mills <[email protected]> Reviewed-by: Michael Matloob <[email protected]>
Change https://golang.org/cl/309337 mentions this issue: |
Change https://golang.org/cl/309529 mentions this issue: |
Change https://golang.org/cl/309549 mentions this issue: |
The object representing a module directive may have a "Deprecated" field but not a "Version" field. Other objects representing module versions have "Path" and "Version" fields but not "Deprecated". For #40357 Change-Id: Iad8063dfa6f7ceea22981a8a8f99e65fa3b7ffa0 Reviewed-on: https://go-review.googlesource.com/c/go/+/309337 Trust: Jay Conrod <[email protected]> Run-TryBot: Jay Conrod <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Bryan C. Mills <[email protected]>
For golang/go#40357 Change-Id: Iba0e912123287ed4fe5b0a34acca76b883046529 Reviewed-on: https://go-review.googlesource.com/c/website/+/309529 Trust: Jay Conrod <[email protected]> Reviewed-by: Bryan C. Mills <[email protected]>
Proposal: A Mechanism For Deprecating Modules
Authors: @peterbourgon, @adg
This proposal is a replacement for part of #38762. The other part is replaced by #40323.
Problem
Currently, there is no standard way for package authors to signal to consumers that a major version of a package is deprecated and no longer supported. Package consumers can easily inadvertently select an unmaintained major version (see #40323) and miss important new features or bug fixes.
Proposal
We propose adding a
Deprecated:
comment for modules that is recognized by go tooling to flag that a module version as deprecated. This is parallel to theDeprecated:
comments that can be used to flag a package or identifier as deprecated.When selecting a deprecated module version, the go tool will print a note to the user alerting them to the deprecation, and printing the deprecation text provided by the module author. Authors can use this notification to direct package consumers to more recent major versions, or an entirely new module that replaces the deprecated one.
As with package-level deprecation comments, module deprecation comments can span multiple lines.
Example
The author of module
github.com/peterbourgon/ff
has released a new major versionv2
(current versionv2.0.1
), and wants to deprecate the old major version treev1
(latest versionv1.7.4
).In the
v1
branch, they add a module declaration to thego.mod
file, above the module declaration:They commit this change and tag it as a new patch release
v1.7.5
.There are several ways this deprecation message will or will not be displayed to package consumers.
If a consumer is fetching the module at major version
v1
for the first time, the command succeeds but a deprecation warning is printed as part of go get's output:If a consumer is currently using the module at version e.g.
v1.7.4
, no warnings will be printed when that module version is fetched. However, if the consumer tries to upgrade the dependency, they will see the deprecation warning. Notably, the upgrade will still succeed.If a consumer is currently using the module at major version 1 (e.g.
v1.7.4
), and runs a command that lists the latest version available in that major version tree (i.e.v1.7.5
), the warning is printed.Integrations
pkg.go.dev
Module deprecation warnings should be prominently displayed on pkg.go.dev. It is worth exploring the linkification of module paths when displaying module deprecation notices.
Editor integration
If and when editors upgrade dependencies, these deprecation notices should be surfaced to the user as appropriate.
The text was updated successfully, but these errors were encountered: