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

all: Become a Go module (fixes #5148) #5384

Merged
merged 6 commits into from Dec 18, 2018
Merged

all: Become a Go module (fixes #5148) #5384

merged 6 commits into from Dec 18, 2018

Conversation

calmh
Copy link
Member

@calmh calmh commented Dec 17, 2018

Purpose

Use the new hotness. Build cleanly outside GOPATH. Use modern tools for vendoring.

The changes in more detail:

  • Our dependencies are now declared in go.mod & go.sum. Managing these is done using go get, go get -u, go mod tidy, go mod vendor while in the modules mode (GO111MODULE=on or outside GOPATH).

  • The vendor directory is still populated, using go mod vendor. This means there is no difference in building stuff for those still using GOPATH etc. In the long run, Go 1.12+, we can drop the vendor dir.

  • The non-Go dependencies we have for go generate (protobuf schemas), and the XDR builder, aren't easily accessible as modules. Instead I hacked the build script to clone and check out the stuff we need, if it isn't already present, when we run go run build.go proto. This isn't done very often by most developers, so I think it's fine.

  • I removed the stuff in build.go for juggling GOPATH and building outside of it. If you're outside GOPATH today and on a modern compiler, the module support will kick in instead.

Testing

It still builds for me.

@calmh
Copy link
Member Author

calmh commented Dec 17, 2018

So there's a hilarious amount of protocol failures due to updating the protobuf package, so that's interesting and merits investigation...

@calmh
Copy link
Member Author

calmh commented Dec 18, 2018

This is not user facing, and should really only affect those juggling dependencies or wanting to work without gopath. Given that, is anyone strongly opposed to this change, @AudriusButkevicius @imsodin ?

@AudriusButkevicius
Copy link
Member

This is probably the future, so I am ok with that. I am not planning to review the diff as I presume it's a massive noop change.

Copy link
Member

@imsodin imsodin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I knew how to deal with the oddities of gvt, now I guess I'll have to learn about go mod - I don't mind.

@calmh calmh merged commit 944ddcf into syncthing:master Dec 18, 2018
@calmh
Copy link
Member Author

calmh commented Dec 18, 2018

Welcome to the future. Now to fix all the unexpected build failures...

@calmh calmh deleted the mod branch December 18, 2018 11:37
@calmh calmh added this to the v1.0.1 milestone Jan 1, 2019
@st-review st-review added the frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion label Dec 19, 2019
@syncthing syncthing locked and limited conversation to collaborators Dec 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants