Modify the golang.mk to support go modules

[soorena776]

Signed-off-by: soorena776 [email protected]

[soorena776]

PTAL @jbw976 @negz

[negz]

@soorena776 I like the direction, but I have to admit I'm fairly ignorant around how Go modules work (and also pretty bad at make). Can you provide some context around why this PR duplicates the existing library rather than updating or replacing it? I'm also curious what steps if any would be required to migrate our projects from dep to modules; my understanding is modules pick the oldest possible dependency while dep picks the newest possible?

[negz]

Prior art: #63

[soorena776]

@negz Sorry about not providing an intro earlier! Please read below:

golang_with_modules.mk is a copy of the existing golang.mk, in which

• go.vendor.check is renamed to go.vendor.verify, since dep check alternative is go mod verify
• go.vendor and go.vendor.verify have been updated to use go mod instead of dep
• go.vendor.lite has been deleted since its essentially the same as go.vendor when using go mod
• @(DEP) target is deleted, since it's no longer needed

Since other projects are still using golang.mk, I created a new library and referenced it in the consuming project

[bassam]

I don't think its ideal to have two golang.mk files. Any way to combine the two?

[soorena776]

@bassam I can definitely combine this with the current one. The only concern there is if other repositories still want to stick with dep, and need changes in this repo

[soorena776]

@bassam modified the existing golang.mk instead

[hasheddan]

There was some work done on this in #63 that addressed modules and maintained support for dep as well. Do you believe it is insufficient for this issue? If you want to sync up on anything I did there feel free to let me know! Thanks for driving this :)

[soorena776]

@hasheddan Thanks for the pointer to #63. I wasn't aware of this effort (my bad in not searching existing related work) when I started this. The main difference seems to be that you tried to maintain the support for dep, whereas I am not fully convinced if it's necessary.
In addition, there are a few gotchas when using go modules that I addressed:

• the ~/go/pkg/mod folder is created by go modules and is read only, which causes issues with the current clean up targets. this was tackled by go.writeable.mod.folder target
• supporting manifest and code generation after removing vendor folder
• undoing the change in go.mod when running go get command, which happens when installing tools

Please let me know what you think

[bassam]

I'm not sure we can switch all projects using this project all together. It might be prudent to leave dep as the default, and and support for go modules as an option GO_MODULES=1. the Crossplane projects can start with that turned on, and we can migrate other projects over time.

[soorena776]

@bassam I agree, and that is why I initially created a separate file.
I thought about adding a switch to enable go mod. The problem is that this will make the already hard-to-read golang.mk even more convoluted with conditions and an additional switch for dep. Given that once we migrate from dep we will never switch back, and there is no reason to alternate between the two in a single project over time, do you still think adding a switch makes sense? The alternative is to create a separate golang_with_modules.mk (see the first commit in this PR) and once all projects are switched just simply delete the older file.

[bassam]

I took a quick look at the diff and I'm not sure its that bad to support go modules and dep. I think you can contain the changes in different flavors of targets and using variables for command line switches.

I worry about the two file approach. I think it will be a a while before everyone switches to go modules, and that in the meantime we have two files to maintain.

[bassam]

can you not use the same @GO111MODULE=off approach as in golang.mk?

[soorena776]

No, as the comment above mentions versioned go packages do need GO111MODULE=on.

[soorena776]

I thought about downloading the binary directly, but there is no binary distribution for this tool: https://github.com/kubernetes-sigs/controller-tools/releases/tag/v0.2.1

so we do need to build the go src and install it, by using go get

[bassam]

Would be good to test this for Multiplatform builds (try make build.all)

[soorena776]

@bassam tested on jenkins for:

using dep:
https://jenkinsci.upbound.io/blue/organizations/jenkins/crossplane%2Fstack-aws/detail/PR-31/2/pipeline/30

using go modules:
https://jenkinsci.upbound.io/blue/organizations/jenkins/crossplane%2Fstack-aws/detail/PR-20/85/pipeline/

[jbw976]

awesome, thank you for working through everything to make this a complete and robust solution @soorena776!