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: cmd/go: record default build tags in go.mod #43288

Closed
arp242 opened this issue Dec 20, 2020 · 17 comments
Closed

proposal: cmd/go: record default build tags in go.mod #43288

arp242 opened this issue Dec 20, 2020 · 17 comments

Comments

@arp242
Copy link

arp242 commented Dec 20, 2020

I have a project which depends on https://github.com/mattn/go-sqlite3. I'd like to use SQLite's JSON extensions, which requires settings the sqlite_json build tag:

$ go build -tags=sqlite_json

In my opinion, having to specify this for every build invocation is not very good UX, either for myself or for $random_users of my program.

There are no real good solutions for this as far as I can find:

  • A wrapper Makefile or shell script isn't too hard, but it breaks the ease-of-use of "go build will always build your project", may be hard to get working fully correct on all platforms, and adds an external dependency (I don't typically have make installed, as I don't really need it).

    It works, but "go build always correctly builds your project" seems like a worthwhile behaviour to have.

  • I can fork the go-sqlite3 repo, modify it to remove the build constraints, and add replace github.com/mattn/go-sqlite3 => example.com/mysqlite in go.mod, but keeping this updated and maintained is some amount of work.


Previously there wasn't really any place to specify these kind of things, but now that we have a go.mod it's something that could potentially be added there; for example:

module test

go 1.13

tags osusergo netgo sqlite_omit_load_extension sqlite_json

require (
	github.com/mattn/go-sqlite3 v1.14.5
)

In this example it also adds osusergo, netgo, and sqlite_omit_load_extension to avoid dynamic linking, which is probably another common use case. Right now people often use CGO_ENABLED=0 go build for this.

