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

update go1.21 #18184

Merged
merged 5 commits into from
Aug 14, 2023
Merged

update go1.21 #18184

merged 5 commits into from
Aug 14, 2023

Conversation

shoenig
Copy link
Member

@shoenig shoenig commented Aug 9, 2023

This PR updates the build system to use go1.21. In addition,

  • update golangci-lint
  • eliminate the Min/Max helpers for the now builtin min/max
  • swap out depguard and use semgrep instead (was failing with go1.21, and has been troublesome in the past too)
  • fixup a TLS error string check that seemingly changed in go1.21

Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

LGTM

@shoenig shoenig merged commit d9341f0 into main Aug 14, 2023
25 checks passed
@shoenig shoenig deleted the update-go1.21 branch August 14, 2023 13:43
shoenig added a commit that referenced this pull request Aug 14, 2023
(manual cherry-pick of d9341f0)

* build: update to go1.21

* go: eliminate helpers in favor of min/max

* build: run go mod tidy

* build: swap depguard for semgrep

* command: fixup broken tls error check on go1.21
shoenig added a commit that referenced this pull request Aug 14, 2023
* update go1.21 (#18184)

(manual cherry-pick of d9341f0)

* build: update to go1.21

* go: eliminate helpers in favor of min/max

* build: run go mod tidy

* build: swap depguard for semgrep

* command: fixup broken tls error check on go1.21

* fixup job_endpoint.go

---------

Co-authored-by: Seth Hoenig <shoenig@duck.com>
pkazmierczak pushed a commit that referenced this pull request Aug 15, 2023
pkazmierczak pushed a commit that referenced this pull request Aug 15, 2023
* build: update to go1.21

* go: eliminate helpers in favor of min/max

* build: run go mod tidy

* build: swap depguard for semgrep

* command: fixup broken tls error check on go1.21
pkazmierczak added a commit that referenced this pull request Aug 15, 2023
…480cc924c8cf00f981e604e02c62140-to-1.5

admin: manual backport of #18184 to 1.5.x
pkazmierczak added a commit that referenced this pull request Aug 15, 2023
…480cc924c8cf00f981e604e02c62140-to-1.6

admin: manual backport of #18184 to 1.6
lgfa29 added a commit that referenced this pull request Sep 13, 2023
lgfa29 added a commit that referenced this pull request Sep 13, 2023
lgfa29 added a commit that referenced this pull request Sep 13, 2023
@IamTheFij
Copy link
Contributor

I just want to bring some awareness to a downside of staying cutting edge here.

I think it's great for the application part of Nomad, but it makes it difficult for other projects that are trying to use nomad/api. I have been looking to move tasks of mine over to use the Nomad Task Socket, which requires the API client included in 1.7. It included a moderate amount of updates to get Diun capable, and I just started looking at upgrading Prometheus, but it seems that will be even more significant.

I don't know if there's really a way to work around this (maybe pulling API into it's own package based on a different Go API), but I figured I'd at least shed some light on it.

Thanks!

@shoenig
Copy link
Member Author

shoenig commented Jan 2, 2024

Hi @IamTheFij the /api submodule should still only require go1.20 per its go.mod file; you shouldn't need go1.21 if that's the only module you have a dependency on. (let us know if that somehow isn't the case!)

@IamTheFij
Copy link
Contributor

@shoenig, so that's true per the go.mod file, but it's impossibe to build on go1.20 and results in the following error:

/go/pkg/mod/github.com/hashicorp/nomad/api@v0.0.0-20231213195942-64e3dca9274b/error_unexpected_response.go:11:2: package slices is not in GOROOT (/usr/local/go/src/slices)

Something that will also make it easier would be if the /api submodule had a release. I created an issue to track that as well. #19613 Perhaps that could be expanded to include some dedicated testing of that module as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.4.x backport to 1.4.x release line backport/1.5.x backport to 1.5.x release line backport/1.6.x backport to 1.6.x release line
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants