-
Notifications
You must be signed in to change notification settings - Fork 115
Add Ubuntu 24.04 Arm32 Helix Dockerfile #1120
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some differences between this file and https://github.com/dotnet/dotnet-buildtools-prereqs-docker/blob/main/src/ubuntu/24.04/helix/Dockerfile. Are these differences intentional? I'm curious why the multi-arch Dockerfile that already exists doesn't work for Arm32?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main issue is that the package won't install as-is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also libcurl
isn't available in the arm
feed because of Y2038. libcurl4t64
is there in its place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks like a trivial difference that could be handled dynamically within a single Dockerfile. You could check the value of TARGETARCH
to determine whether to install libcurl4
or libcurl4t64
, set the name to a variable and then include that variable name in the list of packages.
I'd really like to work hard to avoid duplication of content between Dockerfiles in this repo and set a better precedent than in the past.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a good idea however that doesn't change the part about libmsquic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't that be conditionally executed as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can. It is just making the Dockerfile a bit complicated. However, I can give that a try.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. That wasn't quite as bad as I expected. I was able to single source the Dockerfile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for accommodating my requests. Just a few minor things to clean up.
Co-authored-by: Matt Thalman <[email protected]>
Thanks for your help on this @mthalman. I'm quite happy with the result and that we were able to set a good standard for single sourcing Dockerfiles for multiple architectures. Thanks for your leadership (and patience)! |
Successfully tested/built on Apple M1 with:
docker build --pull -t helix-arm32 --platform linux/arm .
We don't have a 20.04 Arm32 helix image. I think we can skip that and just focus on tip (for this architecture). Although there is this PR #1043.
The following may well be hiding an incompatibility. I propose we merge as-is and then validate with runtime tests (and then possibly disable appropriate tests).
Done as a result of:
@mthalman @wfurt