-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Allow mono samples to build and run again #54089
Conversation
Tagging subscribers to this area: @directhex Issue DetailsCopied from src/libraries/Directory.Build.props Evidently this is sufficient for the console samples to build again:
|
@akoeplinger @ViktorHofer once again I don't really know what I'm doing... How wrong is this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, though we should probably look at moving these properties from src/librares/Directory.Build.props to some common location so we don't need to duplicate them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix. Can you please avoid the duplication and move the properties into the root's Directory.Build.props file after the block in which PackageRID
and $(NetCoreAppCurrent)
is defined?
Also when doing so, please remove this block:
runtime/src/libraries/Directory.Build.props
Lines 139 to 146 in bc16a7b
<MicrosoftNetCoreAppRefPackDir>$([MSBuild]::NormalizeDirectory('$(ArtifactsBinDir)', 'microsoft.netcore.app.ref'))</MicrosoftNetCoreAppRefPackDir> | |
<MicrosoftNetCoreAppRefPackRefDir>$([MSBuild]::NormalizeDirectory('$(MicrosoftNetCoreAppRefPackDir)', 'ref', '$(NetCoreAppCurrent)'))</MicrosoftNetCoreAppRefPackRefDir> | |
<MicrosoftNetCoreAppRefPackDataDir>$([MSBuild]::NormalizeDirectory('$(MicrosoftNetCoreAppRefPackDir)', 'data'))</MicrosoftNetCoreAppRefPackDataDir> | |
<MicrosoftNetCoreAppRuntimePackDir>$([MSBuild]::NormalizeDirectory('$(ArtifactsBinDir)', 'microsoft.netcore.app.runtime.$(PackageRID)', '$(Configuration)'))</MicrosoftNetCoreAppRuntimePackDir> | |
<MicrosoftNetCoreAppRuntimePackRidDir>$([MSBuild]::NormalizeDirectory('$(MicrosoftNetCoreAppRuntimePackDir)', 'runtimes', '$(PackageRID)'))</MicrosoftNetCoreAppRuntimePackRidDir> | |
<MicrosoftNetCoreAppRuntimePackRidLibTfmDir>$([MSBuild]::NormalizeDirectory('$(MicrosoftNetCoreAppRuntimePackRidDir)', 'lib', '$(NetCoreAppCurrent)'))</MicrosoftNetCoreAppRuntimePackRidLibTfmDir> | |
<MicrosoftNetCoreAppRuntimePackNativeDir>$([MSBuild]::NormalizeDirectory('$(MicrosoftNetCoreAppRuntimePackRidDir)', 'native'))</MicrosoftNetCoreAppRuntimePackNativeDir> |
bf2eeee
to
fc797d4
Compare
@ViktorHofer Done, thanks! |
I believe this code block is now dead: runtime/src/mono/wasm/build/WasmApp.Native.targets Lines 33 to 36 in ccec848
|
And a few more that should also be removed. Would be great if you could do that as well in the PR:
|
edede82
to
59baba6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than removing the entry from src/mono/wasm/build/WasmApp.InTree.props
, LGTM!
Applied all the suggested changes, aside from ones that made the tests fail
Any idea why tests started failing when removing the properties from the two wasm files? I thought those are dead anyway. |
Not sure - I didn't have a chance to look into it yet. If I had to guess - the wasm runtime build is different from other platforms because there's a step that runs after the libraries are built. So perhaps the final runtime artifacts for testing are in a slightly different location. |
Which files? And where are these failing? |
@radical This is what I did initially after adding the properties to the root I don't recall what was failing specifically, but it was the general wasm app building for the libraries tests, I believe. (don't seem to have AzDO links to the oldest CI runs) |
Those files are used outside the tree too, like in workloads. So, we can't depend on runtime repo's |
There's one failure still that I can't explain:
The consoles are empty https://helix.dot.net/api/2019-06-17/jobs/54ff2c22-c6df-4b58-8d1b-fe28a368448a/workitems/JIT.IL_Conformance/console
Not sure if that could be related to the changes here. |
Copied from src/libraries/Directory.Build.props Evidently this is sufficient for the console samples to build again: ``` $ make run MONO_CONFIG=Release MONO_ENV_OPTIONS= ../../../.././dotnet.sh publish -c Release -r linux-x64 Microsoft (R) Build Engine version 16.11.0-preview-21254-21+e73d08c28 for .NET Copyright (C) Microsoft Corporation. All rights reserved. Determining projects to restore... All projects are up-to-date for restore. Used runtime pack: /home/aleksey/working/dotnet-runtime/artifacts/bin/microsoft.netcore.app.runtime.linux-x64/Release/ Used runtime pack: /home/aleksey/.nuget/packages/microsoft.aspnetcore.app.runtime.linux-x64/6.0.0-preview.4.21253.5 HelloWorld -> /home/aleksey/working/dotnet-runtime/artifacts/bin/HelloWorld/x64/Release/linux-x64/HelloWorld.dll HelloWorld -> /home/aleksey/working/dotnet-runtime/artifacts/bin/HelloWorld/x64/Release/linux-x64/publish/ COMPlus_DebugWriteToStdErr=1 \ MONO_ENV_OPTIONS="" \ ../../../../artifacts/bin/HelloWorld/x64/Release/linux-x64/publish/HelloWorld Hello World from Mono! System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e HelloWorld, Version=6.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35 .NET 6.0.0-dev ```
They are not based on the ArtifactsBinDir property
The tests seem to otherwise look in a coreclr directory for libmono
ae0e9a3
to
cd0656b
Compare
Copied from src/libraries/Directory.Build.props
Evidently this is sufficient for the console samples to build again: