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: Go 2: Interface and []byte types become package scoped #32283
Comments
@itsjamie I've read your proposal a few times, but I honestly don't understand what you are proposing. A package can only be included into a compile a single time. A different major version is effectively a different package. This is represented today by separate import paths. |
@kardianos, the idea behind the proposal was to understand the issue from folks who are used to working with languages that require you only use a single version of a dependant in the build. Which as you pointed out is the incorrect mental model in a module + SIV world, since each version is effectively it's own package, therefore there shouldn't be this connection made between the two. However, if you start from the perspective that a My understanding of SIV + aliases was to allow for easier incremental changes by introducing major versions as stepping stone points for API changes without requiring you to rename your module, however, it's likely that a change to the output is hidden in the type-system via I/O contracts that don't include the package type. I understand this would be a special case, and is more deserving of perhaps a linting rule rather than full-on type system support. I was looking to see if there was an approach that covered this particular aspect. Reducing the chance of this foot-gun to developers because of how folks approach versioning given previous works. The straw man issue presented to me was a database driver that used its own scratch memory for writing information on the active query. The user had seen two versions of the same driver write to this area in slightly different ways, causing for incorrect information to be written to the database and went unfound because it was a field that recorded information for later debugging, which when it was needed, was found to be corrupted. The issue was that a |
@itsjamie Nothing prevents you from creating this linter today. How far have you taken your prototype linter that do these checks? What are your blockers? Please keep in mind the Go compilers don't do linting, so this is a firmly in the realm outside of the core Go tools.
This is not a good straw man. Who presented this straw man to you? Perhaps I could correct their understanding of how database drivers generally work and why it wouldn't be an issue (I maintain several). |
Doing it as a lint step is not ideal and was an acknowledgement that it not necessarily need be a compile time error but could fit into the automatic rewrite flow of import paths for ‘go mod’ as a warning that I/O edges had changed. This proposal was to add package typing to these I/O types so a user could tightly bind their usage with the importer as a first step to ensure future upgrades at least check to make sure these contracts didn’t change. I haven’t implemented a prototype linter to identify these cases, to answer your question though I don’t see any blocker to the implementation and running it during a import path rewrite. I believe it would be a net benefit to allow adding package identifiers to these types to encode in the type system what produced the bytes or what type of bytes a consumer can consume successfully. I hope I clarified the proposal for you! |
I see what you are getting at, but you can already do what you want if you don't use []byte, io.Reader, io.Writer nor, interface{}, but define more specific types instead, and change those when you move from v1 to v2. Like that, if your IO API or layout changes, you can signal this by changing the types involved. The user of your package will then get a compile error when they upgrade and still try to use your old I/O types. |
@itsjamie Thank you for the clarifications. Here are some issues currently:
I highly recommend you prototype your proposal. I believe source code analysis available today is sufficient to perform the checks you propose. |
What does SIV mean? |
It would help to see a small example. I don't understand the suggestion here. To the extent that I do understand it, it seems to me that it would break almost all existing packages. |
Given that most don't understand I think it's clear: The proposal is not well-formed or doesn't address an issue that is widespread. I think it needs some additional thought as to how it could be adequately described and implemented. I think I will close this proposal and refocus on if there is a clearer way to describe this. However, to answer your questions @ianlancetaylor.
SIV being Semantic Import Versioning defined here https://research.swtch.com/vgo-import.
In the article, it's recommended that the package Moauth becomes Pocoauth. The article later goes on to discuss the possibilities for automatic API updates between a semantically versioned v1 -> v2, for the parts of the API that are similar. This is what the proposal was trying to aid, to reduce the locations where an API contract currently would state it hadn't changed, even though it had. To help aid automated fixing. Doing this by adding type information to byte structures and other I/O edges. So if someone wanted to offer a v1 -> v2 call of this API, the wrapping function would need to explicitly type convert the byte to a The example I put forward using the example packages from the article would be: Moauth provides the ability in its API to read a JWE from an
The initial implementation only sends the claims section of the decoded JSON Web Token on. However in Pocoauth they decided to make the backwards-incompatible change to this API of instead sending the entire decoded JWT for this API. However, an automatic API update between Moauth -> Pocoauth would not know based on the types that anything had changed, and downstream processors of this []byte would fail after an API upgrade that seemingly had no changes to the type structure. The automatic |
As I stated before, in your example, the easiest way to make sure type EntireDecodedJwt struct {
Buffer []byte
}
func GetDecodedJWE(body io.Reader) EntireDecodedJwt Like that, in V2, callers will get a syntax error when they try to use the result of the function, because it has changed and can't be used like a normal []byte anymore. |
Background
Go currently identifies types that are created and exported by a package and prefixes their usage in external packages by the package name. One detractor of modules has been the possibility of two packages communicating through an
io.Reader
or[]byte
memory area causing issues between two versions of a package being used on the reader and writer leading to unexpected behaviour.This has been presented as the reasoning behind why a SAT solver, and therefore singular version builds lead to more predictable and less error-prone during runtime.
Abstract
I propose these related changes to the type system in Go.
mypkgv2.[]byte
.io.Reader
is aliased tomypkgv2.Reader
.This information is used during compile time at the package boundaries to ensure the consumer is importing the expected version.
Importers would hold the responsibility of re-exporting the automatically generated alias if it was used by a tertiary importer, otherwise, it would appear as the original type to reduce the virality by default.
Rationale
This is intended to fix the issue of untyped boundaries across packages. Allowing multiple versions of the same package is great, and for the majority of exported types, allows for clear compile time errors that the consumer hasn't been updated to match the importer, but there are issues at package edges with the above, and this was one thought on how this could be approached in a semantic import world that allows for multiple versions to making incremental upgrading easy, a laudable goal.
The text was updated successfully, but these errors were encountered: