Roslyn: DeterministicSourcePaths can break building if source control information not available

[KevinCathcart]

Version Used:
dotnet sdk 2.2.300
Steps to Reproduce:

1. dotnet new console
2. dotnet build /p:ContinuousIntegrationBuild=true

Actual Behavior:

C:\Program Files\dotnet\sdk\2.2.300\Roslyn\Microsoft.Managed.Core.targets(102,5): error : SourceRoot items must include at least one top-level (not nested) item when DeterministicSourcePaths is true

Expected Behavior:
Build Succeeds.

The mere fact that we we are performing a continuous integration build does not mean that source control information will be available to MSBuild. However deterministic builds are enabled by default, and turning on ContinuousIntegrationBuild, means that DeterministicSourcePaths is enabled.

That would be fine if DeterministicSourcePaths worked when no source control information was available, but it requires SourceRoot items, which are only created by a source control information provider like the Source Link packages.

This bit me when I tried to add a new Test project to a solution being built with /p:ContinuousIntegrationBuild=true. I did not bother to add a Source Link NuGet package since I was not going to package or publish the test project on the CI server, merely run it, so I don't actually need Source Link data in the pdbs.

Obviously I know the workarounds (add the Source Link package or set DeterministicSourcePaths to false in the test project).

But ideally they would not be needed. Instead ideally, either a) DeterministicSourcePaths would only be set if a source control information provider package was installed, or b) that DeterministicSourcePaths was somehow made to work even without a source provider package.

@tmat I think you are the expert on this stuff, so this is probably your issue.

[tmat]

DeterministicSourcePaths does not need source control information. It just needs to know where the repository root is located.

You can define the repository root by adding the following to the Directory.Build.props file in the root dir:

<ItemGroup>
  <SourceRoot Include="$(MSBuildThisFileDirectory)/"/>
</ItemGroup>

[KevinCathcart]

tmat, that is strictly speaking true, but other than manually adding a SourceRoot element, or using a Source Link package, there is no mechanism for populating that. I would also argue that calling any SourceRoot item source control information is proper, given that the comments already call it that. (Any target that reads [...], SourceRoot, and other source control properties and items [...]).

My gripe is that with a property as generically named as "ContinuousIntegrationBuild" (which is likely to grow additional functionality over time, since it is the obvious mechanism for informing the SDK that we are running in a CI server), it really should not be necessary to make any source level changes.

My real preference would be to downgrade that error to a warning. (Perhaps only if the developer did not explicitly set DeterministicSourcePaths?) Another reasonable alternative would be to introduce some special MSBuild property that if set would generate a SourceRoot item. It would be easy to configure the CI server set that property to the root of the "workspace" (or whatever it happens to call the equivalent concept).

[tmat]

@KevinCathcart The problem is that msbuild does not know what the repository root directory is - it does not have a concept of repository. Someone has to determine that. SourceLink is able to do that based on the source control information. As I pointed out above another option is to specify it explicitly in Directory.Build.props. Yet another option is to read CI environment variable as you suggest. The problem is that there is no standard naming of these variables, so far we have a proposal for such naming but it's not being implemented yet.

The intention is that ContinuousIntegrationBuild is going to be used for builds on CI server and that these builds are by default configured to be fully deterministic, even if that means requiring additional settings and reporting an error if you don't have them in place (or if your projects don't use some package like SourceLink that fills in these settings).

[gafter]

@jaredpar Please close if you think that is appropriate.

[KevinCathcart]

@tmat:

The intention is that ContinuousIntegrationBuild is going to be used for builds on CI server and that these builds are by default configured to be fully deterministic, even if that means requiring additional settings

One problem is that you don't offer these additional settings in a clean way. The only options involve adding or modifying files in the source repository. That is a bad thing. If the build server needs to supply additional information, like the root of the checked out repo, it should be able to do that via MSBuild properties.

Otherwise, it means either: one cannot build older revisions deterministically, or you can by having the build server drop in extra files, or modify some file checked into source control. Anybody who cares about deterministic builds would NOT want the build server adding files to the build, or modifying the files in the build.