-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Enable building tests with UseLocalAppHostPack=true #99354
Conversation
Tagging subscribers to this area: @hoyosjs Issue DetailsContributes to #97791
|
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.
Quick question. Otherwise, lgtm
@@ -543,6 +543,7 @@ | |||
<Target Name="RestorePackage"> | |||
<PropertyGroup> | |||
<_ConfigurationProperties>/p:TargetOS=$(TargetOS) /p:TargetArchitecture=$(TargetArchitecture) /p:Configuration=$(Configuration) /p:CrossBuild=$(CrossBuild)</_ConfigurationProperties> | |||
<_ConfigurationProperties Condition="'$(UseLocalAppHostPack)' == 'true'">$(_ConfigurationProperties) -p:EnableAppHostPackDownload=false -p:EnableTargetingPackDownload=false -p:EnableRuntimePackDownload=false</_ConfigurationProperties> |
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.
I guess the restore proj doesn't import eng/targetingpacks.targets? If it does it's enough to pass down the option, right?
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.
Yup, this can be removed when eng/targetingpacks.targets
is imported. I kept this stopgap PR small. (I will try importing it and open a separate PR)
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
Contributes to #97791
This is to enable building clr tests with -p:UseLocalAppHostPack=true (once the host subset is already built). This option is useful for community-supported platforms, which are listed in KnownFrameworkReference, KnownRuntimePack etc. in the dotnet/installer repo and we don't expect the official SDK to provide packs for these platforms.