-
Notifications
You must be signed in to change notification settings - Fork 1.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
Dotnet publish with filter profile #685
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
<ItemGroup> | ||
<PackageReference Include="Newtonsoft.Json"> | ||
<Version>9.0.1</Version> | ||
</PackageReference> | ||
</ItemGroup> | ||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,8 @@ namespace Microsoft.NET.Build.Tasks | |
{ | ||
internal class RuntimeOptions | ||
{ | ||
public string tfm { get; set; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The property doesn't really align with the rest of the properties in the .json file.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Runtime options are only read by the dotnet host, so we have some leeway here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why can't this value be inferred from the existing "Framework" property? Also, TFMs don't have patch numbers in them. So if we had a bug fix in crossgen between 1.1.0 and 1.1.1, how would the cache get updated? The assembly was crossgened using 1.1.0 and cached in "netcoreapp1.1", but when I installed 1.1.1, there is no way to get the cache updated, because the assembly was already cached for "netcoreapp1.1". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The caching step is typically done by admin or installer scripts. We do not expect the end user to do this themselves. When this feature was spec'd, we decided to not do side by side install for the same TFM, to fix the crossgen bug- we would replace the assemblies already cached with the updated assembles, using the above mechanism The readytorun images are supposed to be resilient to runtime changes within the same TFM, worst case we are to fallback to jitting @gkhanna79 : Do correct me if i am wrong There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ramarag is correct. |
||
|
||
public RuntimeConfigFramework Framework { get; set; } | ||
|
||
public List<string> AdditionalProbingPaths { get; set; } | ||
|
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.
Taking both
TargetFramework
andTargetFrameworkMoniker
seems redundant.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 had a chat with @nguerrera there seems to be no other clean way
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.
There seems to be no good way to get reliably the same TargetFramework which was used to create the TargetFrameworkMoniker. Since we use the user supplied TargetFramework at build time, we want it to be the same here