Skip to content

Upgrade to Go 1.19 - #6723

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

mislav merged 6 commits into
trunkfrom
go-1.19-upgrade

Conversation

@mislav

@mislav mislav commented Dec 12, 2022

Copy link
Copy Markdown
Contributor

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

cmbrose commented Dec 12, 2022

Copy link
Copy Markdown
Member

@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

mislav commented Dec 12, 2022

Copy link
Copy Markdown
Contributor Author

@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
Copy Markdown
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.

5 participants