Skip to content

Add go.mod #947

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

Closed
methane opened this issue Apr 4, 2019 · 12 comments · Fixed by #1003
Closed

Add go.mod #947

methane opened this issue Apr 4, 2019 · 12 comments · Fixed by #1003
Milestone

Comments

@methane
Copy link
Member

methane commented Apr 4, 2019

Issue description

I have not used Go mod yet.

How should we do in v1.5 for better go mod support?

@methane methane added this to the v1.5.0 milestone Apr 4, 2019
@julienschmidt
Copy link
Member

Nothing. go modules are for dependency management. For now, we don't have any.
All we need is proper semantic versioning tags.

@taoso
Copy link

taoso commented May 31, 2019

Nothing. go modules are for dependency management. For now, we don't have any.

@julienschmidt the google.golang.org/appengine/cloudsql is required.

"google.golang.org/appengine/cloudsql"

@julienschmidt
Copy link
Member

The file has a build flag: +appengine.
I assume it only builds when this package is available anyway.

We're certainly not going to start forcing everyone to fetch a copy of couldsql.

@taoso
Copy link

taoso commented Jul 12, 2019

@julienschmidt

The file has a build flag: +appengine.
However, if you use go-sql-driver in your go mod enabled project, your go.mod file will get the following line

github.com/go-sql-driver/mysql v1.4.1
...
google.golang.org/appengine v1.5.0 // indirect

It is weird. The google.golang.org/appengine is not always needed.

@taoso
Copy link

taoso commented Jul 12, 2019

@julienschmidt is it really needed to shift the the cloudsql registration?

mysql/appengine.go

Lines 21 to 24 in 8f4b98d

RegisterDialContext("cloudsql", func(_ context.Context, instance string) (net.Conn, error) {
// XXX: the cloudsql driver still does not export a Context-aware dialer.
return cloudsql.Dial(instance)
})

@methane
Copy link
Member Author

methane commented Jul 17, 2019

"Starting October 1, 2019, new deployments using this version will not be available." -- https://cloud.google.com/appengine/docs/standard/go/

We can remove appengine.go, at least from 2019-10-01.

@alexedwards
Copy link

@julienschmidt

We're certainly not going to start forcing everyone to fetch a copy of couldsql.

But it is fetched whenever someone runs go mod tidy, and listed in the go.mod file even if it is never used. A minimal example:

// File: main.go
package main

import "database/sql"
import _ "github.com/go-sql-driver/mysql"

func main() {
	_, _ = sql.Open("mysql", "user:password@/dbname")
}
$ go clean -modcache
$ go mod init example.com
$ go mod tidy
go: downloading github.com/go-sql-driver/mysql v1.4.1
go: extracting github.com/go-sql-driver/mysql v1.4.1
go: downloading google.golang.org/appengine v1.6.2
go: extracting google.golang.org/appengine v1.6.2

$ cat go.mod
module example.com

go 1.13

require (
	github.com/go-sql-driver/mysql v1.4.1
	google.golang.org/appengine v1.6.2 // indirect
)

$ ls $GOPATH/pkg/mod/google.golang.org/[email protected]/
aetest             datastore    LICENSE            search
appengine.go       delay        log                socket
...

@Delphier
Copy link

@julienschmidt Should support go.mod

@methane
Copy link
Member Author

methane commented Sep 17, 2019

I will remove appengine.go after 2019-10-01. Please wait it.

@julienschmidt
Copy link
Member

My plan would be to move the dialer into a separate package (github.com/go-sql-driver/cloudsql-dialer) which can be imported and registers the dialer when required.

@julienschmidt
Copy link
Member

Also see https://github.com/go-sql-driver/mysql/blob/go-mod/go.mod
Unfortunately, that still pulls google.golang.org/appengine, even though it isn't listed as a requirement in the go.mod file.

@methane
Copy link
Member Author

methane commented Sep 17, 2019

My plan would be to move the dialer into a separate package (github.com/go-sql-driver/cloudsql-dialer) which can be imported and registers the dialer when required.

I don't think it is worth enough:

  • It affects only for GAE/Go 1.9 users, and it is breaking change for them.
  • No one can deploy their app to GAE/Go 1.9 after 2019-10-01.

@julienschmidt julienschmidt reopened this Sep 17, 2019
@julienschmidt julienschmidt changed the title go mod support? Add go.mod Sep 17, 2019
This was referenced Sep 17, 2019
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 a pull request may close this issue.

5 participants