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

Use Microsoft.NETCore.App 1.1.1 when targeting netcoreapp1.1 #878

Merged
merged 3 commits into from
Feb 18, 2017

Conversation

dsplaisted
Copy link
Member

When targeting netcoreapp1.1, use version 1.1.1 of the package and shared runtime unless otherwise specified via RuntimeFrameworkVersion

Fixes (half of) #860

…ared runtime unless otherwise specified via RuntimeFrameworkVersion

Fixes (half of) dotnet#860
@@ -68,6 +68,9 @@ Copyright (c) .NET Foundation. All rights reserved.
-->

<PropertyGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'">
<!-- If targeting netcoreapp1.1, and RuntimeFrameworkVersion is not specified, use version 1.1.1 -->
<RuntimeFrameworkVersion Condition="'$(RuntimeFrameworkVersion)' == '' And '$(_TargetFrameworkVersionWithoutV)' == '1.1'">1.1.1</RuntimeFrameworkVersion>
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we were getting away from this. It sucks to have to patch this file whenever there's a patch. Also, given all of the issues with NETStandard.Libary 1.6 -> 1.6.1, I think it would be prudent to get to a place where simply upgrading the SDK never changes your package graph.

@nguerrera
Copy link
Contributor

Why only half?

@@ -68,6 +68,9 @@ Copyright (c) .NET Foundation. All rights reserved.
-->

<PropertyGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'">
<!-- If targeting netcoreapp1.1, and RuntimeFrameworkVersion is not specified, use version 1.1.1 -->
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add a better comment here explaining why we are doing this.

@dsplaisted
Copy link
Member Author

@MattGertz for approval

Scenario

  • Build an app targeting .NET Core 1.1. Currently, 2 DiaSymReader dlls totalling 2MB in size will unnecessarily be copied to the output folder. With this change, an updated version of the DiaSymReader package will be used which doesn't have this issue
  • Build a self-contained app targeting .NET Core 1.0. Currently, this will use version 1.0.0 of the runtime unless you specify a different version in the RuntimeFrameworkVersion property. This change will default to 1.0.4 of the runtime, which is currently the latest patch version of 1.0.

Bug

#860

Workarounds

Set RuntimeFrameworkVersion property to 1.1.1

Risk

Low

Performance Impact

Low

Regression Analysis

Not a regression

dsplaisted and others added 2 commits February 17, 2017 13:27
Update tests and add comment about the RuntimeFrameworkVersion selection logic.
Update version of the CLI we use so that it will have 1.0.4  and 1.1.1 for our tests to run with.
@srivatsn srivatsn merged commit 2052949 into dotnet:master Feb 18, 2017
mmitche pushed a commit to mmitche/sdk that referenced this pull request Jun 5, 2020
…0190814.8 (dotnet#878)

- Microsoft.AspNetCore.Mvc.Analyzers - 3.0.0-preview9.19414.8
- Microsoft.AspNetCore.Mvc.Api.Analyzers - 3.0.0-preview9.19414.8
- Microsoft.AspNetCore.Analyzers - 3.0.0-preview9.19414.8
- Microsoft.AspNetCore.Components.Analyzers - 3.0.0-preview9.19414.8
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

5 participants