dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.71k stars 3.98k forks source link

Determinism end-user experience is poor #26231

Open IanKemp opened 6 years ago

IanKemp commented 6 years ago

I created a new .NET Standard 2.0 class library in Visual Studio 2017 (15.6.6) this morning and set the Assembly Version to 1.0.*. Upon build, I was confronted with the error message:

The specified version string contains wildcards, which are not compatible with determinism. Either remove wildcards from the version string, or disable determinism for this compilation

Cue research into what the heck determinism means in the context of C# compilation, leading me to know that I do, in fact, want it disabled for my use case. Cue more research into how to disable it, which finally lead me to this answer on Stack Overflow. Not very discoverable!

I see that #22660 and #22973 somewhat addressed this issue by making the error more explicit, but in my opinion it's still not useful or helpful enough:

This is compounded by the fact that there doesn't seem to be a Microsoft documentation page explaining how to control determinism from the command-line or in a .csproj. The closest I have found is this.

tl;dr this is all a bit of a mess and it's definitely not a great experience for someone who is dipping their toe in .NET Standard. Suggestions to make things better:

sharwell commented 6 years ago

This is a great report covering the complete user experience. Thanks for taking the time to report the details of the various pain points you hit!

leading me to know that I do, in fact, want it disabled for my use case

My biggest initial concern is the documentation led someone to this conclusion.

IanKemp commented 6 years ago

@sharwell from the linked SO answer:

However if you are using TeamCity, TFS or other CI/CD tool, it's probably better to keep the version number controlled and incremented by them and pass to build as a parameter

which is exactly my scenario. I would very much like to enable determinism, but right now it's not an option.

Regarding documentation, @jaredpar's blog post is pretty complete, but obviously in no way official.

sharwell commented 6 years ago

@IanKemp I believe you misread the SO answer. When you pass those two properties to the build, it will generate an assembly info file containing the values from it. Here is an example of the properties specified in a manually-incremented file (AssemblyVersion defaults to the value of Version if not specified separately):

https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/f01be7a8262f146766af90138530ee0f21ddfba1/StyleCop.Analyzers/Directory.Build.props#L11-L13

When using these properties, you do not want wildcard versioning, which means determinism can be enabled.

IanKemp commented 6 years ago

@sharwell Thank you - this is what happens when I am sick... and when there isn't appropriate documentation. (Insert sound of dead horse beating.)

So then I guess the only remaining question is: why would you ever want to use in the `Version` properties, and is it worth updating the documentation and whatnot to handle that case in a nicer manner?

sharwell commented 6 years ago

why would you ever want to use in the Version properties

Aside from backwards compatibility (avoiding breaking projects that used it in the past), I am not aware of any reason.

maxima120 commented 4 years ago

it has to be in the project properties UI.. whatever reason for using it or not using it..

BoCoKeith commented 3 years ago

Two thumbs up for adding a determinism checkbox to the project properties. Having to add a tags to the project file to enable auto-versioning is just not right.

At the least: how about adding a comment in the Assembly.cs templates that tells you what to do if you want to enable auto-versioning. Simple fix, no code change, less confusion.

tmat commented 1 month ago

Some doc improvements: https://github.com/dotnet/dotnet-api-docs/pull/9892