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

Pick up new CoreFX packages and abandon old ones #10074

Merged
merged 1 commit into from
Mar 14, 2017

Conversation

wtgodbe
Copy link
Member

@wtgodbe wtgodbe commented Mar 10, 2017

This change does a few things:

  • Separates our external dependencies into external/project.json
  • Changes all dependencies on System.xyz to Microsoft.Private.CoreFx.NETCoreApp
  • Removes several JIT/config project.json files (Everything except benchmark, benchmark+roslyn, benchmark+serialize), and re-assigns all test projects who previously depended on those, to depend on test-dependencies/project.json
  • Adds external.depproj, which handles restoring dependencies from external/project.json into a Targeting Pack folder, which tests reference directly.
  • Moves from netcoreapp1.1 to netcoreapp2.0, where possible.

There's a lot of changes here, but the relevant files should be at the top of the changeset - everything else is either a deletion of a project.json file, or a deletion of a Property referencing one of those files. (Or @AndyAyersMS fixing a bad ReferencePath in an .ilproj).

I did not touch any of the Perf stuff, as those tests still depend on using NetStandard1.4. Once that changes, we should be able to get rid of those project.json's as well, and point perf tests at test-dependencies/project.json.

@wtgodbe
Copy link
Member Author

wtgodbe commented Mar 10, 2017

@wtgodbe wtgodbe force-pushed the ProjectJson branch 4 times, most recently from 326a0da to 0b55bfd Compare March 10, 2017 00:08
@wtgodbe
Copy link
Member Author

wtgodbe commented Mar 10, 2017

CC @pgavlin as well, for the JIT stuff

@gkhanna79
Copy link
Member

Is the Pri1 test build and run clean with it locally and (clean run) in Helix?

@wtgodbe
Copy link
Member Author

wtgodbe commented Mar 10, 2017

@gkhanna79 I sent a few jobs to helix that came back clean, but haven't done a full pri-1 job yet - I'm setting that up now

"Microsoft.DotNet.xunit.performance.analysis": "1.0.0-alpha-build0040",
"Microsoft.DotNet.xunit.performance.runner.Windows": "1.0.0-alpha-build0040",
"Microsoft.Win32.Primitives": "4.4.0-beta-24913-02",
"Newtonsoft.Json": "8.0.3",
"Microsoft.NETCore.Platforms": "2.0.0-beta-25109-03",
"Microsoft.NETCore.Targets": "1.2.0-beta-24913-02",
Copy link
Member

Choose a reason for hiding this comment

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

This package is not needed.

"imports": [
"dnxcore50",
"netcoreapp1.1",
Copy link
Member

Choose a reason for hiding this comment

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

This is not needed.

@ericstj
Copy link
Member

ericstj commented Mar 13, 2017

You may need some additional package refs. We still produce packages for those listed here: https://github.com/dotnet/versions/blob/master/build-info/dotnet/corefx/master/Latest_Packages.txt. Some of those overlap with NETCore.App but there is more than just S.R.CS.Unsafe that does not.

@mellinoe
Copy link

LGTM. We can add the extra package references now, or later when folks start wanting to use them. As long as the current suite of tests is satisfied then that seems ok to me.

@wtgodbe
Copy link
Member Author

wtgodbe commented Mar 13, 2017

@gkhanna79 @weshaggard @ericstj has the UnicodeEncoding class changed recently? Looks like https://github.com/dotnet/coreclr/blob/master/tests/src/CoreMangLib/cti/system/text/unicodeencoding/unicodeencodinggetbytecount1.cs#L192 may need to change, based on the current errors.

Also, with the changes from #10150, tests are passing in Helix.

@wtgodbe
Copy link
Member Author

wtgodbe commented Mar 13, 2017

@dotnet-bot test OSX x64 Checked Build and Test

@jkotas
Copy link
Member

jkotas commented Mar 13, 2017

I have changed UnicodeEncoding in #10124 . The CI was green with my change. Is this running more tests?

@jkotas
Copy link
Member

jkotas commented Mar 13, 2017

https://github.com/dotnet/coreclr/blob/master/tests/src/CoreMangLib/cti/system/text/unicodeencoding/unicodeencodinggetbytecount1.cs#L192

Yes, it needs to change. It is build break caused by adding a new API. It is expected - it was discussed during the review and determined to be ok.

@wtgodbe
Copy link
Member Author

wtgodbe commented Mar 13, 2017

@jkotas what should it be changed to?

@jkotas
Copy link
Member

jkotas commented Mar 13, 2017

(char[])null, 0, 0. Or delete the test - there is redundant test in corefx with much more comprehensive coverage.

@wtgodbe
Copy link
Member Author

wtgodbe commented Mar 13, 2017

I'll just insert the fix for now.

@AndyAyersMS
Copy link
Member

Fixes for the odd path corruptions I introduced look good (at least a sampling of them). Thanks for fixing this.

@wtgodbe wtgodbe merged commit 70bbb20 into dotnet:master Mar 14, 2017
@wtgodbe wtgodbe deleted the ProjectJson branch March 14, 2017 01:26
@karelz karelz modified the milestone: 2.0.0 Aug 28, 2017
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.

8 participants