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.

Sign up for GitHub

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

Jump to bottom

proposal: cmd/go: optional mode - semantic import versioning #47034

Closed
itsjamie opened this issue Jul 2, 2021 · 6 comments
Closed

proposal: cmd/go: optional mode - semantic import versioning #47034

itsjamie opened this issue Jul 2, 2021 · 6 comments
Labels
FrozenDueToAge Proposal
Projects
Proposals (old)
Milestone
Proposal

Comments

@itsjamie
Copy link
Contributor

itsjamie commented Jul 2, 2021 •
edited

Optional Semantic Import Versioning

Earlier proposal; trying to make it more formal, with definitions around the module file changes, and constraints. #44550

Additionally, made the behavior for resolution of naked imports a keyword you flip on for your project, so that if a change were made no old code would break.

Goal

To keep all current behavior and to allow the new naked import resolution to be entirely opt-in.
Even after a user has opted-in, allow the benefits of Semantic Import Versioning, such as utilizing two majors in the same compilation unit.

Intention

Across a large project, when you do a major version upgrade of Go code, many files are touched. If that contribution includes that only some source files in the compilation unit are being upgraded and others are being left, it requires manual review outside a code review tool because no lines of code have changed in those files. It can be difficult to pin down which files are being left because there are no source code changes to those files.

The driving purpose behind this change is to increase the ability to catch issues upon reviewing source code when upgrading a major version with Semantic Import Versioning where riding between two major versions was intentional.

Alongside these code review simplifications, the other aspect is allowing for semantic import versioning to also be closer and more in line with other package management solutions as an opt-in behavior for users.

Where the import path is resolved in combination with the package management manifest, in Go, the go.mod file.

My assertion is that managing a project will be easier under the optional mode while allowing the same affordances for the compilation process that the current behavior does.

Module File Change Proposal

Define additional keyword in go.mod: optional.
Default value: false.

Extend require syntax.
require module-path module-version [default]*

The default tag is optional itself and used to indicate which version should resolve with the naked import when modules with duplicate module-path components are found.

Documentation:

Optional mode changes the default behavior of v0/v1 resolution to instead resolve to the version described in the go.mod file with the locality defined to the module compilation unit. Import statements that lack semantic import versions MUST resolve their version via go.mod. In Optional mode, when two modules share the same module-path one MUST have the default identifier.

Examples

Majority upgrade to v2 with specific files staying at v1

module example.com/mymodule

go 1.18
optional true

require (
  + example.com/othermodule v2.0.0 default
  example.com/othermodule v1.2.3
  example.com/thismodule v1.2.3
  example.com/thatmodule v1.2.3
)

file: stay-on-v1.go
import (
  + "example.com/othermodule/v1"
  - "example.com/othermodule"
)

file: upgrade-to-v2.go
import (
  "example.com/othermodule"
)

Majority stays at v1 with specific files upgrading to v2.

module example.com/mymodule

go 1.18
optional true

require (
  + example.com/othermodule v2.0.0
  + example.com/othermodule v1.2.3 default
  - example.com/othermodule v1.2.3
  example.com/thismodule v1.2.3
  example.com/thatmodule v1.2.3
)

file: stay-on-v1.go
import (
  "example.com/othermodule"
)

file: upgrade-to-v2.go
import (
  + "example.com/othermodule/v2"
  - "example.com/othermodule"
)

Constraints

Your choice of opting your module into optional mode impacts the handling of your own module but doesn't change the behavior outside your own compilation unit. SIV as it exists today would still apply outside the bounds of how your own module is handled.

Performance Impacts

Depending on the current resolution algorithm, this could make optional modules a boundary for optimizing a global resolution of package identifiers. However, given that multiple majors are allowed, I believe this will not be a blocking issue for this proposal.
@gopherbot gopherbot added this to the Proposal milestone Jul 2, 2021
@itsjamie itsjamie mentioned this issue Jul 2, 2021
Closed
@ianlancetaylor ianlancetaylor changed the title Proposal: Optional Mode - Semantic Import Versioning proposal: cmd/go: optional mode - semantic import versioning Jul 2, 2021
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Jul 2, 2021
@mvdan
Copy link
Member

mvdan commented Jul 3, 2021

I'm confused by this proposal. It seems to add more changes to the go.mod file which weren't discussed in the previous thread, so it seems like a fairly different proposal in general.

@neild
Copy link
Contributor

neild commented Jul 4, 2021

Leaving aside the specific go.mod syntax, if I understand this proposal correctly it proposes:

  • A go.mod file may indicate a specific major version of a module as the "default".
  • When an import path does not contain a major version suffix, the import resolves to the default major version of the module.
  • Import paths containing a major version suffix are interpreted as today.

