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

proposal: Consider adding "ban" to go.mod #32780

Closed
smoyer64 opened this issue Jun 25, 2019 · 11 comments
Closed

proposal: Consider adding "ban" to go.mod #32780

smoyer64 opened this issue Jun 25, 2019 · 11 comments

Comments

@smoyer64
Copy link

smoyer64 commented Jun 25, 2019

Our organization has fully embraced the Go module dependency management
introduced in Go 1.11 but we have a recurring frustration that the tool could easily handle. This problem isn't limited to a a single package but I'll outline the most common occurrence below. I should also point out that the word ban could be replaced by veto, reject or any other word that is a better antonym for require.

We pretty consistently use the following import statement in our code:

import log "github.com/sirupsen/logrus"

If you use log.Error(err) before adding the import, many IDEs will be helpful and import the standard library's log package for you. There are three or four packages that we consistently have to expel from our code. What I'm proposing is that mod.go provide a section that allows us to specify packages that we don't want imported.

module github.com/example/example

go 1.12

require (
	github.com/sirupsen/logrus v1.4.2
        ...
)

ban (
        log
        github.com/google/martian/log
)

This behavior further strengthens Go's dependency management by making sure libraries that aren't intended to be used in a project are rejected. A pleasant side effect is that IDEs will continue to highlight the error as a missing package so you can properly add the required import statement.

@gopherbot gopherbot added this to the Proposal milestone Jun 25, 2019
@mark-rushakoff
Copy link
Contributor

mark-rushakoff commented Jun 25, 2019

Edit (per #32780 (comment)): I should have used go mod edit -exclude instead of the /dev/null replacement.


On one project I worked on, we deprecated a repository in favor of another. While refactoring, it was easy to accidentally reintroduce a dependency on the deprecated project, so I added a replace github.com/myorg/deprecated-project => /dev/null to go.mod.

But that is a total hack, and it gives you a not-very-informative message when you mistakenly import that package:

go: finding github.com/myorg/deprecated-project latest
foo.go:43:2: unknown import path "github.com/myorg/deprecated-project": cannot find module providing package github.com/myorg/deprecated-project

First-class support for blacklisting specific packages/modules is something I would have used in the past, had it been available.

@thepudds
Copy link
Contributor

thepudds commented Jun 25, 2019

@smoyer64 what are you using for an editor or IDE, and are you using goimports or gopls, or maybe something else?

Are you looking for functionality similar to .goimportsignore?

From https://godoc.org/golang.org/x/tools/cmd/goimports:

To exclude directories in your $GOPATH from being scanned for Go files, goimports respects a configuration file at $GOPATH/src/.goimportsignore which may contain blank lines, comment lines (beginning with '#'), or lines naming a directory relative to the configuration file to ignore when scanning. No globbing or regex patterns are allowed. Use the "-v" verbose flag to verify it's working and see what goimports is doing.

CC @stamblerre @heschik

@smoyer64
Copy link
Author

@thepudds We're using a variety of editors but the most prevalent is VSCode with gopls under the hood followed closely by vim with vim-go. In both cases we could use .goimportsignore but that will "ban" a package globally. I'd like to be able to use go.mod to manage all my dependency versions (or lack thereof) on a project by project basis. We have used the replace trick above but we'd like to globally use a Git precommit hook that globally rejects go.mod files that contain replace constructs.

@heschi
Copy link
Contributor

heschi commented Jun 26, 2019

Regarding the specific proposal: go.mod deals with modules, not packages, so it would be pretty strange IMO to put ban log in there.

Regarding goimports: it already prefers imports that are present in the same package. So if you have a package with a file that has import log "github.com/sirupsen/logrus", then make a new file and use log, it should copy over that import. Theoretically it could consider the whole module for that sort of thing.

Ultimately this sounds like an editor feature to me. As a point of reference, Eclipse has the concepts of "favorite imports" and "type filters", which in combination sound very close to what you're asking for here.

@smoyer64
Copy link
Author

It does seem a bit strange to ban the log package in go.mod as it's "well known". But go.mod definitely includes URLs for code that has not yet been converted into modules, so the case can be made that it handles packages now.

Following the thoughts related to .goimportsignore above, maybe the better answer would be to make goimports module aware. If you're in a module, why couldn't it use that file local to the module instead of the one in your $GOPATH?

@heschi
Copy link
Contributor

heschi commented Jun 26, 2019

go.mod has the path of repositories that have been implicitly converted to a module, like github.com/sirupsen/logrus. It never has package names like github.com/sirupsen/logrus/hooks/syslog.

I don't understand the focus on not adding the thing you don't want. Wouldn't it be better for goimports or the editor to add the thing you do want, i.e. import log "github.com/sirupsen/logrus"?

@smoyer64
Copy link
Author

Well ... if everything always worked in the way you imagine, you wouldn't need to to focus on the negatives (which would be more pleasant as is general in life). I've been using Maven for Java dependency management for many years and there are still occasions where you need to force dependency versions (as you can in Go modules) or exclude dependencies as I'm suggesting here. If a very mature and IMHO stream-lined dependency management system still has this problem, is it realistic for Go modules to solve the problem in its first year?

Pardon my incorrect terminology above ... I guess what I was trying to say is that if a repository can be implicitly considered a module for require purposes, it feels like you should be able to make the same implicit conversion if you want to ban it.

A couple more good ideas for alternate words for ban are exclude and blacklist.

@bcmills
Copy link
Contributor

bcmills commented Jul 3, 2019

@mark-rushakoff, you can already use an exclude directive or GOPROXY to forbid entire modules. This proposal seems to focus on individual packages instead.

@bcmills
Copy link
Contributor

bcmills commented Jul 3, 2019

@smoyer64, as @thepudds and @heschik note, this feature is not a good fit for the go.mod file: the go.mod file operates on modules, not packages.

.goimportsignore seems like the right tool for this. Consider filing a proposal to make goimports look for a .goimportsignore file at the module root instead of just GOPATH/src.

@bcmills bcmills closed this as completed Jul 3, 2019
@smoyer64
Copy link
Author

smoyer64 commented Jul 3, 2019

@bcmills From the discussion above, I agree that's the correct solution to my problem. I'll submit the bug over there but wanted to thank you all for indulging me and for the productive discussion.

@gocs
Copy link

gocs commented Sep 28, 2019

I prefer refuse and
I would like to add the case for local packages.
Example: autogenerated packages, buildignored packages:

module github.com/example/example

go 1.12

require (
	github.com/sirupsen/logrus v1.4.2
        ...
)

refuse (
        github.com/example/local/autogenerated
        github.com/example/local/buildignored
)

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

No branches or pull requests

7 participants