pkg / errors

Simple error handling primitives
https://godoc.org/github.com/pkg/errors
BSD 2-Clause "Simplified" License
8.18k stars 691 forks source link

Create go.mod #203

Closed hanzei closed 4 years ago

puellanivis commented 5 years ago

https://github.com/pkg/errors/pull/191

Please provide concrete reasons why it is necessary. go.mod has only applied to builds within that module, and does not propagate constraints transitively. This package only uses standard library imports.

As such, there is no cause for a go.mod.

hanzei commented 5 years ago
  1. There is no downside of having one.
  2. It helps the Go Modules eco system.
  3. https://github.com/golang/go/wiki/Modules#should-i-still-add-a-gomod-file-if-i-do-not-have-any-dependencies
  4. It states that there are not dependencies.
puellanivis commented 5 years ago

The linked to PR, and the transitively linked to Issue all already link to that resource.

Note my comment:

Please provide concrete reasons why it is necessary.

A decision has already been made with the given reasons. None of them suggest “necessity”. You state no new reasons. :woman_shrugging: I do not understand why you would expect a different response.

davecheney commented 5 years ago

Thank you but no

There is no downside of having one.

Every file has a cost to maintain. For example, even tho this file is conceptually empty it contains the line "go1.12". Do we have to update that in a year when Go 1.12 is no longer supported? Not having one lowers that cost.

It helps the Go Modules eco system.

How? Is there something is broken in the Go module system because this file is not present?

https://github.com/golang/go/wiki/Modules#should-i-still-add-a-gomod-file-if-i-do-not-have-any-dependencies

That page is a gestalt of community contributions, mostly well meaning, but non normative.

It states that there are not dependencies.

Why do I have to repeat what the code says already? Having to repeat this information in two places is a recipe for them getting out of date.

hanzei commented 5 years ago

Every file has a cost to maintain. For example, even tho this file is conceptually empty it contains the line "go1.12". Do we have to update that in a year when Go 1.12 is no longer supported? Not having one lowers that cost.

If this library drops the support for go 1.12, you don't have to alter this line. The line is just there for future go versions.

How? Is there something is broken in the Go module system because this file is not present?

It helps the eco system by showing that this library declared a go modules and is using the concept of go modules.

That page is a gestalt of community contributions, mostly well meaning, but non normative.

But in the end it's the common knowledge and agrement of the community, which might be more valuable the rules from the go team.

Why do I have to repeat what the code says already? Having to repeat this information in two places is a recipe for them getting out of date.

Because reading to all go files to find out if the codebase has dependencies is a cumbersome task. Please note that go.mod get updated every time you do any go action like go test. Hence it's very hard to miss an update in this file.

Another reason is that with library doesn't work outside of GOPATH.

Also it declares a module name which might become independed of actual URL of the repo.

puellanivis commented 5 years ago

If this library drops the support for go 1.12, you don't have to alter this line. The line is just there for future go versions.

False. This line breaks building on 1.11.x where x < 4: https://github.com/golang/go/issues/30446 The reason the travis-ci passes for 1.11 is because it’s skipping go mod support due to being built in the GOPATH within travis.

Because reading to all go files to find out if the codebase has dependencies is a cumbersome task. Please note that go.mod get updated every time you do any go action like go test. Hence it's very hard to miss an update in this file.

Automated file churn is still unnecessary churn for such a simple package.

Another reason is that with library doesn't work outside of GOPATH.

This is false. The package works just fine outside of GOPATH. Since it has no dependencies or subpackages there was never any reason to do any development of this package inside GOPATH anyways.

Also it declares a module name which might become independed of actual URL of the repo.

There is no reason why this package would ever need to use any method of reference other than the simple URL of the repo. Why make things more complicated, when a simple adequate solution exists?

hanzei commented 5 years ago

False. This line breaks building on 1.11.x where x < 4: golang/go#30446 The reason the travis-ci passes for 1.11 is because it’s skipping go mod support due to being built in the GOPATH within travis.

This is a bug, that was fixed in go 1.11.4. People using 1.11.x with x < 4 should just update there version.

But I've removed the line. Now it also compatible with all go 1.11 versions.

There is no reason why this package would ever need to use any method of reference other than the simple URL of the repo. Why make things more complicated, when a simple adequate solution exists?

There has been a couple of times where Repos changed there URL. Who knows how long GH will exists? Module names make a repo independent from the URL.

davecheney commented 5 years ago

If this package moves to a different GitHub repo then it is not the same package. This property is at the heart of SIV — things with different names are different.

On 8 Aug 2019, at 00:00, Hanzei notifications@github.com wrote:

False. This line breaks building on 1.11.x where x < 4: golang/go#30446 The reason the travis-ci passes for 1.11 is because it’s skipping go mod support due to being built in the GOPATH within travis.

This is a bug, that was fixed in go 1.11.4. People using 1.11.x with x < 4 should just update there version.

But I've removed the line. Now it also compatible with all go 1.11 versions.

There is no reason why this package would ever need to use any method of reference other than the simple URL of the repo. Why make things more complicated, when a simple adequate solution exists?

There has been a couple of times where Repos changed there URL. Who knows how long GH will exists? Module names make a repo independent from the URL.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

hanzei commented 5 years ago

If this package moves to a different GitHub repo then it is not the same package.

Well, this go modules it can stay the same. It's about the modules, not where the module lives.

arp242 commented 4 years ago

Please provide concrete reasons why it is necessary.

