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

Roll forward to 24.04 #27

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Roll forward to 24.04 #27

wants to merge 4 commits into from

Conversation

donporter
Copy link
Collaborator

Update dockerfile to build image against Ubuntu 24.04. Minor other changes to deal with python environment.

Dockerfile Outdated Show resolved Hide resolved
eric-unc
eric-unc previously approved these changes May 20, 2024
@donporter
Copy link
Collaborator Author

@brent-munsell suggests we wait until gradescope supports 24.04 for autograders, which is reasonable. I am in favor of this suggestion, so let's just leave this open until then (or until I have a good example for using this image in the gradescope autograder). There is no rush.

@jesse-wei
Copy link
Collaborator

jesse-wei commented Oct 13, 2024

In 211, we have a specific use case for Ubuntu 24.04.

Specifically, on 24.04, the installed version of clang-format is 18.1.3. This is just high enough to support .clang-format-ignore files, introduced in 18.1.0. This would allow us to exclude the googletest directory in our labs from being formatted (without the bandaid solution that we're using right now).

Here's how to confirm that in Ubuntu 24.04, clang-format 18.1.3 is installed:

> docker run --rm -it ubuntu:24.04
root@1bc0b20925ae:/# (yes | apt update && DEBIAN_FRONTEND=noninteractive apt install -y clang-format) >/dev/null 2>&1
root@1bc0b20925ae:/# clang-format --version
Ubuntu clang-format version 18.1.3 (1ubuntu1)

In the container, the version is 14.0.0-1ubuntu1.1.

learncli$ clang-format --version
Ubuntu clang-format version 14.0.0-1ubuntu1.1

Other than this, other packages would also be upgraded, and this seems good to me.

Potential downsides

The obvious downside is something could break, but I don't think this would happen. Just in case, I will personally test code for COMP 211 Labs 2, 3, and 4 (just running GoogleTest test cases) in the container in Windows (x86), M1 macOS (ARM), and Ubuntu 24.04 (x86) before this is merged.

The 211 autograder code would not be affected by this change. I don't know about 530 though. But, seeing as Prof. Porter made this PR, I hope he's okay with it?

@donporter
Copy link
Collaborator Author

I'd prefer to wait until a semester boundary to deploy this for my course. I opened this PR in May, with plenty of lead time for COMP 530. In general, I think we should follow this policy (no breaking changes mid-semester), unless there is a compelling reason (i.e., an entire class is dead in its tracks).

The issue I have is that the development environment is different than the autograder's OS version.

Perhaps you can tag the container differently until the end of the semester? (i.e., keep this on a branch but deploy a different container)?

@jesse-wei
Copy link
Collaborator

I'd prefer to wait until a semester boundary to deploy this for my course. I opened this PR in May, with plenty of lead time for COMP 530. In general, I think we should follow this policy (no breaking changes mid-semester), unless there is a compelling reason (i.e., an entire class is dead in its tracks).

The issue I have is that the development environment is different than the autograder's OS version.

Perhaps you can tag the container differently until the end of the semester? (i.e., keep this on a branch but deploy a different container)?

Yes, "no breaking changes mid-semester" is definitely a good policy. I agree that this should have been merged sooner (in summer). I'll think about deploying this just for COMP 211 because I doubt that for COMP 211 assignments, Ubuntu 22.04 vs. 24.04 would affect anything. @brent-munsell

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