-
Notifications
You must be signed in to change notification settings - Fork 109
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
Add brotli to sourcebuild container #1197
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.
This new dependency will need to be applied equally to all the container images used for source build. Please ensure that all of these contain brotli if they don't already: https://github.com/dotnet/sdk/blob/7dc1e29627f20d47213ef2623b9e74b0e3cb4a4f/eng/pipelines/templates/variables/vmr-build.yml#L15-L28
Thanks for the list, thought it was strange we only had one source build image. |
One thing is: I only need to update non-portable builds. Which ones are those? |
All of the builds for source build are non-portable. |
Does source build support cross build? Or does it just share the container? |
We don't run cross build in these containers for source build. Unified build has its own set of containers for cross build: https://github.com/dotnet/sdk/blob/7dc1e29627f20d47213ef2623b9e74b0e3cb4a4f/eng/pipelines/templates/variables/vmr-build.yml#L29-L48 |
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.
What about Fedora: https://github.com/dotnet/dotnet-buildtools-prereqs-docker/blob/main/src/fedora/40/amd64/Dockerfile. I see it already implicitly has libbrotli
installed, but not brotli-devel
.
@mthalman Thanks, fixed. |
Part of dotnet/runtime#107225