-
Notifications
You must be signed in to change notification settings - Fork 114
Add azurelinux3.0-source-build-test image #1408
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
# Install the base toolchain we need to build anything (clang, cmake, make and the like) | ||
# this does not include libraries that we need to compile different projects, we'd like | ||
# them in a different layer. | ||
RUN apt-get update \ | ||
&& apt-get install -y \ | ||
clang-18 \ | ||
cmake \ | ||
gdb \ | ||
liblldb-18-dev \ | ||
lldb-18 \ | ||
llvm-18 \ | ||
locales \ | ||
make \ | ||
pigz \ | ||
sudo \ | ||
&& rm -rf /var/lib/apt/lists/* | ||
|
||
# Runtime dependencies | ||
RUN apt-get update \ | ||
&& apt-get install -y \ | ||
autoconf \ | ||
automake \ | ||
build-essential \ | ||
cpio \ | ||
curl \ | ||
gettext \ | ||
jq \ | ||
libbrotli-dev \ | ||
libicu-dev \ | ||
libkrb5-dev \ | ||
liblttng-ust-dev \ | ||
libnuma-dev \ | ||
libssl-dev \ | ||
libtool \ | ||
libunwind8 \ | ||
libunwind8-dev \ | ||
uuid-dev \ | ||
zlib1g-dev \ | ||
&& rm -rf /var/lib/apt/lists/* | ||
|
||
# Dependencies for VMR/source-build tests | ||
RUN apt-get update \ | ||
&& apt-get install -y \ | ||
elfutils \ | ||
file \ | ||
&& rm -rf /var/lib/apt/lists/* |
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.
None of these dependencies should be needed. We're not building .NET from source with this container. We're just building the test project which only requires the .NET installation which is already provided in the base sdk
image.
&& apt-get install -y \ | ||
python3 \ | ||
python3-venv \ | ||
dos2unix \ |
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.
This shouldn't be needed. The script file will have the correct line endings when committed in the repo.
python3-venv \ | ||
dos2unix \ | ||
&& rm -rf /var/lib/apt/lists/* | ||
COPY ./install-scancode.sh /tmp |
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 really isn't a need to have this logic defined in a script file. I'd prefer it to be defined directly in the Dockerfile.
@@ -0,0 +1,61 @@ | |||
FROM mcr.microsoft.com/dotnet/sdk:10.0-preview |
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 tag should be explicit about which distro version it's expecting.
FROM mcr.microsoft.com/dotnet/sdk:10.0-preview | |
FROM mcr.microsoft.com/dotnet/sdk:10.0-preview-noble |
src/ubuntu/manifest.json
Outdated
"dockerfile": "src/ubuntu/24.04/sdk/amd64", | ||
"os": "linux", | ||
"osVersion": "noble", | ||
"tags": { | ||
"ubuntu-24.04-sdk-amd64": {} |
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 tag name (and file path) should reflect its purpose. The CODEOWNERS
file will also need to be updated to reflect this.
"dockerfile": "src/ubuntu/24.04/sdk/amd64", | |
"os": "linux", | |
"osVersion": "noble", | |
"tags": { | |
"ubuntu-24.04-sdk-amd64": {} | |
"dockerfile": "src/ubuntu/24.04/source-build-test/amd64", | |
"os": "linux", | |
"osVersion": "noble", | |
"tags": { | |
"ubuntu-24.04-source-build-test-amd64": {} |
@@ -0,0 +1,61 @@ | |||
FROM mcr.microsoft.com/dotnet/sdk:10.0-preview |
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 are we not using Azure Linux?
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.
Yeah, good point. Should be 10.0-preview-azurelinux3.0
instead.
deactivate | ||
|
||
# Setup a script which executes scancode in the virtual environment | ||
RUN cat > /usr/local/bin/scancode <<EOF |
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.
Thoughts on checking this in next to the Dockerfile and copying it in? The benefit is improved maintainability.
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.
Update: 4db806c
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.
Based on all the comments, here's a suggested implementation:
FROM mcr.microsoft.com/dotnet/sdk:10.0-preview-azurelinux3.0-amd64 AS installer
# Install scancode
# Install instructions: https://scancode-toolkit.readthedocs.io/en/latest/getting-started/install.html#installation-as-a-library-via-pip
# See latest release at https://github.com/nexB/scancode-toolkit/releases
RUN SCANCODE_VERSION="32.3.3" \
&& python3 -m venv /venv \
&& source /venv/bin/activate \
&& pip install scancode-toolkit==$SCANCODE_VERSION
FROM mcr.microsoft.com/dotnet/sdk:10.0-preview-azurelinux3.0-amd64
COPY --from=installer /venv /venv
# Install necessary dependencies
RUN tdnf update -y \
&& tdnf install -y \
libgomp \
&& tdnf clean all
# Setup a script which executes scancode in the virtual environment
COPY --chmod=755 ./run-scancode.sh /usr/local/bin/scancode
RUN tdnf update -y && \ | ||
tdnf install -y \ | ||
libgomp \ | ||
&& tdnf clean all |
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.
Installing dependencies should be done before setting up the script file. This takes advantage of Docker's layer caching behavior on dev machines.
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, got it.
# Install instructions: https://scancode-toolkit.readthedocs.io/en/latest/getting-started/install.html#installation-as-a-library-via-pip | ||
# See latest release at https://github.com/nexB/scancode-toolkit/releases | ||
ENV venv="/tmp/scancode-env" | ||
RUN SCANCODE_VERSION="32.3.3" && \ |
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.
Preference is for the syntax to use &&
at the beginning of subsequent lines.
@@ -0,0 +1,29 @@ | |||
FROM mcr.microsoft.com/dotnet/sdk:10.0-preview-azurelinux3.0-amd64 |
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.
We should use a multi-stage Dockerfile instead so that scancode is installed in a separate stage and then copied to the final stage. This saves about 300 MB of disk space in the final image.
Related to dotnet/source-build#3816
Create a specifially image with .NET SDK and scancode installed.