The biggest advantage is that it allows developers to work with this package outside of GOPATH. The wiki link that was mentioned before – and rather curiously handwaved away with "this is just a community contribution" – mentioned that already, but to expand in some more detail on this:

I typically put my code in ~/code. I've been doing that for over a decade, and it works well with most Go packages these days too, as most sport a go.mod file. As a result my ~/go/src has been empty for quite a while and I work from ~/code only, and I am much happier for it. But with this repo it doesn't work as the go toolchain has no idea that ~/code/errors is the github.com/pkg/errors package, resulting in errors:

[~/code/errors](master)% go test
# _/home/martin/code/errors
package _/home/martin/code/errors_test
        imports github.com/pkg/errors: cannot find package "github.com/pkg/errors" in any of:
        /usr/lib/go/src/github.com/pkg/errors (from $GOROOT)
        /home/martin/go/src/github.com/pkg/errors (from $GOPATH)
FAIL    _/home/martin/code/errors [setup failed]
FAIL

[~/code/errors](master)% GO111MODULE=on go
go: cannot find main module, but found .git/config in /home/martin/code/errors
        to create a module there, run:
        go mod init

This has the potential to become even more confusing if the errors_test package from a module will pick up the github.com/pkg/errors package from GOPATH, and while I've been working with Go for a while and am familiar with the "GOPATH history", newer devs are less likely to be.

While "GOPATH mode" probably won't be removed for a while, I believe that "deprecation" of it is planned (originally 1.13, but not 100% sure what the current timeline is). Not supporting a module-based workflow seems rather strange to me, and the previous claim that "The package works just fine outside of GOPATH" doesn't seem to be the case, at least not on my system with Go 1.13.5.

Note: the tests actually fail for me in module mode on my system (it works in GOPATH), so that needs fixing as well, but that's a separate issue.

puellanivis commented 4 years ago

I’ve worked in Go since 1.2, and have never programmed in the GOPATH, so I am more than well aware of the obstacles and issues faced by developing outside of the GOPATH. The problem with your argument is that none of the difficulties of running the tests for this package outside of the GOPATH would actually be addressed by adding a go.mod file.

First: if a package does not have any dependencies, then GOPATH or not, go.mod or not, it should not affect the running of tests. Properly done, none of the tests for this package should need a go.mod or be built inside of the GOPATH.

So, why do we get cannot find package "github.com/pkg/errors" issue then? Because example_test.go is trying to be compiled. But this file does not actually include any tests, so there really is not any reason to even try to compile it in the first place. So, this issue could be mitigated by any of: simply go get github.com/pkg/errors; put it into a subdirectory like examples; renaming it to something other than *.go; or perhaps most interesting, just adding a build flag that would not be set during tests, like // +build example.

So, after addressing how to deal with the unintended compilation of example code, we come to the real issue that you’ve also pointed out, which is facing us when attempting to run the tests outside of GOPATH: the file paths being reported in the stack trace are not static, and so we end up with a ton of string value mismatches. But if we fix these, then we do not need the go.mod anyways, as it should work with any arbitrary path.

And this is where we get to the reason why your argument does not indicate a necessity for a go.mod file: if we address these two issues, then we do not need to build in GOPATH, nor do we need a go.mod file. It should work without both.

arp242 commented 4 years ago

It seems to me that running example tests with a standard no-frills go test would be better, no? Simply adding a standard go.mod is simple and practically maintenance-free. The amount of time being spent here writing essays on why not to add a go.mod file dwarfs the miniscule maintenance cost that the file will introduce 🤷‍♂️

Hiding the example tests – which run fine with a go.mod btw – behind a build flag is much more of a burden to everyone working with this than maintaining that file.

Frankly I find the vigorous opposition to a simple pragmatic fix here quite baffling :confused:

puellanivis commented 4 years ago

We do not actually every compile or use the example_test.go code. The only time it compiles is during tests, because of the _test suffix.

The exact same amount of issues with putting it behind a build tag, would be the exact same amount of maintenance of a go.mod file.

And neither of these actually solve the issue of being able to run the tests outside of GOPATH so, 🤷‍♀

awfm9 commented 4 years ago

I was just passing through here and would like to add: adding a go.mod file would allow projects to vendor a real version of this library, rather than using a pseudo-version for the dependency, which is always a bit ugly. However, seeing how the switch to standard-library error-wrapping almost coincides with the introduction of modules, there might not be much use.

puellanivis commented 4 years ago

We do not need a go.mod to allow importing with a semantic version, the only thing necessary for that is to tag releases with a vX.Y.Z format, as such from one of my projects:

$ grep "errors" go.mod 
    github.com/pkg/errors v0.9.1

If you have a pseudo-version, it’s because you have either pinned a specific commit, or you targeted master or other such branch, than results in pinning a specific commit.

awfm9 commented 4 years ago

We do not need a go.mod to allow importing with a semantic version, the only thing necessary for that is to tag releases with a vX.Y.Z format, ...

Interesting. I was under a false impression then. I suppose this only applies if you have a version of 2 or higher without go.mod file.

AndyMender commented 4 years ago

While I understand that adding support for go modules via the go.mod file increases maintenance overhead, it is also needed by downstream projects for efficient out-of-the-box dependency management.

davecheney commented 4 years ago

As I understand it a go.mod file is not required for this project.

puellanivis commented 4 years ago

it is also needed by downstream projects for efficient out-of-the-box dependency management.

No, it is not. You can include this package just as easily with as without a go.mod file. I have been already for like 3 years.