containerbuildsystem / cachi2

GNU General Public License v3.0
5 stars 20 forks source link

Go 1.22 support (including vendored workspaces) #553

Open eskultety opened 1 month ago

eskultety commented 1 month ago

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: https://github.com/containerbuildsystem/cachi2/discussions/398

Notes:

Maintainers will complete the following section

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:

eskultety commented 3 weeks ago

Since v1:

@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 commented 3 weeks ago

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