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

chore: version bump up to go 1.21 #2557

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

pranav-new-relic
Copy link
Member

@pranav-new-relic pranav-new-relic commented Jan 29, 2024

This is an experiment. DO NOT MERGE.

@codecov-commenter
Copy link

codecov-commenter commented Jan 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (832bbf0) 33.21% compared to head (f44600c) 31.19%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2557      +/-   ##
==========================================
- Coverage   33.21%   31.19%   -2.02%     
==========================================
  Files          93       93              
  Lines       25874    25874              
==========================================
- Hits         8594     8072     -522     
- Misses      17124    17699     +575     
+ Partials      156      103      -53     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines +14 to +16
DEPENDENCIES_FILE ?= dependencies.txt

GOTOOLS ?= $(shell cd $(TOOL_DIR) && go list -f '{{ .Imports }}' -tags tools |tr -d '[]')
GOTOOLS ?= $(shell cd $(TOOL_DIR) && cat $(DEPENDENCIES_FILE))
Copy link
Member Author

@pranav-new-relic pranav-new-relic Jan 29, 2024

Choose a reason for hiding this comment

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

Comment 1

Context

As described earlier, in order to upgrade nfpm and goreleaser to fix a current dependabot alert (#26), go needed to be upgraded to 1.21 as trying to upgrade these modules on 1.19 threw the error

imported by a module that requires go1.21

and hence, the effort to upgrade go to 1.21.

Upon trying to upgrade to go1.21 with the existing structure of the provider, the following error appeared to be thrown

package github.com/newrelic/terraform-provider-newrelic/v2/tools: build constraints exclude all Go files in /home/runner/work/terraform-provider-newrelic/terraform-provider-newrelic/tools

which indicated a probable issue with build constraints in tools.go in the tools directory. Upon removing these build constraints for testing locally, a new error was locally encountered

import XXX a program, not an importable package
Workaround

This is how we've been able to overcome the build constraints exclude all Go files in /home/runner/work/terraform-provider-newrelic/terraform-provider-newrelic/tools error - placing the list of dependencies in a different source and running a go list on the text file (changing the command used in tools.mk). With this, the compilation failure seems to have disappeared.

github.com/goreleaser/goreleaser v1.7.0
github.com/goreleaser/goreleaser v1.23.0
Copy link
Member Author

@pranav-new-relic pranav-new-relic Jan 29, 2024

Choose a reason for hiding this comment

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

Comment 2

It may be observed that goreleaser has been bumped up without any compilation issues, which means this can now work with go1.21.

Comment on lines -164 to +215
github.com/goreleaser/nfpm/v2 v2.15.0 // indirect
github.com/gorilla/websocket v1.4.2 // indirect
github.com/goreleaser/nfpm/v2 v2.35.1 // indirect
github.com/gorilla/websocket v1.5.1 // indirect
Copy link
Member Author

@pranav-new-relic pranav-new-relic Jan 29, 2024

Choose a reason for hiding this comment

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

Comment 3

nfpm, the cause of the Dependabot Alert linked above (#26) has been bumped up too, without an error, as this is now using go1.21.

gopkg.in/alecthomas/kingpin.v2 v2.2.6 // indirect
gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc // indirect
gopkg.in/go-jose/go-jose.v2 v2.6.1 // indirect
gopkg.in/ini.v1 v1.67.0 // indirect
gopkg.in/mail.v2 v2.3.1 // indirect
gopkg.in/src-d/go-billy.v4 v4.2.1 // indirect
Copy link
Member Author

@pranav-new-relic pranav-new-relic Jan 29, 2024

Choose a reason for hiding this comment

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

Comment 4

Line being referred to, if not found in this PR's preview: https://github.com/newrelic/terraform-provider-newrelic/pull/2557/files/bf09d56b548578de5d13e13cf24becc9d20f9861#diff-11f03b0f97705b85807a720c4608013062002af3604f046a09a2571b0ae02554R401

Two new dependabot alerts, #41 and #40 have been created because of the usage of go-git.v4.

The alerts suggest replacing this with go-git/v5, which is already present in the current go.mod, but this does not cease to exist as it is an indirect dependency introduced by github.com/llorllale/go-gitlint v0.0.0-20210608233938-d6303cc52cc5, a requirement at the top of tools/go.mod.

Unfortunately, this looks like we might need to make it an accepted risk because the latest release (three years ago) as shown in the repo also uses go-git.v4, hence, this indirect dependency will always continue to exist in these modules.

https://github.com/llorllale/go-gitlint/blob/master/go.mod#L12

If this is the case, we might need to shift to a linting module that does not have a direct dependency on go-git.v4 anymore.

Edit: I currently seem to find no alternative, but continue to explore. For now, I've raised a WIP PR in go-gitlint to have go-git.v4 discarded and replaced by go-git/v5.

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.

2 participants