-
Notifications
You must be signed in to change notification settings - Fork 175
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
Reduce ue4-build-prerequisites Windows image #144
Conversation
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. |
Additional thought: this PR allows us to control which exactly Windows SDK UE4 will be built against. We possibly want to change it to |
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. |
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:
|
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. |
UPD: I've also successfully built UE4 4.20.3 on both 2019ltsc and 20H2 using changes from this PR. |
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. |
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? |
I'd leave it for now. I suspect 17763 is the default for the MS build tools installer because 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. |
Okay, then I'm leaving it as-is, with 17763. |
@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 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:
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. |
f48b8ed
to
5af3d75
Compare
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.
Okay, I can do that.
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 |
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. |
We already use VS2019 installer since 2ff06d1 |
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
5af3d75
to
608b19a
Compare
Configurations checked with
|
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.
Whoops, I didn't mean to close PR. |
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. |
Okay, nice, I hit #146 on the first run. |
I've tested 4.20 on 20H2. It half-passes |
4.26 passes |
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. |
Testing is finished! |
This commit:
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
Stops installing unneeded VS Build Tools workloads/components and only installs required minimum:
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.