Skip to content

Upgrade to Go 1.19#6723

Merged
mislav merged 6 commits intotrunkfrom
go-1.19-upgrade
Dec 12, 2022
Merged

Upgrade to Go 1.19#6723
mislav merged 6 commits intotrunkfrom
go-1.19-upgrade

Conversation

@mislav
Copy link
Contributor

@mislav mislav commented Dec 12, 2022

The main feature we're opting into with this upgrade is the stricter os/exec behavior: Go 1.19 will no longer look up executables in the current directory by default (in addition to looking up PATH).

See https://github.com/cli/safeexec#readme for more information.

From our own code, we can still continue to explicitly use safeexec instead of os/exec; the added bonus is that we then continue to support relative directory entries in PATH.

With this upgrade our codebase adopts the stricter handling of `os/exec` command lookup in it that it doesn't allow shelling out to a command in the current directory.
@mislav mislav requested a review from a team as a code owner December 12, 2022 15:08
@mislav mislav requested review from samcoe and removed request for a team December 12, 2022 15:08
@mislav mislav requested a review from a team as a code owner December 12, 2022 15:59
@cmbrose
Copy link
Member

cmbrose commented Dec 12, 2022

@mislav - for codespaces, the diff is fine, just mocks. Do we have any concerns about behavioral changes from the new os/exec that we need some manual testing on? Otherwise, LGTM

@mislav
Copy link
Contributor Author

mislav commented Dec 12, 2022

@cmbrose No behavioral changes expected; from our codebase we don't use os/exec path lookup directly; the upgrade is primarily to cover usages of os/exec in dependencies that we do not control.

@jungaretti
Copy link
Contributor

Your devcontainer.json change looks good too

@mislav mislav merged commit ee4afb2 into trunk Dec 12, 2022
@mislav mislav deleted the go-1.19-upgrade branch December 12, 2022 17:54
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.

4 participants