Update RepoToolset to 1.0.0-beta2-62804-01#2113
Conversation
|
@nguerrera @dsplaisted PTAL |
tmat
force-pushed
the
RT1.0.0-beta2-62804-01
branch
2 times, most recently
from
26ae576 to
90afb93
Compare
|
test Ubuntu16.04 Debug please |
|
LGTM @livarcocc I think this qualifies as infrastructure and therefore not subject to ask mode. I'm nevertheless a bit concerned about the churn while 2.1.300 P2 is stabilizing. I guess I don't understand why this has to happen in release/2.1.3xx and not master. |
Either that or ask for delay until P2 is out the door. |
|
I approve of the actual file changes, but I'll let @livarcocc handle whether this can go in to this branch now. |
|
I agree with @nguerrera, it would be better to wait for the prodcon build to be done before merging this change. |
|
+@dleeapho too For practical purposes, we can apply this PR as a set of patches in source-build for preview2. We were hoping to be able to build on the prodcon-built commits without patches wherever possible, though. I don't know how that goal interacts with escrow. |
|
I'll defer to @leecow, but I'm okay with this change, as we are already past sdk in the build and can freeze if necessary. |
|
test Ubuntu16.04 Debug please |
|
test Ubuntu16.04 Release please |
| </PropertyGroup> | ||
|
|
||
| <PropertyGroup> | ||
| <RestoreSources> |
There was a problem hiding this comment.
Do we need to condition these for offline build?
There was a problem hiding this comment.
No, these are overridden by the toolset when building offline. https://github.com/dotnet/roslyn-tools/blob/master/sdks/RepoToolset/tools/DefaultVersions.props#L63
tmat
force-pushed
the
RT1.0.0-beta2-62804-01
branch
from
3185f11 to
dd10bc7
Compare
|
@mmitche Is this a good time to merge this? |
| @@ -0,0 +1,11 @@ | |||
| { | |||
| "sdk": { | |||
| "version": "2.1.300-preview3-008414" | |||
There was a problem hiding this comment.
To avoid a patch, this should be 2.1.300-preview2-008533 to match the preview2 SDK we use in source-build.
There was a problem hiding this comment.
Isn't release/2.1.3xx-src used for source build currently?
There was a problem hiding this comment.
That's the branch used, not sure how that relates to the SDK version though. If you're saying that we could rebase release/2.1.3xx-src on top of this change to make this modification, it's true that wouldn't be a literal patch, but dev branches aren't any better for maintainability than patches.
The SDK we have to use to source-build RC is the one we delivered as 2.1-preview2, which is 2.1.300-preview2-008533 (matching the version we shipped from prodcon).
There was a problem hiding this comment.
Actually, soon we'll have a global.json updater that would replace a patch, so it's probably fine to patch/branch for this change temporarily. LGTM as is.
There was a problem hiding this comment.
I'm saying that we should merge release/2.1.3xx-src into release/2.1.3xx if it is possible to do so now. Then delete release/2.1.3xx-src.
There was a problem hiding this comment.
Isn't that basically what this PR is doing? How does that relate to the SDK version?
There was a problem hiding this comment.
I guess it is, but there might have been more changes in the -src branch since this PR was created. I think it would be better to create a new PR that merges the branch.
There was a problem hiding this comment.
I see, that sounds fine to me. Will you have time to do that soon?
|
Up to @mmitche to say if he feels comfortable with this being merged now. |
|
|
||
| <PropertyGroup> | ||
| <Configuration Condition="'$(Configuration)' == ''">Debug</Configuration> | ||
| <MSBuildAllProjects>$(MSBuildAllProjects);$(MSBuildThisFileDirectory)..\Directory.Build.props</MSBuildAllProjects> |
There was a problem hiding this comment.
This is adding a path to a Directory.Build.props in the parent folder. It should probably be:
<MSBuildAllProjects>$(MSBuildAllProjects);$(MSBuildThisFileDirectory)Directory.Build.props</MSBuildAllProjects>
|
Superseded by #2178 |
8 participants
Enables source build for dotnet/sdk repo with no patches.