Skip to content

Stabilize CLI. #565

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

Merged
merged 13 commits into from
May 21, 2018
Merged

Stabilize CLI. #565

merged 13 commits into from
May 21, 2018

Conversation

crummel
Copy link
Contributor

@crummel crummel commented May 19, 2018

No description provided.

@crummel crummel requested review from dseefeld, dleeapho and dagood May 19, 2018 01:18
@dagood
Copy link
Member

dagood commented May 19, 2018

Looks like without the prerelease spec on MSBuild, we're picking up the Msft-build 15.6.82 MSBuild package in the online build. Not sure how that affects crossgen. Pushing a few commits: updating to 15.7.179 in case the newer version's layout or dependencies are ok in the online build, and to 15.7.999 to force a mismatch and use the source-built package.

Ideally we'd be synced to ProdCon, but we're on a custom branch, and MSBuild isn't even in ProdCon anymore: dotnet/cli#9262.

@crummel
Copy link
Contributor Author

crummel commented May 21, 2018

@nkolev92 Does removing the NuGet version for a stable source-build version make sense?

@dagood
Copy link
Member

dagood commented May 21, 2018

Seems weird that we're hitting this but ProdCon builds apparently didn't... the version of NuGet packages used for the current 2.1-rtm Msft CLI build is 4.7.0-rtm.5148.

@nkolev92
Copy link
Contributor

@crummel If you leave it out, it will generate a random high number.

Why do you need to remove it? Is it causing any issues?

//cc @rrelyea @rohit21agrawal @mishra14

@crummel
Copy link
Contributor Author

crummel commented May 21, 2018

I'm talking to Matt Mitchell now about it - looks like this is actually a CLI issue that was fixed. Going to revert to NuGet change and update CLI to see if that works.

@dseefeld
Copy link
Contributor

Here's a summary of the issue:
What is the issue? Source-build is building the SDK with a prerelease version.

  • CLI had made changes to allow ProdCon to pass a variable to make CLI stable (DropSuffix=true)
  • Source-build added DropSuffix=true to CLI project to stabilize versions.
  • This caused a warning (treated as an error) on prerelease version dependency for CLI’s dependency on msbuild.
  • Source-build added stabilization to msbuild by removing the rc prerelease label.
  • This uncovered a new error with Nuget product packages which were also prerelease versions, although Nuget ships prerelease versions with VS and CLI. Because they ship prerelease versions, source-build cannot make a stable version to fix the issue.
  • Why is this warning treated as an error in source-build, but not in ProdCon? This is still a mystery. Investigation is ongoing.
  • PR currently has code to ignore NU5104 – prerelease version dependency warning.

@dagood
Copy link
Member

dagood commented May 21, 2018

On a hunch that the CLI tooling version that ProdCon uses (2.2.0-preview1-007799) has a functionality change from the one source-build uses (2.1.300-rc1-008673), I ran a build with a few changes:

  • Remove the NoWarn workaround.
  • Remove --stage0 $(DotNetCliToolDir) from the cli.proj command line args. This lets CLI download/use the 2.2 CLI version it normally uses rather than forcing it to use the 2.1 one we provide.

This gives me the behavior we're seeing in ProdCon:

> [DotNetTool] pack    /home/dagood/sb/stableCli-dagood/git/src/cli/src/Microsoft.DotNet.Cli.Utils/Microsoft.DotNet.Cli.Utils.csproj --configuration Debug --no-build --output /home/dagood/sb/stableCli-dagood/git/src/cli/bin/2/fedora.28-x64/packages
> /home/dagood/sb/stableCli-dagood/git/src/cli/.dotnet_stage0/x64/dotnet pack    /home/dagood/sb/stableCli-dagood/git/src/cli/src/Microsoft.DotNet.Cli.Utils/Microsoft.DotNet.Cli.Utils.csproj --configuration Debug --no-build --output /home/dagood/sb/stableCli-dagood/git/src/cli/bin/2/fedora.28-x64/packages
Microsoft (R) Build Engine version 15.6.12.27473 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  Successfully created package '/home/dagood/sb/stableCli-dagood/git/src/cli/bin/2/fedora.28-x64/packages/Microsoft.DotNet.Cli.Utils.2.1.300.nupkg'.
... warning : Issue found with package 'Microsoft.DotNet.Cli.Utils'. ...
...
   "/home/dagood/sb/stableCli-dagood/git/src/cli/build.proj" (default target) (1) ->
   (GenerateNugetPackages target) ->
     ... warning : Issue found with package 'Microsoft.DotNet.Cli.Utils'. ...
     ... warning : Issue: Prerelease dependency in stable package. ...
     ... warning : Description: A stable release of a package should not have a prerelease dependency. ...
     ... warning : Solution: Either modify the version spec of dependency "NuGet.Frameworks [4.7.0-rtm.5156, )" or update the version field in the nuspec. ...

6 Warning(s)
0 Error(s)

We can try a more specific repro later to narrow down the change, but for now I'm satisfied with the workaround and pointing at this CLI/NuGet behavior change as the reason we need it.

@crummel crummel merged commit c7012bc into dotnet:dev/release/2.1 May 21, 2018
dseefeld pushed a commit that referenced this pull request Jun 5, 2018
* Stabilize CLI. (#565)

* Stabilize CLI.
* Use stable MSBuild.
* Fix MSBuild version to match prodcon build.
* Modify CLI build file to always ignore NU5104

* Add web to smoke-test, with and without dev certs (#539)

* Add web to smoke-test, with and without dev certs

* Only reset caches when a test will run

Avoid conflicting with a hanging process if there's no need to delete the files anyway.

* Fix some minor issues with the bootstrap documentation.

* Update CoreClr, ProdCon to rtm-26515-03, 20180516-07-1693122, respectively (#553)

* Only set DropSuffix to true when UseStableVersions is true

* Update dotnet-dev-certs version

This version needs to be compatible with the current runtime being produced.
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.

4 participants