Add roslyn repo to known-good by dseefeld · Pull Request #404 · dotnet/source-build

dseefeld · 2018-04-09T16:12:24Z

Roslyn repo has been updated to incorporate previous patches into source. Changes are on a private branch for now.
Added updaters for global.json and Tools.props to update CLI version.
Added a patch to include solution to build, since solution included in roslyn branch is currently incorrect.
Added patch for SDK PR1997 which is required until source-build moves to a newer CLI version than 2.1.101

…d NuSpec.

dagood · 2018-04-09T16:19:46Z

patches/0001-Patch-Tools-for-sdk-pr-1997.patch

@@ -0,0 +1,25 @@
+From 735b7c82a16ddf16f39643830acb2ee62d2f3150 Mon Sep 17 00:00:00 2001


I think this patch should live in tools-local/patches or something like that--to clarify what kind of patch this is and keep repo patches separate.

Yes. I moved it.

dagood · 2018-04-09T16:22:08Z

init-tools.sh

 "$__scriptpath/Tools/crossgen.sh" "$__scriptpath/Tools"

+echo "Patching Sdk to workaround issue https://github.com/dotnet/sdk/pull/1997"
+git apply --ignore-whitespace --whitespace=nowarn --directory="$__scriptpath" "$__scriptpath/patches/0001-Patch-Tools-for-sdk-pr-1997.patch"


Is it reasonable to do this during tools-local/init-build.proj to avoid customizing this script? (With some semaphore in the Tools dir to avoid attempting it more than once?)

@dagood and I discussed offline. I will leave it in the init-tools.sh script since 1) it is very temporary and will go away on the next update of CLI version and 2) it can more easily participate in the Tools setup semaphore.

dagood · 2018-04-09T16:24:50Z

repos/roslyn.proj

-    <NuSpecFiles Include="$(ProjectDirectory)/src/NuGet/Microsoft.CodeAnalysis.Build.Tasks.nuspec" />
  </ItemGroup>

+  <Target Name="UpdateCLIVersion"


Could this be in dir.targets with the json and props paths parameterized? (But we can put the move off until we use it to remove patches in other repos too.)

I'd rather leave this here for now and move it when we need it for others.

jaredpar · 2018-04-09T16:58:58Z

repos/roslyn.proj

+
+    <UpdateXml  XmlFilePath="$(ProjectDirectory)/build/Targets/Tools.props"
+                PathToElement="Project/PropertyGroup/dotnetSdkVersion"
+                NewElementValue="$(SDK_VERSION)" />


This needs to be removed once we merge the private branch as we jus condition away this check altogether.

This can just be removed in this PR as it's based on the dev/sourcebuild branch.

In reply to: 180162495 [](ancestors = 180162495)

Removed. Thanks.

crummel · 2018-04-09T16:55:08Z

init-tools.sh

 "$__scriptpath/Tools/crossgen.sh" "$__scriptpath/Tools"

+echo "Patching Sdk to workaround issue https://github.com/dotnet/sdk/pull/1997"
+git apply --ignore-whitespace --whitespace=nowarn --directory="$__scriptpath" "$__scriptpath/patches/0001-Patch-Tools-for-sdk-pr-1997.patch"


--directory shouldn't be needed here, we need it for applying patches to repos because they were generated with the repo directory as root. It doesn't hurt anything but I think it makes it less clear here.

In theory the working dir might not be $__scriptpath at this point though, and --directory="$__scriptpath" adds support for that, right?

With help from @dagood , found that this was the issue with the git apply in CI. Removed the --directory option.

333fred · 2018-04-09T19:38:34Z

patches/roslyn/0002-Add-solution-for-source-build.patch

@@ -0,0 +1,147 @@
+From c7dbba01272248423989a1816625dccb3e129b74 Mon Sep 17 00:00:00 2001


This patch is unnecessary, this commit of roslyn has Sourcebuild.sln.

Updated the sha to the commit that fixes SourceBuild.sln.

333fred · 2018-04-09T19:40:20Z

We will need to update the PR to pull in roslyn from dev15.7.x instead of dev/sourcebuild as we get that branch merged in.

remove temporary sln. Remove xml updater since it isn't needed. Move patch to tools-local dir and update git apply to not use --directory.

…ft.json versions. Adding dotnet#409 to re-add this later.

…ce-build into updateRoslynFor2.1

dseefeld added 7 commits April 6, 2018 21:30

Move roslyn repo to branch without patches

ec6f3d3

Patch tools for SDK issue: dotnet/sdk#1997

75d5b62

Add updaters to update global.json and Tools.props

5237e13

Change UpdateXml to write output properly

1d69556

Add patches for roslyn

397d3b8

Update roslyn.proj to build the source-build solution. Remove unneede…

6195a38

…d NuSpec.

Add roslyn to known-good.proj

bac9af8

dseefeld requested review from crummel and dagood April 9, 2018 16:12

dagood reviewed Apr 9, 2018

View reviewed changes

jaredpar reviewed Apr 9, 2018

View reviewed changes

crummel reviewed Apr 9, 2018

View reviewed changes

333fred reviewed Apr 9, 2018

View reviewed changes

dseefeld and others added 2 commits April 9, 2018 20:24

Move roslyn sha to commit that corrects issue with SourceBuild.sln and

439a1ab

remove temporary sln. Remove xml updater since it isn't needed. Move patch to tools-local dir and update git apply to not use --directory.

Merge branch 'dev/release/2.1' into updateRoslynFor2.1

9918a03

dagood approved these changes Apr 9, 2018

View reviewed changes

dseefeld added 2 commits April 10, 2018 12:49

Remove UpdateJson task because it is causing a conflict with newtonso…

e0039e8

…ft.json versions. Adding dotnet#409 to re-add this later.

Merge branch 'updateRoslynFor2.1' of https://github.com/dseefeld/sour…

1ffb505

…ce-build into updateRoslynFor2.1

dseefeld merged commit d777c06 into dotnet:dev/release/2.1 Apr 10, 2018

dseefeld deleted the updateRoslynFor2.1 branch April 10, 2018 15:06

		@@ -0,0 +1,25 @@
		From 735b7c82a16ddf16f39643830acb2ee62d2f3150 Mon Sep 17 00:00:00 2001

		@@ -0,0 +1,147 @@
		From c7dbba01272248423989a1816625dccb3e129b74 Mon Sep 17 00:00:00 2001

Conversation

dseefeld commented Apr 9, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

333fred commented Apr 9, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants