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

format go-list do not marshal #2

Closed
wants to merge 1 commit into from
Closed

format go-list do not marshal #2

wants to merge 1 commit into from

Conversation

colin-sitehost
Copy link

Currently the exec.Cmd that is run contains formatting instructions for
-json and -f, in >go1.17 this just works, as -json takes precidence.
This has becomes an error in go1.17, so we should pick one and just use
that. My preferences is to have go-list do the parsing, use -f, and just
chomp the string before passing it to ioutil.ReadFile.

Currently the exec.Cmd that is run contains formatting instructions for
-json and -f, in >go1.17 this just works, as -json takes precidence.
This has becomes an error in go1.17, so we should pick one and just use
that. My preferences is to have go-list do the parsing, use -f, and just
chomp the string before passing it to ioutil.ReadFile.
@ldez
Copy link
Owner

ldez commented Jul 14, 2021

Could you revert the change on the go.mod? thank you

@colin-sitehost
Copy link
Author

I would be happy to, but per the spec, the go directive is supposed to indicate the version a module was written with, and these changes are for the go1.17 toolchain, tested against it, and generated with it.

@ldez
Copy link
Owner

ldez commented Jul 14, 2021

But this works for go <1.17, and it's not a go1.17 specific feature, the behavior doesn't change between go1.16 and go1.17.
It's not related to the code but to a call to a command line.

The intention behind the sentence A go directive indicates that a module was written assuming the semantics of a given version of Go. is clear to me: define the minimal version supported by the module.

In fact the only require change is to remove -f {{.GoMod}}, because currently it is just ignored.

The arguments to list -m are interpreted as a list of modules, not packages.
The main module is the module containing the current directory.
The active modules are the main module and its dependencies.
With no arguments, list -m shows the main module.
With arguments, list -m shows the modules specified by the arguments.

@ldez
Copy link
Owner

ldez commented Jul 14, 2021

I fixed the problem in bedb342

@ldez ldez closed this Jul 14, 2021
@colin-sitehost
Copy link
Author

I cannot find the reference, but usually see the go directive used to mean "the maximum version supported by the module".

@ldez
Copy link
Owner

ldez commented Jul 14, 2021

When you create a module, the go command setup the go directive to the current Go version, because you are writing the code with your current Go version, it doesn't mean that the code works with the previous versions of Go (ex: if you are using io instead of ioutils).

Then it's weird to indicate a maximum version because it's not useful information, in some points of view it can be seen as false information because it's impossible to say that your code works with all the previous versions of Go.

For me, everybody uses the go directive to define the minimal version, I have a lot of examples.

The specification seems clear to me:

If an older Go version builds one of the module's packages and encounters a compile error, the error notes that the module was written for a newer Go version.

@colin-sitehost
Copy link
Author

colin-sitehost commented Jul 14, 2021

I agree that people frequently try and use it to define a minimal version, since it can be used as a gate that "affects use of new language features" thereby enforcing some notion of backwards compatibility. However, it "was originally intended to support backward incompatible changes to the Go language", and this only works if you use it to mean I support version up to go 1.17. This is also further muddied by the desire by the core team to only support current release -1, meaning they only care about go 1.15 and go 1.16 currently. Also, there is a breaking change coming in 1.18.

I thought there was an issue about this, but we should really pin down what the version means: min, max, or something else.

@colin-sitehost
Copy link
Author

colin-sitehost commented Jul 14, 2021

There is a ticket: golang/go#30791; thre does not seem to be any consensus.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants