Go 1.22 support (including vendored workspaces) #553

eskultety · 2024-06-10T13:24:34Z

Go 1.22 introduced another toolkit feature that affects cachi2 directly - vendored workspaces meaning that projects can now combine the vendoring feature (local module replacements) and workspaces (controlling local modules centrally wrt/ dependencies and language version requirements).

Go enabled this via a new subcommand go work vendor which needs to be run instead of the existing go mod vendor. This looks like the only difference and so we should be able to address it very simply by running that modified vendoring command when we detect the project uses workspaces.

Given the low complexity of the code no design doc accompanies this PoC as the changes should be straightforward, provided we finish the workspaces support in #457.

Note this PoC is based on work proposed in #457 so as such we need to wait for that PR before we consider this to be merged.

References: #398

Notes:

I successfully built a modified (re-enabled workspaces which upstream disables by the means of GOWORK=off) downstream version of hypershift
I successfully prefetched upstream kubernetes and grafana repos

Maintainers will complete the following section

Commit messages are descriptive enough
Code coverage from testing does not decrease and new code is covered
Docs updated (if applicable)
Docs links in the code are still valid (if docs were updated)

Note: if the contribution is external (not from an organization member), the CI
pipeline will not run automatically. After verifying that the CI is safe to run:

approve GitHub Actions workflows by clicking a button
approve the Red Hat Trusted App Pipeline container build by commenting /ok-to-test
(as is the standard for Pipelines as Code)

cachi2/core/package_managers/gomod.py

When commit e0732c7 enabled full Go 1.21 support the semantics it used for the Go toolchain selection conditions was very unfortunate as it revolved too much around the 'go_max_version' variable which at the time was set to be 1.21. The problem there is that once we try bumping the max version to say 1.22 the conditions stop making sense and as a bonus the existing tests start failing. The reasons for that are the following: - we're not actually planning on updating the container image with each Go version we're going to support unless absolutely necessary; that is thanks to the GOTOOLCHAIN=auto mechanism introduced by 1.21 that will download any new toolchain as needed automatically - without changing unit tests we suddenly can't pass the trivial condition on selecting the old fallback 1.20 if the existing unit test's base Go release is 1.21.X and the new max supported version is 1.22 Tweak the selection conditions so that they finally make sense semantically: - use go_max_version only to check incoming requests for incompatible new releases - otherwise always explicitly compare against 1.21 and only fall back to 1.20 if the base release is above 1.21 which would lead to dirtying the source repo due to compatibility issues on Golang's side Fixes: e0732c7 Signed-off-by: Erik Skultety <[email protected]>

This commit introduces just preliminary support of 1.22, i.e. without vendored workspaces support. Future patches will take care of adding support for vendored workspaces. Signed-off-by: Erik Skultety <[email protected]>

Introduced by Go 1.22 and controlled via 'go mod vendor'. We already can work with plain module vendoring, so this is just a minor change for us where based on detecting whether workspaces are in use along with vendoring we just switch the command from 'go mod vendor' to 'go work vendor'. Signed-off-by: Erik Skultety <[email protected]>

Signed-off-by: Erik Skultety <[email protected]>

eskultety · 2024-06-26T09:52:12Z

Since v1:

rebased on top of Add support for go workspaces #457
added an e2e integration test (to verify that a project can actually be built with a vendored workspace)

@chmeliik @brunoapimentel I'm ambivalent on the decision to test this in an e2e test (I did make sense to me to verify building with everything vendored, i.e. we didn't need to fetch anything, also there's a lack of real projects to test the feature with), so let me know if you want me to model it as a basic integration test or e2e is fine.

Note: Due to the ^above the integration test currently points to my own fork and based on the decision and the overall acceptance of the integration test I will push the change to the test origin repo.

chmeliik · 2024-06-26T10:32:07Z

cachi2/core/package_managers/gomod.py

