Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Switch to using MicrosoftNETCorePlatformsPackageVersion #19130

Merged
merged 1 commit into from
Jul 26, 2018

Conversation

chsienki
Copy link

Switch to using MicrosoftNETCorePlatformsPackageVersion as the RuntimeFrameworkVersion parameter in the test wrappers and SDK props file.

Ensures there is always the correct version of netcoreapp selected during restore + build, and prevents the tests from failing to build because they were restored against a different version.

…eFrameworkVersion parameter in the test wrappers and SDK props file
@chsienki
Copy link
Author

After merging #19044 I realized that I was using the wrong version to restore against. If it happens to line up (as it did before the merge), it works fine, but can break if the netcoreapp version doesn't match the runtime version.

@chsienki
Copy link
Author

@dotnet-bot test OSX10.12 x64 Checked CoreFX Tests please

@4creators
Copy link

4creators commented Jul 26, 2018

Actually I have hit the problem, however, it seems to be part of larger test dependency graph inconsistencies which result in a bunch of warnings. I am working on fixing in #19109 PR. However, this change does not affect my work.

Copy link

@4creators 4creators left a comment

Choose a reason for hiding this comment

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

LGTM modulo work in #19109

@4creators
Copy link

@dotnet-bot test OSX10.12 x64 Checked CoreFX Tests please

@@ -7,7 +7,7 @@

<PropertyGroup>
<TargetFramework>netcoreapp3.0</TargetFramework>
<RuntimeFrameworkVersion>$(MicrosoftNETCoreRuntimeCoreCLRPackageVersion)</RuntimeFrameworkVersion>
<RuntimeFrameworkVersion>$(MicrosoftNETCorePlatformsPackageVersion)</RuntimeFrameworkVersion>
Copy link
Member

Choose a reason for hiding this comment

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

You should know that this doesn't map to the version of Microsoft.NETCore.App if that is what you are trying to get here. It does it a lot of cases but not in all cases.

Copy link
Author

Choose a reason for hiding this comment

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

Oh, interesting. Is there a better value to use?

Copy link
Member

Choose a reason for hiding this comment

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

It doesn't look like you guys consume Microsoft.NETCore.App directly in coreclr. If it is a true dependency then you guys should start consuming it like corefx does https://github.com/dotnet/corefx/blob/master/dependencies.props#L42.

jashook pushed a commit to jashook/coreclr that referenced this pull request Aug 14, 2018
…eFrameworkVersion parameter in the test wrappers and SDK props file (dotnet#19130)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants