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

[Go] Add go.mod file #7720

Merged
merged 5 commits into from
Jan 4, 2023
Merged

Conversation

le-michael
Copy link
Collaborator

@le-michael le-michael commented Dec 18, 2022

This will help the go cli and editors understand which directory is relevant to our go library.

Migration to go modules is recommended by the go team since version 1.16. See, https://github.com/golang/go/wiki/Modules

Note:
This change will also set the minimum version of go to be latest version (1.19) in order to use this library. That means clients using an older version of go will be required to upgrade their project.

We can reduce the minimum version to an older release if we see an issue with this.

@dbaileychess dbaileychess enabled auto-merge (squash) December 22, 2022 20:22
@dbaileychess dbaileychess merged commit 6420fa5 into google:master Jan 4, 2023
@ceejatec
Copy link

ceejatec commented Jan 4, 2023

This go.mod file is incorrect, isn't it? It specifies the module name as "github.com/google/flatbuffers", but the module name in all documentation is "github.com/google/flatbuffers/go". eg.

https://pkg.go.dev/github.com/google/flatbuffers/go
https://google.github.io/flatbuffers/flatbuffers_guide_use_go.html

With this new go.mod file in place, now previously-working code is breaking with errors like

github.com/google/flatbuffers/go: github.com/google/flatbuffers/go@v0.0.0-20230104045611-6420fa5c8856: parsing go.mod:
module declares its path as: github.com/google/flatbuffers
    but was required as: github.com/google/flatbuffers/go

@le-michael le-michael mentioned this pull request Jan 4, 2023
@le-michael
Copy link
Collaborator Author

This go.mod file is incorrect, isn't it? It specifies the module name as "github.com/google/flatbuffers", but the module name in all documentation is "github.com/google/flatbuffers/go". eg.

You're right. Sent out a fix at #7756. Thanks for reporting this!

@ceejatec
Copy link

ceejatec commented Jan 4, 2023

Thanks!

@WineChord
Copy link

@le-michael a tag prefixed with "go" must be provided such as "go/v0.1.0" or else automatic module finding by go mod tidy will consistently result in "ambiguous import":

xxx imports
            github.com/google/flatbuffers/go: ambiguous import: found package github.com/google/flatbuffers/go in multiple modules:
            github.com/google/flatbuffers v2.0.6+incompatible (/root/go/pkg/mod/github.com/google/flatbuffers@v2.0.6+incompatible/go)
            github.com/google/flatbuffers/go v0.0.0-20230104232246-af9ceabeef1a (/root/go/pkg/mod/github.com/google/flatbuffers/go@v0.0.0-20230104232246-af9ceabeef1a)

@le-michael
Copy link
Collaborator Author

It seems that after introducing the go.mod we're exporting two modules for flatbuffers now on pkg.go.dev.

https://pkg.go.dev/github.com/google/flatbuffers
and
https://pkg.go.dev/github.com/google/flatbuffers/go

The former has the correct version history and version on release while the latter is the new module and has an auto generated version derived from the version tag and commit id. Ideally, we'd want to introduce the go.mod file in the root directory of this repository to maintain the version history for existing clients but that will lead to the language server with being polluted with suggestions from our test files.

I think we may have to revert this change to prevent the version history of our package on go.dev from getting messier and reintroduce the go.mod file in a future PR once we figure to correct location to install it.

frc971-automation pushed a commit to frc971/971-Robot-Code that referenced this pull request Jan 8, 2023
I don't understand exactly what's going on with the Go package
mirroring. For some reason gazelle doesn't want to mirror the
flatbuffers package. Possibly because we already have it defined in
our workspace. Not sure. There's some magic somewhere that I don't
have insight into. Regardless, we should explicitly point gazelle at
our vendored version of flatbuffers. That's what this patch
accomplishes.

I think this was caused by
google/flatbuffers#7756 or
google/flatbuffers#7720, but I'm not sure. At
least the timing matches.

This patch effectively reverts
Ibd850b77987625d359a38bf53d7db4ae9bc77dc2.

Signed-off-by: Philipp Schrader <philipp.schrader@gmail.com>
Change-Id: I489d2150811bd00ab810a3038bbac2d0dc74ff6d
sunwen18 pushed a commit to sunwen18/flatbuffers that referenced this pull request Jan 9, 2023
Co-authored-by: Derek Bailey <derekbailey@google.com>
@vitaly-eureka-security
Copy link

In the last several days we are consistently getting

github.com/google/flatbuffers/go: ambiguous import: found package github.com/google/flatbuffers/go in multiple modules:
	github.com/google/flatbuffers v22.11.23+incompatible (/home/runner/go/pkg/mod/github.com/google/flatbuffers@v22.11.23+incompatible/go)
	github.com/google/flatbuffers/go v0.0.0-20230110200425-62e4d2e5b215 (/home/runner/go/pkg/mod/github.com/google/flatbuffers/go@v0.0.0-20230110200425-62e4d2e5b215)

when trying to update dependencies with go get.

Seems to be related to the double export in pkg.go.dev.

@le-michael
Copy link
Collaborator Author

@vitaly-eureka-security can you try removing the github.com/google/flatbuffers dependency from your go.mod file then run go mod tidy to install the new github.com/google/flatbuffers/go dependency?

You should be able to update the dependency using go get -u github.com/google/flatbuffers/go after doing that.

We're planning to leave the go.mod file as is and we will start tagging the go subdirectory so that the module version matches the flatbuffer release version.

@vitaly-eureka-security
Copy link

@vitaly-eureka-security can you try removing the github.com/google/flatbuffers dependency from your go.mod file then run go mod tidy to install the new github.com/google/flatbuffers/go dependency?

You should be able to update the dependency using go get -u github.com/google/flatbuffers/go after doing that.

We're planning to leave the go.mod file as is and we will start tagging the go subdirectory so that the module version matches the flatbuffer release version.

@le-michael - thanks for the reply. I followed the instructions and removed the dependency.
Running go mod tidy resulted in bringing the dependency back (it is indirect dependency we receive from apache arrow):

$ go mod tidy
go: downloading github.com/google/flatbuffers v2.0.8+incompatible

Running go get -u ./... afterwards resulted in:

$ go get -u ./...
github.com/<module> imports
        github.com/snowflakedb/gosnowflake imports
        github.com/apache/arrow/go/arrow/ipc imports
        github.com/google/flatbuffers/go: ambiguous import: found package github.com/google/flatbuffers/go in multiple modules:
        github.com/google/flatbuffers v2.0.8+incompatible (<path>\go\pkg\mod\github.com\google\flatbuffers@v2.0.8+incompatible\go)
        github.com/google/flatbuffers/go v0.0.0-20230110200425-62e4d2e5b215 (<path>\go\pkg\mod\github.com\google\flatbuffers\go@v0.0.0-20230110200425-62e4d2e5b215)

@le-michael
Copy link
Collaborator Author

@vitaly-eureka-security I opened up an issue #7780 we can continue the discussion there.

@le-michael
Copy link
Collaborator Author

@vitaly-eureka-security should be fixed now. Please let me know if it's still broken.

@vitaly-eureka-security
Copy link

@vitaly-eureka-security should be fixed now. Please let me know if it's still broken.

Thanks @le-michael - works like a charm!

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.

5 participants