Add buildtools to CoreFXLab. #1095

crummel · 2017-01-04T22:01:26Z

Build stuff is mostly copied from CoreFX.
We retain the ability to build the old way for now.
.xprojs are replaced by .csprojs.
Product changes were bruteforced to make it build, they definitely require review.

dnfclas · 2017-01-04T22:01:32Z

Hi @crummel, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

crummel · 2017-01-04T22:14:13Z

I'm going to clean up the .csprojs a bit, they were copied from CoreFX as well and I don't think everything in them applies to CoreFXLab.
Source changes and other concerns:

Adding netcoreapp1.0 as a target framework was needed to get several of the projects to compile. I don't know why this is. Is it a problem?
System.IO.Pipelines.File gave me "System.Threading.Overlapped 4.3.0 is not compatible with net451" so I upgraded it to net46, which cascaded back through the System.IO.Pipelines projects. Is this a real problem or is something wrong with how I'm building it?
Buildtools includes analyzers that catch some DLL imports that aren't cross-platform. I've disabled these in those source files, but we could also turn the analyzers off globally.
Some projects needed more dependencies declared. I'm not sure why this is.
Tests are not yet converted.
Would someone mind trying the debugging scenario that we were concerned this would break?
@joshfree @KrzysztofCwalina @shiftylogic @ahsonkhan

davidfowl · 2017-01-05T01:42:03Z

@crummel are these 2017 enabled? I'm not a fan of using completely custom everything.

crummel · 2017-01-05T20:01:08Z

@davidfowl I haven't tried with 2017, I'll spin up a machine and see if it works. Is your concern just using the projects in VS? That should work, although we weren't sure if we needed to extra work to support debugging.

davidfowl · 2017-01-05T21:48:32Z

@davidfowl I haven't tried with 2017, I'll spin up a machine and see if it works. Is your concern just using the projects in VS? That should work, although we weren't sure if we needed to extra work to support debugging.

More concerned that we're not dogfooding what people are using in real life. CoreFx and CoreCLR builds are very special compared to corefxlab. These are regular class libraries that should be using the regular .NET Core tooling that we're shipping with VS2017.

joshfree · 2017-01-05T23:22:59Z

Thanks for bringing the latest build tools to corefxlab @crummel

@KrzysztofCwalina any concerns before this gets merged in?

KrzysztofCwalina · 2017-01-05T23:35:49Z

@joshfree, I would like @crummel to verify that VS 2017 is working, including debugging. I thought we agreed that this is pri 0 requirement.

crummel · 2017-01-05T23:39:13Z

@joshfree @KrzysztofCwalina I do get a build error in 2017, I'm looking into it.

davidfowl · 2017-01-09T04:50:25Z

dependencies.props

@@ -0,0 +1,138 @@
+<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">


What is this?

davidfowl · 2017-01-09T04:51:20Z

src/System.Binary.Base64/System.Binary.Base64.csproj

@@ -0,0 +1,65 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">


These files should be using the new SDK syntax.

davidfowl · 2017-01-09T04:51:57Z

src/System.Binary.Base64/System.Binary.Base64.csproj

+<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+  <PropertyGroup>
+    <ProjectGuid>b9ecbaf7-524d-4134-ae37-eff7802b221f</ProjectGuid>


All of this needs to be deleted. These projects should look like this https://github.com/aspnet/Security/blob/dev/src/Microsoft.AspNetCore.Authentication.Cookies/Microsoft.AspNetCore.Authentication.Cookies.csproj

You will need to pull in a newer CLI in order to use the new SDK syntax. I think CoreFxLab would be a great candidate for dogfooding that build portion of the CLI, that is a little trickier to do in CoreFx right now.

weshaggard · 2017-01-09T18:51:08Z

init-tools.cmd

@@ -0,0 +1,83 @@
+@if not defined _echo @echo off


Why aren't you using the bootstraping scripts?

joshfree · 2017-01-10T18:15:53Z

I do get a build error in 2017, I'm looking into it.

@crummel how is this going?

crummel · 2017-01-10T19:59:07Z

@joshfree I can push out a new version that's just the SDK 15.0 changes, but I'm still getting errors with adding buildtools to that. If we just want the project changes and come back later to actually add buildtools to the projects I can get an updated PR out today.

joshfree · 2017-01-10T20:16:29Z

If you want to break up the work into multiple PRs that's fine - so long as VS use isn't regressed in the repo.

- Build stuff is mostly copied from CoreFX. - We retain the ability to build the old way for now. - .xprojs are replaced by .csprojs. - Product changes were bruteforced to make it build, they definitely require review.

crummel · 2017-01-17T18:11:33Z