There are some previous proposals to make go build look at some build-time configuration (#39005, #42343) which have the problems of potential arbitrary code execution and large amount of complexity. This proposal is much more limited and only about build tags, which should avoid those problems.

The -tags flag is somewhat special as it affects the control flow of the program and that any errors from using the wrong flags may not be seen at runtime (e.g. when using an SQL query with JSON functions), and can be seen as a "dependency" of a program. I think it may be worth thinking about tracking this "dependency" in the go.mod file; I think it fits the purpose of the file without stuffing too much "cruft" in there.


Open questions:

  • What to do if someone uses go build -tags=othertag? Do we merge the tags? Override the default ones?

  • What if a dependency has a tags directive? What if two dependencies have different tags directives? It probably makes the most sense to only look at this for the main project you're building, and ignore tags directive from dependencies.

@gopherbot gopherbot added this to the Proposal milestone Dec 20, 2020
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Dec 21, 2020
@ianlancetaylor
Copy link
Contributor

I think we've fairly consistently said that we aren't going to put build configurations into go.mod. That's not what it's for.

In particular it wouldn't make sense to do so for anything other than a main package.

@arp242
Copy link
Author

arp242 commented Dec 21, 2020

Do you have any other ideas on how to solve this particular problem @ianlancetaylor, or do you not think it's a big enough problem worth solving and that "do nothing" is the best solution?

Thinking again, go.mod may actually not be the best location – at least not with the syntax in the example – as ./cmd/a and ./cmd/b may require different build flags 🤔 A //go:tags a_build_tag directive would be an alternative, but no one likes expanding those either (for good reasons).

At any rate, it's a problem I've run in to on various occasions, and IMO it's something that can probably be improved in some way – I don't really care about the exact method as such.

@go101
Copy link

go101 commented Dec 21, 2020

how about a new file go.build, which contains several build targets (build constraint and option compositions).

@ianlancetaylor
Copy link
Contributor

The go command is not a complete build system that can handle all different cases. It's a real problem, and perhaps someday Go will provide a real solution, but today it does not. So my current recommendation would be using a shell script to build your program.

@bcmills
Copy link
Contributor

bcmills commented Dec 21, 2020

This seems like a duplicate of #42343. Please see the discussion there.

@bcmills
Copy link
Contributor

bcmills commented Dec 21, 2020

A wrapper Makefile or shell script isn't too hard, but it breaks the ease-of-use of "go build will always build your project"

Any assumption of specific build tags break the ease-of-use of “go build will always build your project. For the consistency reasons you alluded to earlier, the tags for your own module cannot propagate to dependencies, so they would have to duplicate your tags as well.

@bcmills
Copy link
Contributor

bcmills commented Dec 21, 2020

In my opinion, having to specify this for every build invocation is not very good UX, either for myself or for $random_users of my program.

That's why we have GOFLAGS and GOENV (see https://tip.golang.org/cmd/go/#hdr-Environment_variables). This would be adding yet a third way to specify default tags for every go invocation.

@arp242
Copy link
Author

arp242 commented Dec 21, 2020

This seems like a duplicate of #42343. Please see the discussion there.

Yeah, I read that (and #39005, which is even more similar) but I don't either are really a duplicate as this is much more limited.

I don't think having complete build configuration is really necessary, but build tags are different as they can affect the runtime behaviour of a program, and can be viewed as a "dependency" of a program. This is different from some convenience shortcuts to build on your program on Windows or omitting debug symbols in your binary: these don't affect the control flow of your program.

There are some other flags to do this, but they're comparatively obscure are rarely needed to correctly build a program.

the tags for your own module cannot propagate to dependencies, so they would have to duplicate your tags as well.

They can't propagate up (i.e. from a dependency to main), but that's okay; it's about propagating down to dependencies which have build tags to change the behaviour (such as the net package, or go-sqlite3). This is only really useful for a main package.

@bcmills
Copy link
Contributor

bcmills commented Dec 21, 2020

I'd like to use SQLite's JSON extensions, which requires setting the sqlite_json build tag

That seems like it would be better addressed by making the JSON functionality a separate importable Go package — for example, by compiling the JSON extension as a separate library and statically linking it into the binary as described in https://www.sqlite.org/loadext.html#statically_linking_a_run_time_loadable_extension. But that's a separate issue that you should probably take up as an issue in the go-sqlite3 issue tracker (CC @mattn).

@bcmills
Copy link
Contributor

bcmills commented Dec 21, 2020

build tags are different as they can affect the runtime behaviour of a program, and can be viewed as a "dependency" of a program.

I would argue that that is an abuse of build tags. Build tags should change the implementation or optimization of a program (as in the case of osusergo and netgo), not the user-facing API. Go API dependencies should be represented as package imports, not build tags.

@bcmills
Copy link
Contributor

bcmills commented Dec 21, 2020

CC @jayconrod @matloob

@arp242
Copy link
Author

arp242 commented Dec 21, 2020

Thanks; I'll have to look at that; I solved it in the meantime by just maintaining a "fork" (it's just a single +build line that needs removing) and replace [..] => [..]-ing it. But it's something that could affect other packages too, so it's not just about this specific SQLite example; although it probably is the most common use case.

This indeed isn't the best use of build tags; I see your point, but sometimes alternatives are hard, especially when you're interfacing with third-party stuff. To be honest I don't know how common this pattern is (and not something that's easy to grep for).

@bcmills
Copy link
Contributor

bcmills commented Dec 21, 2020

This indeed isn't the best use of build tags; I see your point, but sometimes alternatives are hard, especially when you're interfacing with third-party stuff.

Oh, absolutely! But before we go too far down the build-tag route, I would like to better understand what it is that makes those alternatives hard, and whether we can address that difficulty more directly.

I'm guessing that in this case a lot of the difficulty comes from the C linking model and/or the interaction between cgo and the C linker. We can't do much about the former, but perhaps we could improve the latter in some way.

If anything, the fact that the package is provided by a third party makes it more important not to rely on build tags: we have a mechanism (MVS) to reconcile differences in package-import requirements across modules, but no such mechanism to reconcile differences in build tags.

@arp242
Copy link
Author

arp242 commented Dec 22, 2020

