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

Add GCC@4.9.3 #27

Closed
wants to merge 2 commits into from
Closed

Add GCC@4.9.3 #27

wants to merge 2 commits into from

Conversation

davidbeckingsale
Copy link
Collaborator

No description provided.

@vsoch
Copy link
Contributor

vsoch commented Sep 16, 2021

@davidbeckingsale this is the GitHub actions "does not trigger" bug - try changing:

 if: ${{ needs.generate.outputs.empty_matrix == false }}

to

 if: ${{ needs.generate.outputs.empty_matrix == 'false' }}

@davidbeckingsale
Copy link
Collaborator Author

@vsoch - 4.9.3 (and only 4.9.3) needs to build against Ubuntu 18, not 20. I wasn't sure how to write that out in the config.

@vsoch
Copy link
Contributor

vsoch commented Sep 27, 2021

Hmm so for this matrix I don't think that would work - I think we would need changes to uptodate to allow for specifying both kinds of matrix in one build. Or some way to constrain? What do you think? We could also just do another uptodate.yaml for just that version?

@davidbeckingsale
Copy link
Collaborator Author

I think my preference would be a way to explicitly include/exclude a particular entry from the generated matrix - I think this is a feature that would be broadly useful. Does that sound okay?

@vsoch
Copy link
Contributor

vsoch commented Sep 27, 2021

yeah! What would you want that to look like in the yaml? It would be an "exclude this combination" sort of deal, so would it be separate from the build args? maybe like:

dockerbuild:
  excludes:
    - gcc_version: x
      ubuntu_version: y
    
  build_args:
    gcc_version:
      key: gcc
      versions:
       # Needs patch from spack https://github.com/spack/spack/pull/25945
       - "4.9.3"
       - "7.3.0"
       - "8.1.0"
       - "9.4.0"
       - "10.3.0"
       - "11.2.0"

    # Look for ubuntu versions for our base builds
    ubuntu_version:
      key: ubuntu
      name: ghcr.io/rse-radiuss/ubuntu
      type: container
      startat: "20.04"
      filter: 
        - "^[0-9]+[.]04$"

or something like that?

@davidbeckingsale
Copy link
Collaborator Author

davidbeckingsale commented Sep 28, 2021

Yup, so that would exclude (x,y)? I think that's fine, but would it be reasonable to exclude multiple combos?
I think the same for includes would be cool too - so we wouldn't add 4.9.3 to the gcc_versions, but instead would have:

includes:
  - gcc_version: 4.9.3
  - ubuntu_version: 18.04

@vsoch
Copy link
Contributor

vsoch commented Sep 28, 2021

oh yeah the includes might be easier- wouldn't that be akin to providing a matrix alongside the build args?

I think I'll need to think more about this - the "includes" design is kind of redundant to matrix, but if we have a matrix and build args right now they won't both build. So maybe we would want something like include.

@davidbeckingsale
Copy link
Collaborator Author

Yup, I think it would - as long as it got "merged" intelligently I think it would be fine

@vsoch
Copy link
Contributor

vsoch commented Sep 28, 2021

So there is a subtle distinction that I don't want to lose - right now having a pre-determined matrix and letting uptodate generate it are mutually exclusive - you can only have one or the other. This is done because if the user provides "matrix" - we can still have build args, but we filter them down to only include those in the matrix. and we don't run the function to generate the permutations-based one. And if there is no matrix, then there is not filter and we run that function. So I don't see an easy way to maintain that AND allow "merge" of matrices. The most straight forward thing to do is add a (sort of redundant) includes section for extra build args, and it would only be parsed in context of not having a matrix. Is there something we could call that to make it clear? If you are interested in the function it's here (and you can see how it's one or the other) https://github.com/vsoch/uptodate/blob/badb065ccad7d797c153c0fb9e33d79951db655e/parsers/docker/build.go#L276

@vsoch
Copy link
Contributor

vsoch commented Sep 29, 2021

Superseded by #36

@vsoch vsoch closed this Sep 29, 2021
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.

None yet

2 participants