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.

Sign up for GitHub

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

Jump to bottom

Add Dockerfiles for Windows Server 2025 #1183

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Add Dockerfiles for Windows Server 2025 #1183

wants to merge 14 commits into from

Conversation

lbussell
Copy link
Collaborator

Related: #1146

@lbussell lbussell marked this pull request as ready for review November 25, 2024 18:54
mthalman
mthalman reviewed Nov 25, 2024
View reviewed changes
eng/dockerfile-templates/runtime/Dockerfile Outdated Show resolved Hide resolved
eng/dockerfile-templates/runtime/Dockerfile Outdated Show resolved Hide resolved
eng/dockerfile-templates/runtime/Dockerfile Outdated
@@ -57,7 +96,7 @@ RUN `
&& mkdir patch `
&& expand patch.msu patch -F:* `
&& del /F /Q patch.msu `
&& dism /Online /Quiet /Add-Package /PackagePath:C:\patch\windows10.0-{{VARIABLES[cat("kb|", OS_VERSION_NUMBER, "|", when(PRODUCT_VERSION = "3.5", "default", PRODUCT_VERSION))]}}-x64{{if OS_VERSION_NUMBER != "ltsc2019" || PRODUCT_VERSION = "4.8":-ndp48}}{{if PRODUCT_VERSION = "4.8.1":1}}.cab `
&& dism /Online /Quiet /Add-Package /PackagePath:C:\patch\{{winver}}-{{VARIABLES[cat("kb|", OS_VERSION_NUMBER, "|", when(PRODUCT_VERSION = "3.5", "default", PRODUCT_VERSION))]}}-x64{{if OS_VERSION_NUMBER != "ltsc2019" || PRODUCT_VERSION = "4.8":-ndp48}}{{if isServer2025 || isNet481:1}}.cab `
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think isServer2025 is redundant in that condition.

Suggested change
&& dism /Online /Quiet /Add-Package /PackagePath:C:\patch\{{winver}}-{{VARIABLES[cat("kb|", OS_VERSION_NUMBER, "|", when(PRODUCT_VERSION = "3.5", "default", PRODUCT_VERSION))]}}-x64{{if OS_VERSION_NUMBER != "ltsc2019" || PRODUCT_VERSION = "4.8":-ndp48}}{{if isServer2025 || isNet481:1}}.cab `
&& dism /Online /Quiet /Add-Package /PackagePath:C:\patch\{{winver}}-{{VARIABLES[cat("kb|", OS_VERSION_NUMBER, "|", when(PRODUCT_VERSION = "3.5", "default", PRODUCT_VERSION))]}}-x64{{if OS_VERSION_NUMBER != "ltsc2019" || PRODUCT_VERSION = "4.8":-ndp48}}{{if isNet481:1}}.cab `

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is necessary. For server 2025, there is no distinction between .NET 3.5 latest patch .NET 4.8.1 latest patch. To keep the implementation simpler, I directed both of those cases to this applyPatch branch. Either we need to keep this condition so that the filename is correct in the Server 2025 3.5 Dockerfile, or we can add the 1 suffix to the "apply latest 3.5 patch" instructions instead. Then the only difference would be the comment.

eng/dockerfile-templates/runtime/Dockerfile Outdated
@@ -70,7 +109,7 @@ else
: # ngen .NET Fx
{{if PRODUCT_VERSION != "4.7.2":&& }}%windir%\Microsoft.NET\Framework64\v4.0.30319\ngen uninstall "Microsoft.Tpm.Commands, Version=10.0.0.0, Culture=Neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=amd64" `^ else
: # Ngen top of assembly graph to optimize a set of frequently used assemblies
{{if PRODUCT_VERSION = "3.5" || ((PRODUCT_VERSION = "4.8" && !is48SecurityRelease) || PRODUCT_VERSION = "4.8.1"):&& }}%windir%\Microsoft.NET\Framework64\v4.0.30319\ngen install "Microsoft.PowerShell.Utility.Activities, Version=3.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35" `
{{if isNet35 || ((isNet48 && !is48SecurityRelease) || (isNet481 && !isServer2025)):&& }}%windir%\Microsoft.NET\Framework64\v4.0.30319\ngen install "Microsoft.PowerShell.Utility.Activities, Version=3.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35" `
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{{if isNet35 || ((isNet48 && !is48SecurityRelease) || (isNet481 && !isServer2025)):&& }}%windir%\Microsoft.NET\Framework64\v4.0.30319\ngen install "Microsoft.PowerShell.Utility.Activities, Version=3.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35" `
{{if isNet35 || ((isNet48 && !is48SecurityRelease) || (isNet481 && isServer2022)):&& }}%windir%\Microsoft.NET\Framework64\v4.0.30319\ngen install "Microsoft.PowerShell.Utility.Activities, Version=3.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35" `

manifest.datestamps.json Outdated Show resolved Hide resolved
@lbussell
Copy link
Collaborator Author

lbussell@36261be is an alternative to 6691140. I could go either way.

@mthalman
Copy link
Member

lbussell@36261be is an alternative to 6691140. I could go either way.

I don't mind the new style. But I do think that it should be applied consistently across all the templates in a separate PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roozbehid roozbehid roozbehid left review comments

@mthalman mthalman mthalman approved these changes

@MichaelSimons MichaelSimons Awaiting requested review from MichaelSimons

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lbussell @mthalman @roozbehid