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

Reduce ue4-build-prerequisites Windows image #144

Merged

Conversation

slonopotamus
Copy link
Collaborator

@slonopotamus slonopotamus commented Mar 26, 2021

This commit:

  1. Replaces base image from mcr.microsoft.com/dotnet/framework/sdk to mcr.microsoft.com/windows/servercore
    The former is 9GB while the latter is just 5GB.
    dotnet/framework/sdk contains lots of things that are not actually needed to build ue4,
    the hugest is VS Test Agent + VS Build Tools 2019 that take together 2.2GB

  2. Stops installing unneeded VS Build Tools workloads/components and only installs required minimum:

  • MSBuild (needed to build MSBuild-based UE4 tools, like UBT)
  • NuGet (required to run MSBuild and also checked in GenerateProjectFiles.bat)
  • C# compiler (needed to build C# UE4 tools)
  • .NET SDK 4.5 + 4.6.2 (same as above)
  • Windows 10 SDK (needed to build C++ code)
  • VCTools: C++ compiler, VC redist (same as above)

Cumulatively, this commit reduces ue4-build-prerequisites image from 24.5GB down to 10.7GB.
The most important consequence of this change is that it reduces ue4-minimal/ue4-full image sizes by the same delta


See #144 (comment) for test matrix of this PR.

@TBBle
Copy link
Collaborator

TBBle commented Mar 27, 2021

I like this approach. It has the extra benefit that it's feasible to build a ue4-prerequisites against Windows Insider Server Core base images, which don't exist for the mcr.microsoft.com/dotnet/framework/sdk, i.e. #138 would be resolvable.

I'd like to see someone (i.e. I'm not trying to push this all on you, @slonopotamus) test this for UE 4.20 onwards (i.e. the ue4-docker supported versions). I expect LTSC2019 and 20H2 is sufficient coverage; if they work, then anything between should work too, or can be dealt with when identified.

I'd also like to get some non-trivial projects built using the resulting images, e.g. the UE4 Shooter Game, to make sure we're not breaking something that is always going to be needed later in the game's build-process than ue4-minimal.

To be fair, I'm not sure that mooted 'later' issue exists, but #141 was a very similar issue, where part of the UE4 engine build was not happening until the first time you tried to build a game. That was observed and fixed on Linux, but it's possible the same thing will happen on Windows, and if it (for some reason) turns out to to depend on, e.g., .NET Core SDK (which this change excludes) then we'd want to know that and fix it.

@slonopotamus
Copy link
Collaborator Author

slonopotamus commented Mar 27, 2021

Additional thought: this PR allows us to control which exactly Windows SDK UE4 will be built against. We possibly want to change it to Microsoft.VisualStudio.Component.Windows81SDK.

@TBBle
Copy link
Collaborator

TBBle commented Mar 27, 2021

Why Windows 8.1 SDK? The current UE4 docs specify Windows SDK 10.0.18362 or newer.

UE 4.20.0 preferred 10.0.16299.0 and in UE 4.25.0 that was changed to prefer 10.0.18362.0, then 10.0.16299.0.

I also noticed that 4.25.0 is also when they started preferring the VS 2019 toolchain (14.2X), since that's defined in the same file, but they still list VS 2017 toolchains (14.1X) as 'preferred' there.

So we probably should be using the 10.0.16299.0 Windows SDK by default, since that's a preferred version for all ue4-docker-supported UE4 versions.

@slonopotamus
Copy link
Collaborator Author

slonopotamus commented Mar 28, 2021

Why Windows 8.1 SDK?

As far as I know, if you build your app against SDK version N, all its functionality is guaranteed to be present on Windows builds N and later. But the opposite is not true. If you use some function present in version N, you may discover that it doesn't exist at all on older Windows builds. So, the safest strategy is to build against the oldest supported SDK (and Windows 8.1 is not dead yet).

On the other side, it looks like UE4 itself doesn't use any specific functions from Windows 10 SDK and happily builds with any of them. So the only possible source of incompatibilities is game code.

Now, why I specified 17763 in this PR: 17763 is the version that was effectively installed by previous ue4-build-prerequisites code.

So, I see two strategies here:

  1. Use the oldest (8.1). If app compiles, it runs, but game cannot use stuff from newer SDKs.
  2. Use the latest. All modern SDK stuff is available for game, but it is game responsibility to guard with version checks in runtime because underlying OS might not have support.
  3. Use what Epic claim to be preferred. But since we save a single ue4-build-prerequisites for all engine versions currently, it isn't clear what engine version to look at. I also wonder why UE4 prefers some SDK versions over others.
  4. Continue to use 17763. This is actually the same as option 2 currently.

@slonopotamus
Copy link
Collaborator Author

WRT Windows SDK 10.0.18362: it looks like that it isn't available in VS Build Tools 2017. 17763 is the last one that is listed.

@slonopotamus
Copy link
Collaborator Author

UPD: I've also successfully built UE4 4.20.3 on both 2019ltsc and 20H2 using changes from this PR.

