-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Another attempt to fix the SDK insertion issue related to runtime/apphost downloads #77845
Another attempt to fix the SDK insertion issue related to runtime/apphost downloads #77845
Conversation
…indicate whether the restore should download those packages./
…s to the pack call too.
...ageServer/Microsoft.CodeAnalysis.LanguageServer/Microsoft.CodeAnalysis.LanguageServer.csproj
Show resolved
Hide resolved
<RuntimeIdentifiers Condition="'$(BaseOS)' != ''">$(BaseOS)</RuntimeIdentifiers> | ||
<RuntimeIdentifiers Condition="'$(TargetRid)' == '' and '$(BaseOS)' == ''">win-x64;win-arm64;linux-x64;linux-arm64;linux-musl-x64;linux-musl-arm64;osx-x64;osx-arm64</RuntimeIdentifiers> | ||
|
||
<!-- These indicate that the runtime/apphost packages should not be downloaded as part of build/restore --> |
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.
You might want to expand on this comment to explain why we're turning this off.
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.
will do in follow up
@ToddGrun I hope this works because it's nice and clean! |
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.
agreed, this change is much clearer as to what is going on
It was previously mentioned that instead of modifying the RuntimeIdentifiers property to attempt to avoid downloading the runtime/apphost packages during build, we could use the EnableRuntimePackDownload/EnableAppHostPackDownload flags instead. Earlier attempts to use these failed, very likely due to a poor understanding by me how msbuild propogates properties.
This PR reinstates setting the RuntimeIdentifiers property in all context as we did before, and utilizes the flags instead to indicate that we only want these packages downloaded during the restore/pack that we do when packing.