Avoid BuildHost crash in Mono due to missing types #74392
Closed
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
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
dotnet-policy-service
bot
added
the
Community
uweigand
force-pushed
the
buildhost-mono-types
branch
from
3310426
to
2ddb948
Compare
dibarbet
approved these changes
Jul 15, 2024
There was a problem hiding this comment.
I'm fine with this, but would like @jasonmalinowski to take a look as well.
JoeRobich
approved these changes
Jul 16, 2024
jasonmalinowski
approved these changes
Jul 16, 2024
| return LoadProjectFileCoreAsync(projectFilePath, languageName, cancellationToken); | ||
| } | ||
|
|
||
| [MethodImpl(MethodImplOptions.NoInlining)] // Do not inline this, since this uses MSBuild types which are being loaded by the caller |
There was a problem hiding this comment.
You may want to make a note that this was mono specific, lest somebody tries to undo this change being confused why it works on other platforms.
There was a problem hiding this comment.
Sure, I've updated the comment now.
|
I filed #74407 to look at what it'd take to get our tests running on mono to catch issues like this. |
uweigand
force-pushed
the
buildhost-mono-types
branch
from
2ddb948
to
26593eb
Compare
The LoadProjectFileAsync routine calls EnsureMSBuildLoaded to make sure the Microsoft.Build.dll is accessible, but requires types from that assembly within the routine itself. This may not always work with Mono, as the JIT may look up the types during compilation. Fixed by splitting out a LoadProjectFileCoreAsync routine, similarly to what is already done for GetProjectsInSolution. Fixes dotnet/runtime#101121.
uweigand
force-pushed
the
buildhost-mono-types
branch
from
26593eb
to
3bcf996
Compare
3 tasks
|
Not sure why this is failing now after just a comment change ... seems an infrastructure issue. |
omajid
added a commit
to omajid/dotnet-roslyn
that referenced
this pull request
Jul 24, 2024
I wanted to run the roslyn test suite on Linux on arm64. Building
everything worked, but running tests would fail with errors like:
Errors Microsoft.CodeAnalysis.CSharp.Emit.UnitTests_1
Could not find 'dotnet' host for the 'X64' architecture.
You can resolve the problem by installing the 'X64' .NET.
With the changes in this commit, I can build and run all tests using:
$ ./eng/build.sh --restore --build --pack
$ ./eng/build.sh --test
I would, eventually, like to be able to run the tests on other
platforms, including s390x and ppc64le. Hopefully that might help
identify/fix issues like dotnet#74392
earlier.
One thing I am not too happy about in this change is how many places
need an explicit addition of "linux-arm64". That won't scale as we want
to add more architectures.
omajid
added a commit
to omajid/dotnet-roslyn
that referenced
this pull request
Jul 24, 2024
I wanted to run the roslyn test suite on Linux on arm64. Building
everything worked, but running tests would fail with errors like:
Errors Microsoft.CodeAnalysis.CSharp.Emit.UnitTests_1
Could not find 'dotnet' host for the 'X64' architecture.
You can resolve the problem by installing the 'X64' .NET.
With the changes in this commit, I can build and run all tests using:
$ ./eng/build.sh --restore --build --pack
$ ./eng/build.sh --test
I would, eventually, like to be able to run the tests on other
platforms, including s390x and ppc64le. Hopefully that might help
identify/fix issues like dotnet#74392
earlier.
One thing I am not too happy about in this change is how many places
need an explicit addition of "linux-arm64". That won't scale as we want
to add more architectures.
omajid
added a commit
to omajid/dotnet-roslyn
that referenced
this pull request
Jul 24, 2024
I wanted to run the roslyn test suite on Linux on arm64. Building
everything worked, but running tests would fail with errors like:
Errors Microsoft.CodeAnalysis.CSharp.Emit.UnitTests_1
Could not find 'dotnet' host for the 'X64' architecture.
You can resolve the problem by installing the 'X64' .NET.
With the changes in this commit, I can build and run all tests using:
$ ./eng/build.sh --restore --build --pack
$ ./eng/build.sh --test
I would, eventually, like to be able to run the tests on other
platforms, including s390x and ppc64le. Hopefully that might help
identify/fix issues like dotnet#74392
earlier.
One thing I am not too happy about in this change is how many places
need an explicit addition of "linux-arm64". That won't scale as we want
to add more architectures.
omajid
added a commit
to omajid/dotnet-roslyn
that referenced
this pull request
Jul 26, 2024
I wanted to run the roslyn test suite on Linux on arm64. Building
everything worked, but running tests would fail with errors like:
Errors Microsoft.CodeAnalysis.CSharp.Emit.UnitTests_1
Could not find 'dotnet' host for the 'X64' architecture.
You can resolve the problem by installing the 'X64' .NET.
With the changes in this commit, I can build and run all tests using:
$ ./eng/build.sh --restore --build --pack
$ ./eng/build.sh --test
I would, eventually, like to be able to run the tests on other
platforms, including s390x and ppc64le. Hopefully that might help
identify/fix issues like dotnet#74392
earlier.
One thing I am not too happy about in this change is how many places
need an explicit addition of "linux-arm64". That won't scale as we want
to add more architectures.
omajid
added a commit
to omajid/dotnet-roslyn
that referenced
this pull request
Jul 26, 2024
I wanted to run the roslyn test suite on Linux on arm64. Building
everything worked, but running tests would fail with errors like:
Errors Microsoft.CodeAnalysis.CSharp.Emit.UnitTests_1
Could not find 'dotnet' host for the 'X64' architecture.
You can resolve the problem by installing the 'X64' .NET.
With the changes in this commit, I can build and run all tests using:
$ ./eng/build.sh --restore --build --pack
$ ./eng/build.sh --test
I would, eventually, like to be able to run the tests on other
platforms, including s390x and ppc64le. Hopefully that might help
identify/fix issues like dotnet#74392
earlier.
One thing I am not too happy about in this change is how many places
need an explicit addition of "linux-arm64". That won't scale as we want
to add more architectures.
333fred
pushed a commit
that referenced
this pull request
Jul 29, 2024
I wanted to run the roslyn test suite on Linux on arm64. Building
everything worked, but running tests would fail with errors like:
Errors Microsoft.CodeAnalysis.CSharp.Emit.UnitTests_1
Could not find 'dotnet' host for the 'X64' architecture.
You can resolve the problem by installing the 'X64' .NET.
With the changes in this commit, I can build and run all tests using:
$ ./eng/build.sh --restore --build --pack
$ ./eng/build.sh --test
I would, eventually, like to be able to run the tests on other
platforms, including s390x and ppc64le. Hopefully that might help
identify/fix issues like #74392
earlier.
One thing I am not too happy about in this change is how many places
need an explicit addition of "linux-arm64". That won't scale as we want
to add more architectures.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
The test run failed. Think it's still using stale integration test bits. Worry that this PR may be snagged on stale state. I rebased locally, pushed to a new branch and opened a new PR. Hopefully that one will get past these issues #74994. |
|
Closing as I got this change to merge in cleanly in #74994. Sorry for losing track of this. |
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.
The LoadProjectFileAsync routine calls EnsureMSBuildLoaded to make sure the Microsoft.Build.dll is accessible, but requires types from that assembly within the routine itself. This may not always work with Mono, as the JIT may look up the types during compilation.
Fixed by splitting out a LoadProjectFileCoreAsync routine, similarly to what is already done for GetProjectsInSolution.
Fixes dotnet/runtime#101121.