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

Fix Mono AOT LLVM #2539

Merged
merged 3 commits into from
Mar 8, 2024
Merged

Conversation

caaavik-msft
Copy link
Contributor

@caaavik-msft caaavik-msft commented Mar 8, 2024

Similar to what broke with the WASM runs, it seems that Mono AOT LLVM also will always append the publish directory onto the end of the OutDir and OutputPath that we pass in, so we need to remove it from the end of those paths and then append it the executable path and working directory.

We are doing some more testing on this first before it can be ready to merge however, but this seems like the likely fix.
Testing is done, this PR is ready for a merge.

@caaavik-msft
Copy link
Contributor Author

I'm fixing a 2nd bug in this PR, if you specified a moniker monoaotllvmnet90 it was still using the non .NET specific MonoAOTLLVM moniker in the runtime and so the DotNetSdkValidator was failing.

Copy link
Member

@LoopedBard3 LoopedBard3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with successful local tests when using monoaotllvmnet90 runtime.

@timcassell
Copy link
Collaborator

Do we need to account for MonoAOTLLVM and Wasm monikers in the sdk validator? And should we deprecate those?

@caaavik-msft
Copy link
Contributor Author

I have created another issue for that: #2540

@timcassell
Copy link
Collaborator

It would also be nice to get integration tests for this (#2536).

@timcassell timcassell merged commit 9a9d7e7 into dotnet:master Mar 8, 2024
8 checks passed
@timcassell
Copy link
Collaborator

Thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants