rse-ops / docker-images

Collection of Docker image bases provided by the RSE-ops community.
https://rse-ops.github.io/docker-images/
MIT License
5 stars 8 forks source link

Add GCC@4.9.3 #27

Closed davidbeckingsale closed 2 years ago

vsoch commented 3 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

vsoch commented 2 years ago

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 commented 2 years ago

Superseded by #36