For example, as I write this today the import path "rsc.io/quote" resolves to v1 of the "rsc.io/quote" module. This proposal would provide a way to make that import path equivalent to (for example) "rsc.io/quote/v3" by changing only the go.mod of the module containing the import.

This proposal permits changing the major version of a module depended upon by another module with a go.mod change, whereas today such a change requires changing every import statement referencing the old module.

This proposal makes it less possible to interpret a .go source file independently of the go.mod file of its module. Consider this file:

package example
import "example.com/mod"
func F() mod.T { return mod.T{} }

Today, I can examine this file and state that the function example.F returns a "example.com/mod".T. Under this proposal, example.F might return "example.com/mod/v2".T (or any other version of T), depending on the go.mod applying to the file containing the example.F function.

I think this proposal would benefit from more explanation of the problem that it is intended to solve. It permits selecting the major version of a module to use without rewriting import paths, but it is not clear to me why rewriting import paths is a problem. Perhaps it is a problem, but if so, it would be best to first understand why.

@itsjamie
Copy link
Contributor Author

itsjamie commented Jul 5, 2021

I'm confused by this proposal. It seems to add more changes to the go.mod file which weren't discussed in the previous thread, so it seems like a fairly different proposal in general.

@mvdan I added what I believed was required from #44550 to reduce source tree changes when you intentionally wanted to ride between two versions. This is the addition of the default attribute on the require syntax to help the resolution of an import statement. Secondarily, I made the behavior of how naked imports were resolved (v0/v1) vs go.mod based explicitly opt-in at the module level with the optional keyword.

A go.mod file may indicate a specific major version of a module as the "default".
When an import path does not contain a major version suffix, the import resolves to the default major version of the module.
Import paths containing a major version suffix are interpreted as today.

@neild with the keyword optional all modules as listed in the go.mod file become the resolution version for naked imports that would traditionally resolve to v0/v1.

The default attribute on the require syntax is simply there to help with the resolution of riding between two majors of the same import path. This was defined so that you could still take advantage of the minimized source tree changes so that reviewing the actual changes on the requirement to utilize both majors was still as minimal a change as necessary.

This proposal makes it less possible to interpret a .go source file independently of the go.mod file of its module.

Yes.

I think this proposal would benefit from more explanation of the problem that it is intended to solve. It permits selecting the major version of a module to use without rewriting import paths, but it is not clear to me why rewriting import paths is a problem. Perhaps it is a problem, but if so, it would be best to first understand why.

I've updated the intention section of the proposal to more fully describe it. But effectively, optional mode makes the resolution of Go dependencies act like other tools, as you did point out, this does mean that you cannot interpret the source file without seeing the related go.mod to determine if they were running in optional mode as I've called it. But, I appreciate the affordances that SIV allowed, so I made what I believe are the minimal modifications to the go.mod file to allow for both a complete definition, but also to make code reviewing a major version upgrade less error-prone for the common case when someone adopts a new major.

@joe-mann
Copy link

joe-mann commented Jul 5, 2021

I am sympathetic to (part of) the intention of this proposal, and whilst I am not completely convinced that this problem warrants a solution that fundamentally changes how module versioning currently works (albeit in an opt-in manner), my greater concern is that (if I have understood the proposal correctly) I don't believe that the proposed solution is an effective remedy to the stated problem.

If I understand correctly, we currently have a situation where a module upgrade may be "partially applied" to another. This may happen either accidentally or intentionally, but in either case it is desirable to make the job of reviewing such changes easier and less error prone. My problem here is that the proposal only appears to offer a partial solution to this problem, and indeed only claims to "increase the ability" to review such changes and discover possible errors. Since there is no guarantee that every file that imports (any version of) example.com/othermodule would be modified, I'm not sure that this proposal, if implemented, would deliver any meaningful improvement on the current situation since you still could not rely on code review tools alone to highlight all affected files.

If this situation warrants a solution at all, I would prefer one that properly covers the problem space, rather than one that offers an incremental improvement with no obvious route to a full solution.

@itsjamie
Copy link
Contributor Author

itsjamie commented Jul 5, 2021

I'm retracting this because an issue that I believed was true within a definition of optional SIV inside the original ticket is untrue.

A user has defined the naked import in their go.mod, the rules as laid out here; #44550 (comment) would resolve in a non-breaking way.

Therefore my thought that a necessity for an optional mode is unfounded.

@itsjamie itsjamie closed this as completed Jul 5, 2021
@rsc rsc moved this from Incoming to Declined in Proposals (old) Oct 20, 2021
@rsc
Copy link
Contributor

rsc commented Oct 20, 2021

This proposal has been declined as retracted.
— rsc for the proposal review group

@golang golang locked and limited conversation to collaborators Oct 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal
Projects
No open projects
Proposals (old)
Declined
Milestone
Proposal
Development

No branches or pull requests
6 participants
@neild @rsc @itsjamie @mvdan @gopherbot @joe-mann