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

Add brotli to sourcebuild container #1197

Merged
merged 4 commits into from
Sep 10, 2024
Merged

Add brotli to sourcebuild container #1197

merged 4 commits into from
Sep 10, 2024

Conversation

agocke
Copy link
Member

@agocke agocke commented Sep 6, 2024

@agocke agocke requested review from a team as code owners September 6, 2024 06:31
Copy link
Member

@mthalman mthalman left a 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

@agocke
Copy link
Member Author

agocke commented Sep 6, 2024

Thanks for the list, thought it was strange we only had one source build image.

@agocke
Copy link
Member Author

agocke commented Sep 6, 2024

One thing is: I only need to update non-portable builds. Which ones are those?

@mthalman
Copy link
Member

mthalman commented Sep 6, 2024

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.

@agocke
Copy link
Member Author

agocke commented Sep 6, 2024

Does source build support cross build? Or does it just share the container?

@mthalman
Copy link
Member

mthalman commented Sep 6, 2024

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

Copy link
Member

@mthalman mthalman left a 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.

@agocke
Copy link
Member Author

agocke commented Sep 10, 2024

@mthalman Thanks, fixed.

@mthalman mthalman enabled auto-merge (squash) September 10, 2024 17:46
@mthalman mthalman merged commit 8bb52e2 into main Sep 10, 2024
73 checks passed
@agocke agocke deleted the add-brotli-devel branch September 10, 2024 19:51
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.

2 participants