Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deterministic sourcepaths #6668

Closed
NinoFloris opened this issue May 2, 2019 · 9 comments
Closed

Deterministic sourcepaths #6668

NinoFloris opened this issue May 2, 2019 · 9 comments

Comments

@NinoFloris
Copy link
Contributor

Now that #6609 is merged. Thanks @saul 🎉

Is FCS getting integrated with SourceLink and Deterministic switches like this Roslyn target?
https://github.com/dotnet/roslyn/blob/2f57d11ecc36d11c06fc862dcb0de37c96d305d2/src/Compilers/Core/MSBuildTask/Microsoft.Managed.Core.targets

It powers the automatic deterministic source paths when these props are set

<Deterministic>true</Deterministic>
<ContinuousIntegrationBuild>true</ContinuousIntegrationBuild>

Which are documented for SourceLink
https://github.com/dotnet/sourcelink/tree/master/docs#deterministicsourcepaths

Starting with .NET Core SDK 2.1.300 a fully deterministic build is turned on when both Deterministic and ContinuousIntegrationBuild properties are set to true.

It seems Microsoft.FSharp.NetSdk.targets should join in the fun!

@cartermp
Copy link
Contributor

cartermp commented May 2, 2019

I'd definitely like that.

@saul
Copy link
Contributor

saul commented May 2, 2019

It seems Microsoft.FSharp.NetSdk.targets should join in the fun!

I think that's the wrong file - for it to apply to this repo, you need to look at: https://github.com/Microsoft/visualfsharp/blob/master/FSharpBuild.Directory.Build.props

Adding <Deterministic>true</Deterministic> (for all builds) and <PathMap> (for CI builds) should achieve what you want. I imagine we also need to set <ContinuousIntegrationBuild>true</ContinuousIntegrationBuild> (unless Arcade does this for us)

@NinoFloris
Copy link
Contributor Author

@saul I meant for the .net sdk to do this for all F# projects, not the compiler repo perse

@saul
Copy link
Contributor

saul commented May 2, 2019

In which case I’m not really following exactly what you’re asking for? PathMap is very specific to the project/solution that it’s set for. You’ve linked to a Roslyn repo-specific targets file - that doesn’t apply to all C# projects.

I think I’m misunderstanding what you’re asking for here.

@cartermp
Copy link
Contributor

cartermp commented May 2, 2019

All .NET SDK projects may not work. It's not used as a default for C# project templates either.

@NinoFloris
Copy link
Contributor Author

NinoFloris commented May 3, 2019

I'm asking for support to automatically enable an F# version of DeterministicSourcePath path to pathmap conversion and the other things that trigger in Roslyn sdk compiler targets when ContinuousIntegration = true and Deterministic = true.

When those two options are on, the PDB will automatically have paths embedded with a deterministic /_/ base dir, this works for both SourceLink and embedded source.

@saul I'm no expert on the full sdk integration story but that roslyn target does not seem repo specific, it's part of the folder

Microsoft.Build.Tasks.CodeAnalysis
This MSBuild tasks contains the core tasks and targets for compiling C# and VB projects.

and is imported by Microsoft.CSharp.Core.targets and Microsoft.VisualBasic.Core.targets also I see all those targets in msbuild binary logs.

@cartermp
Copy link
Contributor

cartermp commented May 4, 2019

So Deterministic is the default for C# and VB here now. There were a few reports by people who wanted the binary version to be different even though they built and nothing changed, but the default seemed like the right choice.

So for this repo we should do this (cc @brettfo @KevinRansom), and I think we should also set Deterministic to true for all F#/.NET SDK projects.

@cartermp cartermp added this to the .NET Core 3.0 milestone May 4, 2019
@NinoFloris
Copy link
Contributor Author

@cartermp and what about the DeterministicSourcePaths (which is an MSBuild prop) support?

@NinoFloris
Copy link
Contributor Author

This seems to be done now builds are done with arcade 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants