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

Build both focal and jammy images. #15074

Merged
merged 1 commit into from Apr 14, 2023

Conversation

luca-heltai
Copy link
Member

Build both focal and jammy versions of the master branch.

@luca-heltai
Copy link
Member Author

In reference to #15060.

With this, we would build both jammy and focal images.

@marcfehling
Copy link
Member

runs-on: ubuntu-20.4

I changed this line in #15060 to make sure we build the focal image on ubuntu-focal.

When building the jammy image, I would suggest to create an entirely new job and specifically ask for runs-on: ubuntu-22.04 (if I understood how to build docker images correctly).

@tjhei
Copy link
Member

tjhei commented Apr 13, 2023

I changed this line in #15060 to make sure we build the focal image on ubuntu-focal.

Why is that important? It shouldn't matter what the host system is.

@marcfehling
Copy link
Member

marcfehling commented Apr 13, 2023

Why is that important? It shouldn't matter what the host system is.

You are right, it doesn't matter. My conception on how to build docker images was wrong, and it's clear now after reading more about their creation. It's only important what's in the image/container, and not on the host.

@luca-heltai Could you change this line back to ubuntu-latest? Sorry about that. EDIT: No need, I can change that back in a separate PR.

Comment on lines +11 to +12
git clone https://github.com/dealii/dealii dealii-$VER && \
cd dealii-$VER && \
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
git clone https://github.com/dealii/dealii dealii-$VER && \
cd dealii-$VER && \
git clone https://github.com/dealii/dealii && \
cd dealii && \

We ship each image with only one version of deal.II, right? To keep all images in the same structure, I would suggest to keep the filenames of the source directories the same as well. This way users really need to only switch the docker image if they want to mess with source code of different versions.

I like the VER argument though! It's an easy way to prepare images on different branches/versions.

Copy link
Member Author

Choose a reason for hiding this comment

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

We only ever use the source directory when debugging. The actual library is installed in /usr/local for all images, and that's all we really need to know about. I think there is value in distinguishing one version from the other and to allow multiple versions of the source, but if you feel strongly about this, I'll make the change you suggest.

Copy link
Member

Choose a reason for hiding this comment

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

No, I don't feel strongly about it.

@marcfehling marcfehling merged commit 3a895b0 into dealii:master Apr 14, 2023
11 of 14 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.

None yet

3 participants