Skip to content

Add more binlogging#775

Merged
crummel merged 2 commits into
dotnet:release/2.1from
crummel:addMoreBinLogging
Sep 17, 2018
Merged

Add more binlogging#775
crummel merged 2 commits into
dotnet:release/2.1from
crummel:addMoreBinLogging

Conversation

@crummel
Copy link
Copy Markdown
Contributor

@crummel crummel commented Sep 14, 2018

In trying to troubleshoot a couple build issues I realized that MSBuild binlogs were nice to have. This change adds binlogging everywhere we can without changing the submodules' build scripts. It also fixes up submodules' RepoBuilds so that they execute with the project directory as their current directory - this was the cause of getting random log files in the repos directory.

@crummel crummel self-assigned this Sep 14, 2018
@crummel crummel requested review from dagood and dseefeld September 14, 2018 21:17
@omajid
Copy link
Copy Markdown
Member

omajid commented Sep 14, 2018

Instead of editing BuildCommand in individual repo files, would changing repos/dir.targets directly work? It contains lines like

<Exec Command="$(BuildCommand) $(RepoApiArgs) $(RedirectRepoOutputToLog)" WorkingDirectory="$(ProjectDirectory)" EnvironmentVariables="@(EnvironmentVariables)" />

@dagood
Copy link
Copy Markdown
Member

dagood commented Sep 17, 2018

I imagine some repos' build scripts don't accept /bl (or other arbitrary msbuild parameters) so it seems likely to take time to sort that out. Many of the changes in this PR also don't run through dir.target's target and run Exec calls directly, so they'd still be needed anyway.

I don't think we're hoping for more command consolidation in the repos for 2.1/2.2. Arcade will give us more consistent commands for master. Refactoring what we can would be nice for maintenance, but I think this PR is fine for now as an incremental diag helper.

@crummel crummel merged commit 63d84cf into dotnet:release/2.1 Sep 17, 2018
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