Move obj and bin directories into repo root #10063

natemcmaster · 2019-05-08T00:27:27Z

Resolves #9928
Part of #7280 (converging to Arcade)

The Arcade SDK requires that the obj/ and bin/ folders be placed in the top-level artifacts/ folder of the repo. https://github.com/dotnet/arcade/blob/master/Documentation/ArcadeSdk.md#single-build-output Although this PR does not complete our Arcade convergence, this is a step towards updating our repo to build with the Arcade SDK.

Changes:

Set output path for build to artifacts/bin/$(ProjectName)/
Set intermediate output path for build to artifacts/obj/$(ProjectName)/
Cleanup .gitignore files (remove duplication between repo-root and tested gitignore files)
Add code check which looks for project files that share the same name (could cause issues)
Rename project files to have unique names (avoid race condition of build output)
Update all locations which were hard-coded to expect bin/ and obj/ in the project directory
Add overrides for tests which still assert test binaries exist in a given location relative to the source code

BrennanConroy · 2019-05-08T15:27:37Z

@jkotalik Said something about installers

natemcmaster · 2019-05-08T16:02:44Z

This is a draft PR. Lots of work to do before it's ready to merge. Among other this, hundreds of tests are written to assume the test binary is in a certain folder relative to the source code.

natemcmaster · 2019-05-08T18:03:15Z

Test failures on this PR correspond exactly with tests that are disabled in Helix. The root of the problem appears to be the same -- the test code makes assumptions about the location of source code relative to the test binary. These assumptions are not true in Helix and not true in this PR where obj/ and bin/ directories have been moved.

Fixing this requires updating a lot of test code. @HaoK opened issues to track most of these test projects months ago, but we haven't seen traction on this. I see some of this issues have been punted or backlogged.

@mkArtakMSFT @Eilon @anurse - to move forward on this PR, I see 3 viable options:

Kick the can down the road. Keep these tests disabled on Helix, and find a way to override the Arcade SDK to keep build output in the location the test code expects.
Disablepoolza. I disable the tests completely until they are updated.
Moar halp! We assign higher priority to fixing tests which are broken in Helix and this PR.

Thoughts?

By the way, here are the test projects that are broken:

analogrelay · 2019-05-08T18:23:05Z

Sounds like it would be good to chat about this at a build team sync (unfortunately I'll be missing this Thursday's one...). I'd like to get them all fixed up and working (since Helix requires it too) but don't really have the resources right now to do it.

HaoK · 2019-05-08T18:26:31Z

The good news is I'm somewhat close to finishing Components and Components E2e (those were mostly node issues). Also the two analyzer issues should be a cheap fix, as its just a max file path issue (rename a bunch of files).

So the big ones are really just: ProjectTemplates + hosting functionals left I think

natemcmaster · 2019-05-08T18:28:03Z

@HaoK what are you doing about NodeJS? I'm wondering if we could re-use the approach I'm taking here to install mssql on-demand and extend it to include installing nodejs on-demand?

HaoK · 2019-05-08T18:29:33Z

Yeah have to install nodjs on demand was basically their suggestion: https://github.com/dotnet/core-eng/issues/5302#issuecomment-490522509 I was going to try this today since they pinged me

natemcmaster · 2019-05-08T18:34:10Z

Let's see if we can combine efforts on this. Will ping you internally.

natemcmaster · 2019-05-09T17:42:48Z

We can discuss this more in manager sync today, but I'm favoring option 1

Kick the can down the road. Keep these tests disabled on Helix, and find a way to override the Arcade SDK to keep build output in the location the test code expects.

My concern is that we've been kicking the can down the road for months, but as this "can" is scoped to only 10 projects, it's maybe not that big a deal.

natemcmaster · 2019-05-09T23:47:49Z

/azp run AspNetCore-ci

azure-pipelines · 2019-05-09T23:48:01Z

Azure Pipelines successfully started running 1 pipeline(s).

natemcmaster · 2019-05-10T22:42:57Z

I'm going to send a PR to the team once this merges. After it goes in, everyone who pulls this change will need to do a git clean to remove obj/ directories, otherwise they'll start hitting dozens of compiler error caused by the compiler starting to pick up generated source code.