@TBBle
Copy link
Collaborator

TBBle commented Mar 28, 2021

I'm pretty sure the 'preferred' versions are what Epic are using on their CI system, so that'll match both what they QA, and what is in the Epic-distributed UE4 Installed Build version. Trying to match compilers for a project that works with both ue4-docker and the Epic-distributed build is how I came across this in the first place.

They mentioned their build farm version in their release notes, e.g. 4.20 lists Visual Studio 2017 v15.6.3 toolchain (14.13.26128) and Windows 10 SDK (10.0.12699.0), while 4.26 lists Visual Studio 2017 v15.9.4 toolchain (14.16.27023) and Windows 10 SDK (10.0.18362.0).

So interestingly, they are still using VS2017 for their published builds as well.

A concern with the Windows 8.1 SDK is that it may contain things that are incompatible with newer C++ standards, i.e. C++14, or conformance improvements activated by non-permissive build flags, that would work if used with the documented-as-supported Windows 10 SDK.

@slonopotamus
Copy link
Collaborator Author

So, do you want me to switch to 16299? Or we just leave 17763 (at least for this PR) because it matches how ue4-docker worked before?

@TBBle
Copy link
Collaborator

TBBle commented Mar 28, 2021

I'd leave it for now. I suspect 17763 is the default for the MS build tools installer because that's what Windows Server LTSC 2019 is using, which is still the most-recent Windows Server LTSC release (I forgot that we are on the VS 2017 channel) 17763 is the newest available there. The VS 2019 channel gives 10.0.19041.0 as the recommended version.

Personally, I lean towards using a newer Windows SDK, because that enables functionality that you can't enable otherwise, while still being able to choose to not use functionality that will not work on older versions of Windows. If you use an older SDK to guarantee compatibility with older (upstream-unsupported) versions of Windows, then you cannot opt-in to newer functionality.

I recognise this is not an uncontroversial position, so I think the easiest thing is to leave it as-is until more site-specific tuneable elements are exposed, and we move to the VS 2019 Build Tools installer channel, which (from memory) will allow finer-grained installation choices too, due to an improved dependency tree.

@slonopotamus
Copy link
Collaborator Author

Okay, then I'm leaving it as-is, with 17763.

@adamrehn
Copy link
Owner

@slonopotamus for a big change to the build prerequisites image like this, we'll need full test coverage before I can feel confident merging the pull request. That includes building all supported versions of the Unreal Engine (4.20 through 4.26) on both the latest LTSC and SAC releases of Windows, and verifying that all produced images are able to successfully package Unreal projects. If you're building the ue4-full image then the ue4-docker test command can be used to run these tests.

Given that this pull request also changes the base image to plain Windows Server Core rather than any .NET Framework runtime image, I'd also be interested in knowing exactly which versions of Windows are and are not affected by the .NET issue discussed on Microsoft's Known issues for containers page:

If you base your image directly on microsoft/windowsservercore, the .NET Framework might not install properly and no install error is indicated. Managed code might not run after the install is complete. Instead, base your image on microsoft/dotnet-framework:4.7.1 or later. As an example, you might see an error when building with MSBuild that's similar to the following:

C:\BuildTools\MSBuild\15.0\bin\Roslyn\Microsoft.CSharp.Core.targets(84,5): error MSB6003: The specified task executable "csc.exe" could not be run. Could not load file or assembly 'System.IO.FileSystem, Version=4.0.1.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' or one of its dependencies. The system cannot find the file specified.

The fact that Microsoft's documentation still includes the details of that issue suggests that one or more currently-supported versions of Windows still suffer from it. If that turns out to just be Windows Server 2016 then we can safely ignore it, since that particular LTSC release has a variety of known bugs and I'm happy to drop support for it anyway as discussed in #99 (comment), but if there are newer SAC releases that are affected too then we need to know.

@slonopotamus slonopotamus force-pushed the reduce-build-prerequisites branch 2 times, most recently from f48b8ed to 5af3d75 Compare March 30, 2021 07:38
@slonopotamus
Copy link
Collaborator Author

If you base your image directly on microsoft/windowsservercore, the .NET Framework might not install properly and no install error is indicated.

I have no idea what host/guest/Docker versions this is talking about. It could possibly be fixed in some updates so we will never hit it. I am running my tests on fully-updated systems.

If you're building the ue4-full image then the ue4-docker test command can be used to run these tests.

Okay, I can do that.

That includes building all supported versions of the Unreal Engine (4.20 through 4.26) on both the latest LTSC and SAC releases of Windows

I am afraid I don't have enough hardware to do that in any adequate amounts of time. I have only two boxes here, one builds ue4-minimal in ~17h while the other takes ~21h. What's worse, the faster of them has to stay on Windows 20H2, so I'd need to juggle with reinstalling different Windows versions on the slower one in order to test through them. So, building 7 engine versions across 7 Windows versions is going to take some time with my resources.

@TBBle
Copy link
Collaborator

