Runtime: Extra runtime directory & missing ConfigurationGroup from bin and obj directories.

[ericstj]

@ViktorHofer noticed we're producing 2 runtime directories.

Eg:

artifacts\bin\runtime\netcoreapp-Windows_NT
artifacts\bin\runtime\netcoreapp-Windows_NT-Debug-x64

This is most likely an issue with runtime.depproj

EDIT by @ViktorHofer: example was missing the OSGroup in the first line. Also changed to netcoreapp to give it a higher severity ;)

[ViktorHofer]

cc @safern @weshaggard

[ericstj]

Just to confirm, this isn't breaking anything just an observation of extra output directory?

[ViktorHofer]

Right, the files are just duplicated across both directory therefore nothing is broken.

[ericstj]

I looked into this. It's actually a collision of our targetpath convention with the runtime directory.

Since runtime.depproj's project name is 'runtime' it collides with our runtimepath convention.

Previously we had BuildConfiguration in our TargetPath (bin\

Interestingly the new convention removes debug/release from the path, so that could be a problem for folks who are cross-compiling by release/debug with a single enlistment.

@weshaggard was it intentional to remove the BuildConfiguration from the bin and obj paths?

[ViktorHofer]

Previously we had BuildConfiguration in our TargetPath (bin\\) with the latest we don't have this (artifactsbin\\)

I thought that was intentional and didn't comment on it. The omission of debug/release indicates that it probably was unintentional.

[weshaggard]

We didn't have BuildConfiguration exactly we had OSPlatformConfig which included a number of the same things. When I switch I tried to use what was set in arcade https://github.com/dotnet/arcade/blob/master/src/Microsoft.DotNet.Arcade.Sdk/tools/ProjectLayout.props#L18 as we already set the Configuration I started depending on that but I overlooked the fact that we don't have ConfigurationGroup included in that when we do this.

I'd suggest we consider starting to encode ConfigurationGroup when we set the Configuration property that we are building. So add it https://github.com/dotnet/corefx/blob/master/eng/buildvertical.targets#L97 and https://github.com/dotnet/corefx/blob/master/eng/buildvertical.targets#L179. We would also need to add it here wherever we do our BuildAll targets like. https://github.com/dotnet/corefx/blob/master/eng/buildvertical.targets#L97.

[ericstj]

I noticed we also are building with different configuration strings for direct project build vs traversal build.

I had a slightly different idea for the fix here: https://github.com/dotnet/arcade/issues/1434

Essentially always standardize the Configuration string in the props files after we rip it apart.

[ericstj]

My tactical change for this introduced race conditions during the release builds. Reopening this as I undid part of the fix.

Note the part that had runtime clashing with the runtime dir is still in, I've only undid the portion that ensured ConfigurationGroup unique paths.

[ViktorHofer]

OK, that's unfortunate. Do you already have a plan how to work around the race condition?

[ericstj]

I undid the change so the race condition is gone. At the moment lacking ConfigurationGroup from the output paths isn't a blocking problem. If I had a choice rather than to invest more into the Configuration-encoding path I'd like to pursue the TargetFrameworks solution which avoids this problem entirely.

FWIW I left in the portion of the fix that avoids the collision between runtime path and the runtime.depproj so the original issue is fixed, just not the other issue I noticed.

[stephentoub]

cc: @alexperovich, @danmosemsft, @KirillOsenkov

Do we have any further idea how to address this? It's breaking source.dot.net. I believe @KirillOsenkov has some ideas for hacks he could use to workaround this, but I'm not sure how robust those are.

[ericstj]

A few ideas all of which are messy / ugly. Just need to pick the least ugly one.

[ericstj]

I'm working on a fix for this in arcade and will submit shortly.