Skip to content

Auto-update submodules to the commits used in latest ProdCon build#371

Merged
dagood merged 2 commits into
dotnet:dev/release/2.1from
dagood:update-from-prodcon
Mar 27, 2018
Merged

Auto-update submodules to the commits used in latest ProdCon build#371
dagood merged 2 commits into
dotnet:dev/release/2.1from
dagood:update-from-prodcon

Conversation

@dagood
Copy link
Copy Markdown
Member

@dagood dagood commented Mar 20, 2018

To try it out, use build.cmd /t:UpdateDependencies to update based on the ProdConCurrentRef source of truth (e.g. if you want to pin to a specific ProdCon build) and build.cmd /t:UpdateToRemoteDependencies to update to the latest manifest on dotnet/versions master.

Uses new updater and dependency info from dotnet/buildtools#1968.

There's some metadata checked into the project files that this updater won't change. I have a followup issue for that: #370.

#342

Also adds a BuildTools updater: #270

Comment thread dependencies.props
for which dotnet/versions commit was last used to update the dependency.
-->
<PropertyGroup>
<ProdConCurrentRef>7daa2e55de668c4c8ee832059aab04917153881e</ProdConCurrentRef>
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@dagood
Copy link
Copy Markdown
Member Author

dagood commented Mar 20, 2018

I forgot to actually update BuildTools here, and that this branch pulls BuildTools release/2.1. Submitted dotnet/buildtools#1975 to port the updater to 2.1 BuildTools, then I'll update this PR.

Comment thread build.proj Outdated
</Target>

<Import Project="$(ProjectDir)dependencies.targets" />
<Import Project="$(ProjectDir)dependencies.targets" Condition="Exists('$(ProjectDir)dependencies.targets')" />
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure I understand why you added the condition. You are adding the files, so they will always exists, right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Kinda described in the commit message:

There's no need for the BfS tarball to include these files: it doesn't need to run repo-global auto-updates like submodule update.

The project files that get into the tarball are manually listed here:

cp $SCRIPT_ROOT/build.proj $TARBALL_ROOT/
cp $SCRIPT_ROOT/dir.props $TARBALL_ROOT/
cp $SCRIPT_ROOT/DotnetCLIVersion.txt $TARBALL_ROOT/
cp -r $SCRIPT_ROOT/keys $TARBALL_ROOT/
cp -r $SCRIPT_ROOT/patches $TARBALL_ROOT/
cp -r $SCRIPT_ROOT/scripts $TARBALL_ROOT/
cp -r $SCRIPT_ROOT/src $TARBALL_ROOT/
cp -r $SCRIPT_ROOT/repos $TARBALL_ROOT/
cp -r $SCRIPT_ROOT/tools-local $TARBALL_ROOT/

I figure things that aren't used in the tarball build aren't included, and there's no need to add these two files to that list.

@crummel is that the intention? Does it need to be so explicit?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we want to keep the tarball as minimal as possible so if the workaround isn't causing too much extra work I think we'd prefer to keep it out. If it's causing issues we can include the new files.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The only concern I have is that someone could add something to dependencies.props/targets and expect it to be set during all source-build builds. Not sure how easy that would be to debug depending on what's added.

Keeping it out of the tarball is a bit of complexity that I'm not sure we need. But maintaining that list of files is also annoying... would *.proj, *.props, *.targets at the root be ok?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That sounds fine to me.

@dagood
Copy link
Copy Markdown
Member Author

dagood commented Mar 21, 2018

Look like the buildtools upgrade broke the build in general. Lots of:

error MSB4064: The "OverrideToolHost" parameter is not supported by the "Csc" task. Verify the parameter exists on the task, and it is a settable public instance property.
error MSB4063: The "Csc" task could not be initialized with its input parameters.

Makes sense: BuildTools merged master into release/2.1 for preview2. Similar merges have also happened in the submodule repos, but we have the old preview1 commits checked out.

I think the cleanest way to get this feature in is to update to preview2 commits first. Getting it working should involve updating BuildTools, so this PR wouldn't need to handle upgrading BuildTools over the preview1-preview2 border.

@dagood
Copy link
Copy Markdown
Member Author

dagood commented Mar 21, 2018

I talked with @eerhardt and @weshaggard about the CI errors in dotnet/standard. The plan is to update standard's release/2.0.0 branch to use a compatible BuildTools. @eerhardt pointed out that the change to use the CLI's copy of MSBuild rather than the one from BuildTools is probably what's breaking--I'm working on fixing and making a dotnet/standard PR.

What I was talking about above about 2.1-preview1 vs. 2.1-preview2 doesn't really apply in this case, since we're still building dotnet/standard 2.0.

Auto-update submodules to ProdCon build commits, and BuildTools.
@dagood dagood force-pushed the update-from-prodcon branch from 51f0ce8 to c08c825 Compare March 26, 2018 23:21
@dagood
Copy link
Copy Markdown
Member Author

dagood commented Mar 27, 2018

@dseefeld @crummel PTAL

@dagood dagood merged commit 4a2377a into dotnet:dev/release/2.1 Mar 27, 2018
@dagood dagood deleted the update-from-prodcon branch March 27, 2018 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants