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

Use Go Modules, Upgrade Module to Go 1.20 #74

Open
wants to merge 1 commit into
base: mainline
Choose a base branch
from

Conversation

adrianosela
Copy link

@adrianosela adrianosela commented May 29, 2023

Note: I don't expect this large PR to be merged, would be great if an AWS internal would go over the steps outlined in this PR to modernize and modularize the repo.

Issue #, if available:

Description of changes:

  • Initializes go module with go 1.20
  • Upgrade Dockerfile to build with Go 1.20
  • Manually set library github.com/twinj/uuid to the last compatible version (v0.0.0-20151029044442-89173bcdda19)
  • Fix makefile to use go build instead of go build -i (flag does not exist in go 1.20)
  • Run go fmt -s -w to go format the whole repo
  • Delete vendor dir (rm -rf vendor) and Run go mod vendor to vendor with go modules

Steps:

On a system with the latest version of go (1.20 at the time of opening this PR)

  • Remove the existing vendor directory with rm -rf /vendor
  • (optional) Run go fmt -s -w . to go format the code base
  • Initialize a new go module for this repo with go mod init
  • Initialize go.sum file with go mod tidy
  • Re-vendor deps with go mod vendor

Note that if you try to build now (with make build) go vet will complain about inconsistent vendoring

  • Run go mod vendor again and try to build again with make build (this will fail, continue reading...)

Note that three steps will vendor a dependency with an incompatible latest-version (github.com/twinj/uuid v1.0.0), so building will fail with

# github.com/aws/session-manager-plugin/src/message
src/message/messageparser.go:170:10: cannot use nil as uuid.UUID value in return statement
src/message/messageparser.go:176:10: cannot use nil as uuid.UUID value in return statement
src/message/messageparser.go:182:10: cannot use nil as uuid.UUID value in return statement
src/message/messageparser.go:188:10: cannot use nil as uuid.UUID value in return statement
src/message/messageparser.go:194:10: cannot use nil as uuid.UUID value in return statement
src/message/messageparser.go:417:14: invalid operation: input == nil (mismatched types uuid.UUID and untyped nil)
src/message/messageparser.go:497:25: undefined: uuid.CleanHyphen
# github.com/aws/session-manager-plugin/src/message
vet: src/message/messageparser.go:170:10: cannot use nil as uuid.UUID value in return statement
make: *** [checkstyle] Error 1
  • Open go.mod and change the version of github.com/twinj/uuid from v1.0.0 to v0.0.0-20151029044442-89173bcdda19
  • Run go mod tidy and go mod vendor again. Try building again (with make build)... You will now get failures related to running go build -i (with an -i flag, which is not supported in go 1.20)
  • Open the makefile and remove the -i from alias in the second line e.g.
-GO_BUILD := go build -i
+GO_BUILD := go build
  • (optional) also remove some directories from the cleanup makefile target, e.g.
 clean:: remove-prepacked-folder
-       rm -rf build/* bin/ pkg/ vendor/bin/ vendor/pkg/ .cover/
+       rm -rf build/* bin/ .cover/
        find . -type f -name '*.log' -delete

✅ Building (with make build) should now succeed for all the OSes

Hopefully this gets done one day!

Testing:

None done. I imagine the SSM team in charge of the plugin has an end-to-end user acceptance test (UAT) and some magic to vet all still works well with the latest-and-greatest version of all dependencies.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

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.

1 participant