Handle non-x86 in dotnet-install.sh on *nix #4132
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Sounds reasonable to me. @tmat? |
|
LGTM. It would be good to add some comments to |
omajid
force-pushed
the
dotnet-install-non-x86-architectures
branch
from
d0f41e1 to
a1afcdc
Compare
I replaced |
dagood
approved these changes
Oct 18, 2019
dagood
reviewed
Oct 18, 2019
omajid
force-pushed
the
dotnet-install-non-x86-architectures
branch
from
a1afcdc to
21a7fac
Compare
dagood
approved these changes
Oct 18, 2019
|
@dagood should this PR be merged? |
Here's the problem: Tasks in arcade, such as `InstallDotNetCore` end up calling this script to install .NET Core for development (building and testing projects). In certain configurations, such as how AspNetCore's global.json is setup, this dotnet-install.sh script is called multiple times to install multiple architectures of .NET Core. This script ends up calling the CLI's dotnet-install.sh to do the heavy work. But it always tells it to use the same path (eg .dotnet), even for different architectures. This works mostly okay if x64 (and x86) are the only architectures. Because x86 is ignored and only x64 gets installed on *nix. On Windows, x64 goes in .dotnet and x86 goes in .dotnet/x86. But as soon as multiple incompatible architectures enter the picture, things start breaking down. For example, as soon as I try and add arm64 to a global.json, this is what can happen: 0. The main bootstrap process (exected as part of ./build.sh) installs an arm64 SDK in ~/.dotnet to run msbuild. 1. The `InstallDotNetCore` tasks tries to install the versions listed in global.json in order of their appearance. 2. This dotnet-install.sh script installs x64 SDK and/or runtime in .dotnet/ (if that version is different from the boostrap SDK). 3. InstallDotNetCore task skips x86 (becuase this is a non-Windows platform). 4. This dotnet-install.sh scripts calls CLI's dotnet-install.sh to install an arm64 SDK and/or runtime in .dotnet/. 5. CLI's dotnet-install.sh sees that the runtime and/or SDK versions are already installed in .dotnet/ and doesn't do anything. 6. When it comes time to execute `dotnet` again, the .dotnet directory contains multiple versions (some from the bootstrap, some from `InstallDotNetCore` task) of the runtime and host. If the version in global.json is newer than the version installed initially, the `dotnet` binary tries to load that latest (and incompatible) x64 hostfxr that was installed. 7. Build blows up because an arm64 process can't load up an x64 libhostfxr. This change uses the approach that the Windows build takes for x86 and x64: .NET Core for the current architecture of the machine is installed at `.dotnet/`. Other SDKs/Runtimes are installed at `.dotnet/$OTHER_ARCH/`. This change doesn't address a larger question: does it make sense to install mutliple SDKs for different architectures? It makes sense to install x64 and x86 on x64 machiens for testing. Does it make any sense to install x86 (or even x64) on arm64? Perhaps it might better if the list in global.json was filtered by Arcade so only the architecture of the build machine and any architectures known to be compatible with it were installed.
omajid
force-pushed
the
dotnet-install-non-x86-architectures
branch
from
21a7fac to
a4df442
Compare
|
I have rebased the patch and fixed up the merge conflicts now. |
|
Thanks for the review and merging this change! |
omajid
added a commit
to omajid/dotnet-aspnetcore
that referenced
this pull request
Oct 23, 2019
arcade uses the runtime section of global.json to decide which architecture + runtime combination needs to be installed. With dotnet/arcade#4132 arcade can install foreign SDKs in separate locations correctly. This change, suggested by @dougbu, makes arcade always install the runtime for the local architecture (which means it should work on arm64 and x64) as well as the x86 architecture (skipped on Linux). This gets us a working SDK/Runtime combo on arm64.
dougbu
pushed a commit
to dotnet/aspnetcore
that referenced
this pull request
Nov 2, 2019
arcade uses the runtime section of global.json to decide which architecture + runtime combination needs to be installed. With dotnet/arcade#4132 arcade can install foreign SDKs in separate locations correctly. This change, suggested by @dougbu, makes arcade always install the runtime for the local architecture (which means it should work on arm64 and x64) as well as the x86 architecture (skipped on Linux). This gets us a working SDK/Runtime combo on arm64.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Here's the problem:
Tasks in arcade, such as
InstallDotNetCoreend up calling this script to install .NET Core for development (building and testing projects).In certain configurations, such as how AspNetCore's
global.jsonis setup, thisdotnet-install.shscript is called multiple times to install multiple architectures of .NET Core.This script ends up calling the CLI's
dotnet-install.shto do the heavy lifting. But it always tells it to use the same path (eg.dotnet), even for different architectures.This works mostly okay if x64 (and x86) are the only architectures. Because x86 is ignored and only x64 gets installed on *nix. On Windows, x64 goes in
.dotnetand x86 goes in.dotnet/x86.But as soon as multiple incompatible architectures enter the picture, things start breaking down. For example, as soon as I try and add arm64 to a
global.json, this is what can happen:The main bootstrap processes (executed as part of ./build.sh) installs an arm64 SDK in ~/.dotnet to run msbuild.
The
InstallDotNetCoretasks tries to install the versions listed inglobal.jsonin order of their appearance.This
dotnet-install.shscript installs x64 in.dotnet/.InstallDotNetCoretask skips x86 (because it's non-Windows platform)This
dotnet-install.shscripts calls CLI'sdotnet-install.shto install arm64 in.dotnet/CLI's
dotnet-install.shsees that the runtime and/or SDK versions are already installed in.dotnet/and doesn't do anything.When it comes time to execute
dotnetagain, the.dotnetdirectory contains multiple versions (some from the bootstrap, some from theInstallDotNetCoretask). If the version in global.json is newer than the version installed initially, thedotnetbinary tries to load that latest (and incompatible) x64 hostfxr that was installed.Build blows up because an arm64 process can't load up an x64
libhostfxr.so.This change uses the approach that the Windows build takes for x86 and x64: .NET Core for the current architecture of the machine is installed at
.dotnet/. Other SDKs/Runtimes are installed at.dotnet/$OTHER_ARCH/.This change doesn't address a larger question: does it make sense to install mutliple SDKs for different architectures? It makes sense to install x64 and x86 on x64 machines for testing. Does it make any sense to install x86 (or even x64) on arm64?
Perhaps it might better if the list in
global.jsonwas filtered by Arcade so only the architecture of the build machine and any architectures known to be compatible with it were installed.For more background, see dotnet/aspnetcore#14790 and #2815
cc @dougbu @dagood @markwilkie @vatsan-madhavan @chcosta @tmat