TBBle commented Mar 30, 2021

MicrosoftDocs/visualstudio-docs#722 is the discussion about servercore-based issue, it specifically was known to affect the VS 2017 15.6 installer. There was supposed to be a follow-up to the note when the bug was fixed, but that seems to have been forgotten.

It does seem, from that discussion, to relate to the installer, not the base image, so if it turns out it wasn't fixed until the VS2019 installer, we would need to switch over to that first, which shouldn't be too painful.

@slonopotamus
Copy link
Collaborator Author

we would need to switch over to that first, which shouldn't be too painful.

We already use VS2019 installer since 2ff06d1

@TBBle
Copy link
Collaborator

TBBle commented Mar 30, 2021

Ah, sorry, you're right. I confused myself, thinking of the VS 2017 channel.

This commit:

1. Replaces base image from mcr.microsoft.com/dotnet/framework/sdk to mcr.microsoft.com/windows/servercore
The former is 9GB while the latter is just 5GB.
dotnet/framework/sdk contains lots of things that are not actually needed to build ue4,
the hugest is VS Test Agent + VS Build Tools 2019 that take together 2.2GB

2. Stops installing unneeded VS Build Tools workloads/components and only installs required minimum:
* MSBuild (needed to build MSBuild-based UE4 tools, like UBT)
* NuGet (needed to run MSBuild)
* C# compiler (needed to build C# UE4 tools)
* .NET SDK 4.5 + 4.6.2 (same as above)
* Windows 10 SDK (needed to build C++ code)
* VCTools: C++ compiler, VC redist (same as above)

Cumulatively, this commit reduces ue4-build-prerequisites image from 24.5GB down to 10.7GB.
The most important consequence of this change is that it reduces ue4-minimal/ue4-full image sizes by the same amount
@slonopotamus slonopotamus force-pushed the reduce-build-prerequisites branch from 5af3d75 to 608b19a Compare March 30, 2021 15:10
@slonopotamus
Copy link
Collaborator Author

slonopotamus commented Mar 30, 2021

Configurations checked with ue4-docker test against this PR

Empty cell means it wasn't tested yet.

ltsc2016 [#152] [#153] ltsc2019 2004 20H2
4.20 [note] ✔️ ✔️ ✔️ ✔️
4.21 ✔️ ✔️ ✔️ ✔️
4.22 ✔️ ✔️ ✔️ ✔️
4.23 ✔️ ✔️ ✔️ ✔️
4.24 ✔️ ✔️ ✔️ ✔️
4.25 [#146] ✔️ ✔️ ✔️ ✔️
4.26 [#146] ✔️ ✔️ ✔️ ✔️

I'm not going to test against 1709, 1803 and 1903: they all are EOL. See #145.

I'm not going to test against 1909, it will become EOL in 1.5 months.

This leaves us with 28 remaining combos.

I will first walk through all engine versions on both ltsc2019 + 20H2 (in parallel, since I have two boxes) and only then handle ltsc2016 + 2004.

@slonopotamus slonopotamus deleted the reduce-build-prerequisites branch March 30, 2021 16:23
@slonopotamus slonopotamus restored the reduce-build-prerequisites branch March 30, 2021 16:23
@slonopotamus
Copy link
Collaborator Author

Whoops, I didn't mean to close PR.

@slonopotamus slonopotamus reopened this Mar 30, 2021
@TBBle
Copy link
Collaborator

TBBle commented Mar 30, 2021

I propose that 2004 isn't a strongly-interesting test, because it's basically the same kernel as 20H2. So maybe now's the time to properly discuss dropping LTSC2016 (#99 (comment)), even though it's still supported upstream until January 2022.

@slonopotamus
Copy link
Collaborator Author

Okay, nice, I hit #146 on the first run.

@slonopotamus
Copy link
Collaborator Author

I've tested 4.20 on 20H2. It half-passes ue4-docker test: build-and-package.py passes while consume-external-deps.py fails because of adamrehn/conan-ue4cli#12. I'm going to count this as "passes".

@slonopotamus
Copy link
Collaborator Author

4.26 passes ue4-docker test on both 20H2 and 2019 LTSC if #149 is applied.

@slonopotamus
Copy link
Collaborator Author

slonopotamus commented Apr 5, 2021

Lo and behold, 20H2 testing is finished. Results are: 4.20 suffers from adamrehn/conan-ue4cli#12. 4.25 and 4.26 suffer from #146.

@slonopotamus
Copy link
Collaborator Author

So maybe now's the time to properly discuss dropping LTSC2016

@TBBle, here you are: #151

@slonopotamus
Copy link
Collaborator Author

slonopotamus commented Apr 11, 2021

Given current results, I'm not expecting anything new to break. Maybe it's time to merge this PR? @adamrehn. Anyway, I'm continuing builds.

P.S. I/O in Hyper-V virtual machines is slow like hell.

@slonopotamus
Copy link
Collaborator Author

Testing is finished!

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.

3 participants