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

Uptake the latest buildtools package, and new CLI #1294

Merged
merged 3 commits into from
Jan 10, 2017

Conversation

dagood
Copy link
Member

@dagood dagood commented Jan 10, 2017

The buildtools package uses shared framework 1.1.0 instead of 1.0.0, so switch to a matching CLI package. This is the same CLI version as CoreFX and CoreCLR use.

See #1286

Known issues: there are binclashes, and a test failure. I'm submitting this PR as-is as a starting point for making this work. Hopefully anyone who needs to do this upgrade will see this PR first and avoid wasted effort. fixed

The buildtools package uses shared framework 1.1.0 instead of 1.0.0, so switch to a matching CLI package. This is the same CLI version as CoreFX and CoreCLR use.
dagood added 2 commits January 9, 2017 19:32
Now more is expected in TestPath that determines the test root. TFM is not added by testing targets anymore. See CoreFX PR 12634.
Without the entry in defaultValues, no ConfigurationGroup was passed to the msbuild command. This caused bin clashes due to a projects not having ConfigurationGroup passed, but the ProjectReferences did have it set due to dir.props.
@dagood
Copy link
Member Author

dagood commented Jan 10, 2017

I couldn't track down what buildtools change started the assumption that ConfigurationGroup is always set as a msbuild property arg, but I was able to figure out what was different between buildtools and corefx. corefx's config.json explicitly calls out ConfigurationGroup in the defaultValues block. This not being set in buildtools' config.json and therefore being left up to dir.props caused binclashes because msbuild thought the projectreference wasn't the same configuration as the direct project build, so they would both build and clash.

@dagood dagood requested review from crummel and naamunds January 10, 2017 16:50
@dagood dagood self-assigned this Jan 10, 2017
Copy link

@crummel crummel left a comment

Choose a reason for hiding this comment

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

LGTM.

@dotnet-bot
Copy link

CROSS Risk Report

Generated: 1/10/2017 10:30:19 AM

High Risk Files

File Score Risk
run.ps1 10 Scripting languages

Risk Score Totals

Total score for high risk files: 10
Total score for all files in PR: 10

Total execution time: 1.59 seconds

@dagood dagood merged commit db0ac3f into dotnet:master Jan 10, 2017
@dagood dagood deleted the upgrade-cli branch January 10, 2017 18:35
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