@davidfowl @joshfree @weshaggard I've updated to use the latest project syntax and confirmed debugging in VS2017 works. I'll do the bootstrap change separately.

joshfree · 2017-01-17T18:24:27Z

I've updated to use the latest project syntax and confirmed debugging in VS2017 works. I'll do the bootstrap change separately.

OK sounds good. Thanks @crummel

KrzysztofCwalina · 2017-01-19T02:49:02Z

Sounds good. Let's merge the change once if passes the tests.

KrzysztofCwalina · 2017-01-19T02:49:14Z

@dotnet-bot test this please

This reverts commit 33548e9. Additional build errors showing up in VS vs MSBuild for some reason. Reverting for now and I'll fix them.

- Build stuff is mostly copied from CoreFX. - We retain the ability to build the old way for now. - .xprojs are replaced by .csprojs. - Product changes were bruteforced to make it build, they definitely require review.

This reverts commit 33548e9. Additional build errors showing up in VS vs MSBuild for some reason. Reverting for now and I'll fix them.

dnfclas added the cla-already-signed label Jan 4, 2017

KrzysztofCwalina requested review from joshfree and davidfowl January 4, 2017 22:13

davidfowl reviewed Jan 9, 2017

View reviewed changes

weshaggard reviewed Jan 9, 2017

View reviewed changes

init-tools.cmd

@@ -0,0 +1,83 @@

@if not defined _echo @echo off

Copy link

Member

weshaggard Jan 9, 2017

Choose a reason for hiding this comment

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

Why aren't you using the bootstraping scripts?

Add buildtools to CoreFXLab.

b9db74a

- Build stuff is mostly copied from CoreFX. - We retain the ability to build the old way for now. - .xprojs are replaced by .csprojs. - Product changes were bruteforced to make it build, they definitely require review.

crummel force-pushed the buildtools branch from ef6a004 to b9db74a Compare January 17, 2017 17:44

KrzysztofCwalina merged commit 33548e9 into dotnet:master Jan 19, 2017

KrzysztofCwalina pushed a commit that referenced this pull request Jan 20, 2017

Revert "Add buildtools to CoreFXLab. (#1095)" (#1138)

f2693ff

This reverts commit 33548e9. Additional build errors showing up in VS vs MSBuild for some reason. Reverting for now and I'll fix them.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add buildtools to CoreFXLab. #1095

Add buildtools to CoreFXLab. #1095

crummel commented Jan 4, 2017

dnfclas commented Jan 4, 2017

crummel commented Jan 4, 2017

davidfowl commented Jan 5, 2017

crummel commented Jan 5, 2017

davidfowl commented Jan 5, 2017

joshfree commented Jan 5, 2017

KrzysztofCwalina commented Jan 5, 2017

crummel commented Jan 5, 2017

davidfowl Jan 9, 2017

davidfowl Jan 9, 2017

davidfowl Jan 9, 2017

weshaggard Jan 9, 2017 •

edited

davidfowl Jan 10, 2017

weshaggard Jan 9, 2017

joshfree commented Jan 10, 2017

crummel commented Jan 10, 2017

joshfree commented Jan 10, 2017

crummel commented Jan 17, 2017

joshfree commented Jan 17, 2017

KrzysztofCwalina commented Jan 19, 2017

KrzysztofCwalina commented Jan 19, 2017

		@@ -0,0 +1,138 @@
		<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

		@@ -0,0 +1,65 @@
		<?xml version="1.0" encoding="utf-8"?>
		<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

Add buildtools to CoreFXLab. #1095

Add buildtools to CoreFXLab. #1095

Conversation

crummel commented Jan 4, 2017

dnfclas commented Jan 4, 2017

crummel commented Jan 4, 2017

davidfowl commented Jan 5, 2017

crummel commented Jan 5, 2017

davidfowl commented Jan 5, 2017

joshfree commented Jan 5, 2017

KrzysztofCwalina commented Jan 5, 2017

crummel commented Jan 5, 2017

davidfowl Jan 9, 2017

Choose a reason for hiding this comment

davidfowl Jan 9, 2017

Choose a reason for hiding this comment

davidfowl Jan 9, 2017

Choose a reason for hiding this comment

weshaggard Jan 9, 2017 • edited

Choose a reason for hiding this comment

davidfowl Jan 10, 2017

Choose a reason for hiding this comment

weshaggard Jan 9, 2017

Choose a reason for hiding this comment

joshfree commented Jan 10, 2017

crummel commented Jan 10, 2017

joshfree commented Jan 10, 2017

crummel commented Jan 17, 2017

joshfree commented Jan 17, 2017

KrzysztofCwalina commented Jan 19, 2017

KrzysztofCwalina commented Jan 19, 2017

weshaggard Jan 9, 2017 •

edited