@@ -1525,6 +1527,7 @@ def parse_module_line(line: str) -> ParsedModule:
 def _vendor_deps(
    go: Go,
    app_dir: RootedPath,


I think we still need to handle the app_dir vs. workspace root mismatch

packages=({"path": "hi/hiii", "type": "gomod"},), flags=["--gomod-vendor"],

This means that the app_dir will be hi/hiii and so cachi2 will try to parse hi/hiii/vendor/modules.txt, which doesn't exist. We should parse the root vendor/modules.txt.

Tested with https://github.com/cachito-testing/gomod-pandemonium/tree/go-1.22-workspace-vendoring

cachi2 --log-level debug fetch-deps '{"type": "gomod", "path": "."}' --gomod-vendor cp cachi2-output/bom.json root-bom.json cachi2 --log-level debug fetch-deps '{"type": "gomod", "path": "terminaltor"}' --gomod-vendor cp cachi2-output/bom.json submodule-bom.json get_module_purls() { jq < "$1" '.components[].purl | select(test("type=module"))' -r | sort -u } comm -23 <(get_module_purls root-bom.json) <(get_module_purls submodule-bom.json)

pkg:golang/github.com/Azure/[email protected]?type=module pkg:golang/github.com/google/[email protected]?type=module pkg:golang/github.com/go-task/[email protected]?type=module pkg:golang/github.com/Microsoft/[email protected]?type=module pkg:golang/golang.org/x/[email protected]?type=module

^ currently, these are only identified if you're processing the workspace root

Even after fixing the modules.txt parsing, there's still a problem with reporting packages:

get_nonstd_purls() { jq < "$1" '.components[].purl | select(test("^pkg:golang/[^/]*\\.[^/]*/"))' -r | sort -u } comm -23 <(get_nonstd_purls root-bom.json) <(get_nonstd_purls submodule-bom-fixed.json)

pkg:golang/github.com/cachito-testing/[email protected]?type=package pkg:golang/github.com/cachito-testing/gomod-pandemonium/[email protected]?type=package pkg:golang/github.com/cachito-testing/retrodep/v2/retrodep/[email protected]?type=package pkg:golang/github.com/cachito-testing/retrodep/v2/[email protected]?type=package pkg:golang/github.com/Masterminds/[email protected]?type=package pkg:golang/github.com/op/[email protected]?type=package pkg:golang/github.com/pkg/[email protected]?type=package pkg:golang/golang.org/x/sys/[email protected]?type=package pkg:golang/golang.org/x/tools/go/[email protected]?type=package pkg:golang/gopkg.in/[email protected]?type=package

It looks like go list ./... isn't "workspace-aware", so we will need to run that in the workspace root as well. We can probably make that part of the bigger rework

chmeliik · 2024-06-26T11:45:54Z

Since v1:
* rebased on top of [Add support for go workspaces #457](https://github.com/containerbuildsystem/cachi2/pull/457)

* added an e2e integration test (to verify that a project can actually be built with a vendored workspace)
@chmeliik @brunoapimentel I'm ambivalent on the decision to test this in an e2e test (I did make sense to me to verify building with everything vendored, i.e. we didn't need to fetch anything, also there's a lack of real projects to test the feature with), so let me know if you want me to model it as a basic integration test or e2e is fine.

Note: Due to the ^above the integration test currently points to my own fork and based on the decision and the overall acceptance of the integration test I will push the change to the test origin repo.

Basic would have been enough IMO, but there's no harm in keeping it e2e when it's already e2e

cachi2/core/package_managers/gomod.py

pierDipi · 2024-08-08T09:23:16Z

@eskultety do you know when we can start using Go 1.22 in Konflux ?

I'm getting

2024-08-08 09:19:07,145 INFO go.mod reported versions: '1.22'[go], '1.22.6'[toolchain]
2024-08-08 09:19:07,145 ERROR Failed to fetch gomod dependencies
2024-08-08 09:19:07,149 ERROR PackageManagerError: Required/recommended Go toolchain version '1.22.6' is not supported yet.
Error: PackageManagerError: Required/recommended Go toolchain version '1.22.6' is not supported yet.
  Please lower your required/recommended Go version and retry the request. You may also want to open a feature request on adding support for this version.

and I'm assuming the feature request has been already requested somewhere given this PR

eskultety · 2024-08-13T05:51:51Z

@eskultety do you know when we can start using Go 1.22 in Konflux ?

I'm getting

2024-08-08 09:19:07,145 INFO go.mod reported versions: '1.22'[go], '1.22.6'[toolchain]
2024-08-08 09:19:07,145 ERROR Failed to fetch gomod dependencies
2024-08-08 09:19:07,149 ERROR PackageManagerError: Required/recommended Go toolchain version '1.22.6' is not supported yet.
Error: PackageManagerError: Required/recommended Go toolchain version '1.22.6' is not supported yet.
  Please lower your required/recommended Go version and retry the request. You may also want to open a feature request on adding support for this version.

and I'm assuming the feature request has been already requested somewhere given this PR

@pierDipi do you use vendored workspaces? If so, this work is still in progress, if not, this PR is not the right place to discuss - #591

chmeliik reviewed Jun 10, 2024

View reviewed changes

cachi2/core/package_managers/gomod.py Show resolved Hide resolved

eskultety mentioned this pull request Jun 17, 2024

Add support for go workspaces #457

Merged

4 tasks

eskultety added 4 commits June 26, 2024 08:16

tests: integration: Add e2e test for Go 1.22 workspace vendoring

63e6800

Signed-off-by: Erik Skultety <[email protected]>

eskultety force-pushed the go122_workspaces branch from b361dd9 to 63e6800 Compare June 26, 2024 09:46

eskultety changed the title ~~POC Go 1.22 support (including vendored workspaces)~~ Go 1.22 support (including vendored workspaces) Jun 26, 2024

chmeliik reviewed Jun 26, 2024

View reviewed changes

cachi2/core/package_managers/gomod.py Show resolved Hide resolved

brunoapimentel requested review from brunoapimentel June 26, 2024 20:42

This was referenced Jul 4, 2024

Improve the user experience for Go workspaces #564

Open

Add support for Go workspaces containerbuildsystem/cachito#999

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Go 1.22 support (including vendored workspaces) #553

Go 1.22 support (including vendored workspaces) #553

eskultety commented Jun 10, 2024 •

edited

Loading

eskultety commented Jun 26, 2024 •

edited

Loading

chmeliik Jun 26, 2024

chmeliik Jun 26, 2024 •

edited

Loading

chmeliik Jun 26, 2024

chmeliik commented Jun 26, 2024

pierDipi commented Aug 8, 2024 •

edited

Loading

eskultety commented Aug 13, 2024

Go 1.22 support (including vendored workspaces) #553

Are you sure you want to change the base?

Go 1.22 support (including vendored workspaces) #553

Conversation

eskultety commented Jun 10, 2024 • edited Loading

Maintainers will complete the following section

eskultety commented Jun 26, 2024 • edited Loading

chmeliik Jun 26, 2024

Choose a reason for hiding this comment

chmeliik Jun 26, 2024 • edited Loading

Choose a reason for hiding this comment

chmeliik Jun 26, 2024

Choose a reason for hiding this comment

chmeliik commented Jun 26, 2024

pierDipi commented Aug 8, 2024 • edited Loading

eskultety commented Aug 13, 2024

eskultety commented Jun 10, 2024 •

edited

Loading

eskultety commented Jun 26, 2024 •

edited

Loading

chmeliik Jun 26, 2024 •

edited

Loading

pierDipi commented Aug 8, 2024 •

edited

Loading