error CS0579: Duplicate 'System.Reflection.AssemblyCompanyAttribute' attribute

natemcmaster · 2019-05-10T23:30:36Z

Flaky test: InputDateInteractsWithEditContext_NullableDateTimeOffset - appears to be unrelated to this change.

dougbu

This PR contains a number of non-overlapping changes, making it larger than it needs to be and harder to review. Suggest splitting (say) the C++ settings consolidation, project renames and perhaps .gitignore cleanup into a separate PR.

eng/scripts/CodeCheck.ps1

Directory.Build.props

natemcmaster · 2019-05-14T16:14:14Z

@dougbu while it's certainly possible to split this PR, the changes made here were done for related reasons - changing the location of build output made the content of some gitignore files irrelevant, required ensuring project names are unique to avoid overlap in artifacts/bin/, and using a shared file for C++ made it easier to ensure all .vcxproj files have the new setting.

natemcmaster · 2019-05-14T16:14:46Z

In other words, not planning to split this PR unless this somehow blocking the ability to review it.

dougbu

Think I'm about ready to sign off. Suggest a quick offline chat to make sure I'm not missing anything.

dougbu · 2019-05-15T16:15:13Z

src/Components/Directory.Build.props

+    <BaseIntermediateOutputPath />
+    <IntermediateOutputPath />
+    <BaseOutputPath />
+    <OutputPath />


Minor but why don't $(BaseIntermediateOutputPath) and $(IntermediateOutputPath) need to be cleared elsewhere?

Blazor build tools and targets are the only thing which expect the obj/ directory to exist in the project dir. See #5486. Everywhere else I tested this change, the projects and tests were okay with obj being moved to $repoRoot$/artifacts/obj/$projectName$

src/Analyzers/Analyzers/test/Microsoft.AspNetCore.Analyzers.Test.csproj

src/DefaultBuilder/testassets/Directory.Build.props

src/Middleware/CORS/samples/Directory.Build.props

src/Mvc/Extensions.ApiDescription.Client/src/Microsoft.Extensions.ApiDescription.Client.nuspec

dougbu · 2019-05-15T16:30:36Z

src/Servers/IIS/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj

@@ -75,20 +75,15 @@
  <PropertyGroup Label="UserMacros" />
  <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">
    <LinkIncremental>true</LinkIncremental>
-    <OutDir>$(MSBuildProjectDirectory)\bin\$(Configuration)\$(Platform)\</OutDir>


Going forward, will VS restore these settings when we edit project settings? I've seen it undo changes similar to this…

I opened VS and built the project and didn't see .vcxproj files change. What would trigger VS to undo this change?

Change something in the project's property page while in VS. If that update to the project doesn't regurgitate these settings, life is grand.

Seems to be okay. Didn't run into problems with VS when changing properties via UI.

dougbu · 2019-05-15T16:41:28Z

Should we be concerned about the failing Helix run?

natemcmaster · 2019-05-15T16:44:37Z

Helix failed with

Work item IISExpress.FunctionalTests/netcoreapp3.0 in job c2f5f074-88e1-4e43-877f-781abfb5abee has failed.

I'll rebase on changes Justin merged yesterday to handle this.

natemcmaster · 2019-05-15T20:12:21Z

@aspnet-hello triage https://dev.azure.com/dnceng/public/_build/results?buildId=190143

natemcmaster added 4 commits May 7, 2019 16:00

Move obj and bin directories into repo root

3c3561a

fixup! Move obj and bin directories into repo root

6a12e2a

fixup! Move obj and bin directories into repo root

c8a554d

fixup! Move obj and bin directories into repo root

7c211da

Eilon added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label May 8, 2019

natemcmaster added 2 commits May 9, 2019 14:16

Merge branch 'master' into obj-bin

a1034a4

Fix Microsoft.AspNetCore.Identity.Test

ab087df

natemcmaster added 6 commits May 10, 2019 08:59

Workaround test failures caused by changing output directory

f93c3e2

More workarounds for obj/bin dir move

99438a5

Add more workarounds

a7ce724

fixup! Add more workarounds

7c5f497

fixup! fixup! Add more workarounds

