Skip to content

Upgrade global.json to 5.0-alpha1 SDK - #445

Merged
ViktorHofer merged 10 commits into
masterfrom
ViktorHofer-5.0-sdk
Dec 5, 2019
Merged

Upgrade global.json to 5.0-alpha1 SDK#445
ViktorHofer merged 10 commits into
masterfrom
ViktorHofer-5.0-sdk

Conversation

@ViktorHofer

@ViktorHoferViktorHofer commented Dec 2, 2019

Copy link
Copy Markdown
Member

We need the 5.0 SDK to use new VSTest features and we should start using it anyway. Opening this PR to test what will eventually break.

@ViktorHoferViktorHofer added NO-REVIEW Experimental/testing PR, do NOT review it area-Infrastructure labels Dec 2, 2019
@ViktorHoferViktorHofer self-assigned this Dec 2, 2019
Comment threadDirectory.Build.props Outdated
@ViktorHofer

Copy link
Copy Markdown
MemberAuthor

The Rhel6 errors are because of dotnet/installer#5539 not being merged into master yet where a rhel.6 removal commit was reverted.

@ericstj

Copy link
Copy Markdown
Member

I'm guessing all-configurations leg is failing because new SDK might be missing the known frameworks for past netcoreapps? That's where I would reccomend investigating.

@ViktorHofer

Copy link
Copy Markdown
MemberAuthor

/azp run runtime

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@ViktorHofer

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-libraries

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@ViktorHofer

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-coreclr

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@ViktorHofer

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-installer

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@ViktorHoferViktorHofer changed the title [WIP] Upgrade global.json to 5.0-alpha1 SDKUpgrade global.json to 5.0-alpha1 SDKDec 5, 2019
@ViktorHoferViktorHofer removed the NO-REVIEW Experimental/testing PR, do NOT review it label Dec 5, 2019
@ViktorHofer

Copy link
Copy Markdown
MemberAuthor

This is ready, pls can somebody approve.

@ericstjericstj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor comment you can merge if green

-->
<Target Name="_SetRuntimeFrameworksForTestAssemblies"
Condition="'$(SelfContained)' != 'true'"
Condition="'$(SelfContained)' != 'true' and '$(MSBuildProjectExtension)' != '.depproj'"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

</ItemGroup>

<!-- Mark framework references from previous netcoreapp tfms >= 3.0 as copy-local. -->
<Target Name="_FrameworkReferencesToPrivate"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Instead I’d just do this for all items in depproj targets.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Can you elaborate? Not sure I follow. Do you mean to actually Define an ItemDefintionGroup for reference?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

If you mean you would do it for all targets, I would rather not as we include netstandard2.0 here as well which just creates noise in the ref folder, ie dotnet-XmlGenerator.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I meant that the purpose of DepProj is to copy. It flows these items to content here:

<ContentWithTargetPathInclude="@(NuGetDeploy)">

This is effectively doing the same thing you're trying to do (force to copy). If that wasn't working for framework references we should fix it there.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Actually I investigated why this wasn't working with the depproj/binplacing logic. I will submit a follow-up PR when I have some cycles to fix it there.

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@ViktorHofer@ericstj@davidsh