But before we go too far down the build-tag route, I would like to better understand what it is that makes those alternatives hard, and whether we can address that difficulty more directly.

Yes, that makes perfect sense; above all this is a "I this problem that I believe is hard to solve, and I propose we see if there's a way to solve it"-kind of proposal. I probably could have written the proposal better instead of focusing too much on a specific mechanism, but ah well.


The way it works now is like this; sqlite3.go contains the main integration:

package sqlite3

/*
#cgo CFLAGS: -std=gnu99
#cgo CFLAGS: -DSQLITE_ENABLE_RTREE
#cgo CFLAGS: -DSQLITE_THREADSAFE=1

[..snip..]

And then files such as sqlite3_opt_json1.go add some CFLAGS to that (entire file):

// +build sqlite_json sqlite_json1 json1

package sqlite3

/*
#cgo CFLAGS: -DSQLITE_ENABLE_JSON1
*/
import "C"

From a user perspective, one nice way to get this might be to put this in github.com/mattn/go-sqlite3/sqlite3-json, and then:

import (
	_ "github.com/mattn/go-sqlite3"
	_ "github.com/mattn/go-sqlite3/sqlite3-json"
)

func main() {
	db, err := sql.Open("sqlite3", ":memory:") // With JSON.
}

But the sqlite3-json subpackage has no way to pass that -DSQLITE_ENABLE_JSON1 flag to github.com/mattn/go-sqlite3 as far as I know.

Similarly, there is also no way that I can do something like this in example.com/app:

package main

/*
#cgo CFLAGS for github.com/mattn/go-sqlite3: -DSQLITE_ENABLE_JSON1
*/
import "C"

Or something like that (you can add it via CFLAGS, but then we're back to where we started).

The "Statically Linking A Run-Time Loadable Extension" SQLite docs begins with "simply add the -DSQLITE_CORE compile-time option" – but that's not really all that simple.

Any package modifying any build flag for anything else would not be the best of ideas, but I wonder if there isn't a limited way to give a little bit more flexibility with this, such as subpackages being able to add to the CFLAGS of their parent, a main package being able to define CFLAGS, or something else entirely.

I don't know what the ramifications of that would be from a compiler internals perspective though.

@bcmills
Copy link
Contributor

bcmills commented Dec 22, 2020

@arp242

The "Statically Linking A Run-Time Loadable Extension" SQLite docs begins with "simply add the -DSQLITE_CORE compile-time option" – but that's not really all that simple.

I agree that it's not really all that simple, but I still suspect that most or even all of the complexity can be encapsulated within the go-sqlite3 wrapper.

The first approach I would investigate would be something like:

  • Have github.com/mattn/go-sqlite3 link against the SQLite core, and unconditionally build it with -DSQLITE_CORE.
  • Have github.com/mattn/go-sqlite3/json link against the json extension, and provide a Go function (perhaps an init function?) to register or initialize the extension at run-time.
  • Provide similar subpackages for the other supported extensions.

Then a Go consumer can indicate which extensions to link (and initialize) by importing (or blank-importing) specific packages, rather than setting build tags, and Go libraries that each need different SQLite extensions without the maintainer of the resulting binary needing to set any build tags explicitly.

@rsc
Copy link
Contributor

rsc commented Jul 19, 2021

This does appear to be a duplicate of #42343.
I don't have a clear answer to how exactly to change go-sqlite,
but I would suggest just enabling JSON by default there,
removing the configuration hook entirely.
It is not going to increase the size significantly.
Or else what @ianlancetaylor said about building your specific app with a makefile.

@bcmills
Copy link
Contributor

bcmills commented Jun 15, 2022

Duplicate of #42343

@bcmills bcmills marked this as a duplicate of #42343 Jun 15, 2022
@bcmills bcmills closed this as not planned Won't fix, can't repro, duplicate, stale Jun 15, 2022
@seankhliao seankhliao removed this from Incoming in Proposals (old) Jul 3, 2022
@golang golang locked and limited conversation to collaborators Jun 15, 2023
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

6 participants