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

[release/2.1] 2.1.1 fixes, combined #610

Merged
merged 4 commits into from
Jun 14, 2018
Merged

Conversation

dagood
Copy link
Member

@dagood dagood commented Jun 13, 2018

Rolls up #598 (PR) plus(merged) fixes for #608, #609, #586.

@dagood dagood requested review from dseefeld and crummel June 13, 2018 15:10
<!-- Placeholder for setting latest patch version using the bundled version from CLI for previous TFMs, this will not apply until 2.2 -->
- <!-- <LatestAspNetCoreAllPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '2.1'">$(LatestPatchVersionForAspNetCoreAll2_1)</LatestAspNetCoreAllPatchVersion> -->
+ <LatestAspNetCoreAllPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '2.1'">$(LatestPatchVersionForAspNetCoreAll2_1)</LatestAspNetCoreAllPatchVersion>
+ <LatestAspNetCoreAllPatchVersion Condition="'$(_TargetFrameworkVersionWithoutV)' == '2.2'">$(LatestPatchVersionForAspNetCoreAll2_2)</LatestAspNetCoreAllPatchVersion>
Copy link
Member Author

Choose a reason for hiding this comment

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

This patch isn't fully working yet (see #586 (comment)) so I'm going to go ahead and merge #598 since its CI finished and rebase this PR with the fix.

@dagood
Copy link
Member Author

dagood commented Jun 13, 2018

There was a new 2.1.1 build (published to the 2.1 channel) that I've added a commit to uptake. I hear it's branding fixes for sdk and websdk.

@crummel
Copy link
Contributor

crummel commented Jun 13, 2018

I'm not sure yet why we weren't seeing the branding issue; from the changes I think we should have been and I'm continuing to look at it. I think we also need to add <EnvironmentVariables Include="PB_IsStable=$(UseStableVersions)" Condition="'$(UseStableVersions)' != ''" /> here to be completely correct because roslyn-tools-based repos use this variable directly.

@crummel
Copy link
Contributor

crummel commented Jun 13, 2018

Ah, it appears the packages that are produced with the incorrect versions are unpacked and the contents included, so we never see the incorrect version. For the roslyn-tools issue, I think the only place we would actually produce bad packages would be MSBuild, where we're manually setting the version anyway - so this change shouldn't be necessary for 2.1.1, although I think it would be good long-term.

RoslynTools-using repos look for PB_IsStable.
@dagood
Copy link
Member Author

dagood commented Jun 13, 2018

If CI infra ends up too flaky and we want to go ahead with 2.1.1 without the last two commits, dabf6ed passed CI except an infra snag with CentOS7.1 Release. I kicked off a build to run the leg on that commit (edit: which has now succeeded). (It didn't update the CI status indicators because it's a manual build, unfortunately.)

@crummel
Copy link
Contributor

crummel commented Jun 13, 2018

@dotnet-bot test OSX10.12 Release please (java.nio.channels.ClosedChannelException)

@crummel
Copy link
Contributor

crummel commented Jun 13, 2018

This did get the versioning updates (that we didn't really need):

Successfully created package '/mnt/resource/j/workspace/dotnet_source-build/release_2.1/CentOS7.1_Release_prtest/src/websdk/bin/Release/Microsoft.NET.Sdk.Web.2.1.301.nupkg'.
Successfully created package '/mnt/resource/j/workspace/dotnet_source-build/release_2.1/CentOS7.1_Release_prtest/src/sdk/artifacts/Release/packages/Microsoft.NET.Sdk.2.1.301.nupkg'.

@dagood
Copy link
Member Author

dagood commented Jun 13, 2018

@dotnet-bot test OSX10.12 Release (java.nio.channels.ClosedChannelException)

@crummel
Copy link
Contributor

crummel commented Jun 14, 2018

@dotnet-bot test OSX10.12 Release please (looks like a timeout, but didn't take long enough for one. no other error message. machine lost without Jenkins realizing it?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants