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

fix(go): Remove deprecated '-d' flag when doing 'go get' #32506

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

echarrod
Copy link
Contributor

Changes

  • Remove deprecated -d flag from go get.

Context

  • This flag was changed to have no effect in Go 1.18
  • Go's release policy only officially support the latest two major versions, i.e. 1.23 and 1.22 at the moment

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

@echarrod
Copy link
Contributor Author

Ah I see we're using older versions of Go elsewhere, maybe we should update those at the same time 🤔

@rarkins
Copy link
Collaborator

rarkins commented Nov 13, 2024

Usually Renovate will use the latest Go version, but users may pin to an older version. Therefore we ideally would look at the go.mod directive and decide on -d based on the version in use

lib/modules/manager/dockerfile/extract.spec.ts Outdated Show resolved Hide resolved
@@ -243,7 +243,7 @@ export async function updateArtifacts({
}
}

let args = `get -d -t ${goGetDirs ?? './...'}`;
let args = `get -t ${goGetDirs ?? './...'}`;
Copy link
Member

Choose a reason for hiding this comment

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

extract go version from go and / or toolchain directive. drop -d only if it is >=1.18.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry was just about to comment: I made an update to update Go version after I realised it was currently older: #32514

@echarrod
Copy link
Contributor Author

Usually Renovate will use the latest Go version, but users may pin to an older version. Therefore we ideally would look at the go.mod directive and decide on -d based on the version in use

Thanks for the response, I wouldn't have guessed anyone is on an older version, given Go's release policy of only officially support the latest two major versions, i.e. 1.23 and 1.22 at the moment.

It also means you can barely upgrade to any libraries at all if you're on < 1.18, so I wouldn't have thought it would be feasible to do this and run Renovate!

@viceice
Copy link
Member

viceice commented Nov 14, 2024

🤔 go v1.17 was realesed ~3 years ago. https://go.dev/blog/go1.17

https://github.com/search?q=%22go+1.17%22++NOT+is%3Aarchived+path%3Ago.mod&type=code

It seems there are some repos with go v1.17.

@secustor @rarkins Can we really assume someone uses go <1.18 and also is using renovate?

I think @echarrod is right and we can assume any update needs a newer go version now.

@rarkins
Copy link
Collaborator

rarkins commented Nov 14, 2024

Go is a little bit unique though because even if a repo has a go mod directive of 1.17, this means "Use Go 1.17 compatibility" and not "Use Go 1.17". i.e. it's not like a .nvmrc file. So Renovate intentionally runs the latest 1.x Go no matter what the Go mod directive says, because Go is backwards compatible. The only way removing this flag could cause problems is if a user pinned to an older exact version using constraints.

@rarkins
Copy link
Collaborator

rarkins commented Nov 14, 2024

Can we detect if someone has pinned their go constraints, or should we assume they won't?

@viceice
Copy link
Member

viceice commented Nov 14, 2024

Can we detect if someone has pinned their go constraints, or should we assume they won't?

we can detect

@echarrod
Copy link
Contributor Author

Hey all 👋
Is this the blocker for this MR:

  • if:
    • a user has pinned their go constraint
    • and it's < 1.18
  • still use -d in go get?

@viceice
Copy link
Member

viceice commented Nov 27, 2024

yes, as told before

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.

3 participants