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

allow override go version for uses: go/build and go/install #606

Merged
merged 2 commits into from
Aug 16, 2023

Conversation

rawlingsj
Copy link
Member

@rawlingsj rawlingsj commented Aug 16, 2023

The go 1.21 release included breaking changes that result in breaking some go based packages. Until upstream projects support go 1.21 a melange config can pin to a specific go version, i.e. go-1.20.

This causes a problem however as melange configs that take advantage of the reusable go pipeline are not able to specify a different go package version because the melange pipeline hardcodes latest

I'm not 100% this is the right answer but we need to be able to choose which go version is installed for melange configs that leverage the built in go pipeline, we could override the latest go version package like this...

uses: go/build
with:
  go-package: go-1.20

and

uses: go/install
with:
  go-package: go-1.20

An alternative is we remove the go package from the built in pipeline and update all melange configs to explicitly specify the go package using the standard environment config, i.e.

environment:
  contents:
    packages:
      - go-1.20

pipeline:
  - uses: go/build

This seems to be how the ruby pipeline works as no ruby package is defined in the needs section. However that would be a breaking change so I don't think this is viable.

Any other options?

Signed-off-by: James Rawlings <jrawlings@chainguard.dev>
@rawlingsj rawlingsj requested a review from a team as a code owner August 16, 2023 11:43
@rawlingsj rawlingsj requested review from kaniini and removed request for a team August 16, 2023 11:43
Copy link
Member

@imjasonh imjasonh left a comment

Choose a reason for hiding this comment

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

This lgtm but I'd love to have a test

@rawlingsj
Copy link
Member Author

Cool - I did ponder a test and saw there's already one for the mutateStringFromMap so figured I'd throw this up before spending more time on it to see what folks think.

Lemme add a test to check the packages do mutate.

@luhring
Copy link
Contributor

luhring commented Aug 16, 2023

Any other options?

This LGTM, but it might also be worth thinking about using the version for the version pinning, which would honor the provides information we put into the various packages. I don't think this is a crucial aspect of the design though, so don't block on this!

Signed-off-by: James Rawlings <jrawlings@chainguard.dev>
@rawlingsj rawlingsj merged commit 5bbbd51 into chainguard-dev:main Aug 16, 2023
24 checks passed
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