f21d58c

fixup! fixup! fixup! Add more workarounds

0b62097

natemcmaster marked this pull request as ready for review May 10, 2019 22:40

natemcmaster requested review from analogrelay, BrennanConroy, halter73, jkotalik and mikaelm12 as code owners May 10, 2019 22:40

natemcmaster requested review from ryanbrandenburg, SteveSandersonMS, Tratcher and a team as code owners May 10, 2019 22:40

natemcmaster added 3 commits May 10, 2019 16:36

Mark InputDateInteractsWithEditContext_NullableDateTimeOffset as flaky

74f62da

Fix signalr functional tests

aa7719b

Remove --

5323709

dougbu reviewed May 12, 2019

View reviewed changes

eng/scripts/CodeCheck.ps1 Outdated Show resolved Hide resolved

Directory.Build.props Show resolved Hide resolved

PR feedback

586c772

natemcmaster added 4 commits May 14, 2019 09:42

Merge branch 'master' into obj-bin

9c822ec

More workarounds for IIS

a0650a2

More workarounds for IIS tests

3cadb5e

fixup! More workarounds for IIS tests

58c165d

dougbu reviewed May 15, 2019

View reviewed changes

Merge branch 'master' into obj-bin

b492882

dougbu approved these changes May 15, 2019

View reviewed changes

Merge branch 'master' into obj-bin

ef39232

natemcmaster merged commit dc90e11 into dotnet:master May 15, 2019

natemcmaster deleted the obj-bin branch May 15, 2019 20:45

HaoK mentioned this pull request May 27, 2020

Try to fix helix output path warning #22256

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move obj and bin directories into repo root #10063

Move obj and bin directories into repo root #10063

natemcmaster commented May 8, 2019 •

edited

Loading

BrennanConroy commented May 8, 2019

natemcmaster commented May 8, 2019

natemcmaster commented May 8, 2019

analogrelay commented May 8, 2019

HaoK commented May 8, 2019

natemcmaster commented May 8, 2019

HaoK commented May 8, 2019

natemcmaster commented May 8, 2019

natemcmaster commented May 9, 2019

natemcmaster commented May 9, 2019

azure-pipelines bot commented May 9, 2019

natemcmaster commented May 10, 2019

natemcmaster commented May 10, 2019

dougbu left a comment

natemcmaster commented May 14, 2019

natemcmaster commented May 14, 2019

dougbu left a comment

dougbu May 15, 2019

natemcmaster May 15, 2019

dougbu May 15, 2019

natemcmaster May 15, 2019

dougbu May 15, 2019

natemcmaster May 15, 2019

dougbu commented May 15, 2019

natemcmaster commented May 15, 2019

natemcmaster commented May 15, 2019

Move obj and bin directories into repo root #10063

Move obj and bin directories into repo root #10063

Conversation

natemcmaster commented May 8, 2019 • edited Loading

BrennanConroy commented May 8, 2019

natemcmaster commented May 8, 2019

natemcmaster commented May 8, 2019

analogrelay commented May 8, 2019

HaoK commented May 8, 2019

natemcmaster commented May 8, 2019

HaoK commented May 8, 2019

natemcmaster commented May 8, 2019

natemcmaster commented May 9, 2019

natemcmaster commented May 9, 2019

azure-pipelines bot commented May 9, 2019

natemcmaster commented May 10, 2019

natemcmaster commented May 10, 2019

dougbu left a comment

Choose a reason for hiding this comment

natemcmaster commented May 14, 2019

natemcmaster commented May 14, 2019

dougbu left a comment

Choose a reason for hiding this comment

dougbu May 15, 2019

Choose a reason for hiding this comment

natemcmaster May 15, 2019

Choose a reason for hiding this comment

dougbu May 15, 2019

Choose a reason for hiding this comment

natemcmaster May 15, 2019

Choose a reason for hiding this comment

dougbu May 15, 2019

Choose a reason for hiding this comment

natemcmaster May 15, 2019

Choose a reason for hiding this comment

dougbu commented May 15, 2019

natemcmaster commented May 15, 2019

natemcmaster commented May 15, 2019

natemcmaster commented May 8, 2019 •

edited

Loading