-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
[mono] Use $(HostArch) for LLVM packages #34322
Conversation
would be nice if we make the property names platform agnostic, as we can alway use |
You don't need to duplicate the Versions.props, we're using the same version anyway. There's an issue somewhere about turning the LLVM package into a proper meta-package so we don't need these explicit package references anymore. |
Thanks a lot! |
@@ -6,6 +6,7 @@ | |||
|
|||
<!-- Set default Platform --> | |||
<PropertyGroup> | |||
<HostArch>$([System.Runtime.InteropServices.RuntimeInformation]::ProcessArchitecture.ToString().ToLowerInvariant)</HostArch> |
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.
Does it matter that ToLowerInvariant
is missing parenthesis?
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.
Oops
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.
@am11 however it works without them too
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.
Great, i was also thinking that it (might) just work in property functions. 👍
Currently we always download x64 packages -- it's ok for our workloads since we can use them for cross-compilation to arm64 but it's not possbile to use them when you compile dotnet/runtime on an arm64 machine.
Fixes #34311
@akoeplinger should I also duplicate these: https://github.com/dotnet/runtime/blob/master/eng/Versions.props#L127-L132 ?