Add an MSBuild property for disabling any NBGV tasks

[m0sa]

TL;DR

It would be great to have something like

> dotnet build -p:NBGV_Disabled=true

Longer version

I'm constantly hitting issues with NBGV in various inner-loop (developer machine, devcontainers, omnisharp, git submodule) scenarios. It usually works great once it's set up and running correctly in the CI environment, but I find myself adding a condition to the package import so I can turn it of when I don't need it, e.g.

<PackageReference Include="Nerdbank.GitVersioning" Version="..." Condition=" '$(NBGV_Disabled)' != 'true' " />

... so I can opt out of it via an environment variable. I usually don't care what the package version turns out to be while working on a project in my IDE. It would be great if something like this would be supported out of the box.

[macsux]

Second on this requirement, as I need to package it with a custom constant version for purposes of unit tests, and this is messing with the results. Additionally, due to the quirks of MSBuild, conditional package references don't work for me in the way the workaround suggests.

[AArnott]

a custom constant version for purposes of unit tests

@macsux When tests need to predict the version number, do you find the generated ThisAssembly class to be insufficient? It typically satisfies my needs, so I wonder if you tried it.

@m0sa I'm interested in how NB.GV is failing for you such that you want to shut it off. Are these cases where we can improve so as to not fail?
A condition on a PackageReference seems reasonable IMO, if that works for you. But shutting it off completely can break your build as well, if for example you have code that depends on ThisAssembly. So if you can help me understand what exactly breaks, we can see if those parts can be fixed or should be individually deactivatable such that it doesn't break your build for other reasons.

[m0sa]

I only get value from NerdBank when I need to stamp a (pre-)release via CI/CD. At other times (dev inner loop), I just don't care about having everything set up so that NerdBank is happy, and there's always something.

e.g. last time I was fighting this when I had to pull in a dependency via a submodule:

/home/vscode/.nuget/packages/nerdbank.gitversioning/3.4.179-rc/build/Nerdbank.GitVersioning.Inner.targets(17,5): error MSB4018: The "Nerdbank.GitVersioning.Tasks.GetBuildVersion" task failed unexpectedly.
/home/vscode/.nuget/packages/nerdbank.gitversioning/3.4.179-rc/build/Nerdbank.GitVersioning.Inner.targets(17,5): error MSB4018: System.ArgumentException: No git repo found here. (Parameter 'workingDirectory')
/home/vscode/.nuget/packages/nerdbank.gitversioning/3.4.179-rc/build/Nerdbank.GitVersioning.Inner.targets(17,5): error MSB4018:    at Nerdbank.GitVersioning.Managed.ManagedGitContext..ctor(String workingDirectory, String dotGitPath, String committish)
/home/vscode/.nuget/packages/nerdbank.gitversioning/3.4.179-rc/build/Nerdbank.GitVersioning.Inner.targets(17,5): error MSB4018:    at Nerdbank.GitVersioning.GitContext.Create(String path, String committish, Boolean writable)
/home/vscode/.nuget/packages/nerdbank.gitversioning/3.4.179-rc/build/Nerdbank.GitVersioning.Inner.targets(17,5): error MSB4018:    at Nerdbank.GitVersioning.Tasks.GetBuildVersion.ExecuteInner()

And that extends to the PR unit test builds etc, when I just want the DLL but don't care about checking out the entire depth of the github repo, etc.

[AArnott]

Thanks, @m0sa. Did this build failure happen for a project within the submodule then? Did you mean for your version stamp to apply within the submodule, or would you rather turn it off for that project for all builds anyway?

What you show above shouldn't happen within a submodule. And the fact that you're using a 3.4 prerelease makes me wonder if we have a regression that should be fixed. I would love to get some repro steps from you, or more details of how I might recreate the problem.

On the other hand, if the error message is right and the project being built is legitimately not in a submodule, I would potentially change this exception to a build warning and no-op when no git repo is found.

[m0sa]

Yeah, I have a root repository with multiple submodules. This was inside one of the submodule folders, building that submodule with dotnet build.

e.g.

rootgitrepo
+-submodule1
+-submodule2

rootgitrepo/submodule1> dotnet build

What I'd want is for something to force that noop, explicitly, even where there is a git repo.

[macsux]

When tests need to predict the version number, do you find the generated ThisAssembly class to be insufficient? It typically satisfies my needs, so I wonder if you tried it.

@AArnott I need an actual nuget version number which is not present in ThisAssembly. Due to the nature of my project, I'm testing nuget package itself (which is the artifact produced from the project) under different permutations of c# projects where it is added inside docker container. When generating sample projects I need to reference a nuget package to add. If ThisAssembly can be expanded to include other version info (such as nuget), not just assembly version info, it would probably solve my issue. I still think the ability to override the version by passing in Version or PackageVersion into MSBuild should be something that is possible, so having a property that disables nbgv so it doesn't override those values when building would be a win.
Here's unit tests scenario of my project if you're interested
https://github.com/NMica/NMica/blob/master/tests/NMica.Tests/DockerfileSupportedTests.cs

[AArnott]

What I'd want is for something to force that noop, explicitly, even where there is a git repo.

To clarify, do you want it to no-op all the time, even for your official builds, @m0sa? If so, then you should prevent the PackageReference from infecting the projects in your submodules, which may be picking up a Directory.Build.props file from your repo root directory. The way I do this is put my submodule(s) under an extra directory layer:

rootgitrepo
+-ext
  +-submodule1
  +-submodule2

Then create a ext\Directory.Build.props file that does not recursively import the file in the parent directory. Or if you need to still import it, you can set a property before importing the parent file so the parent file can use it as a condition to suppress the package reference.

Does that help?

[rcdailey]

I'd like to throw my vote and use case in here.

I'm setting up a GitHub Action workflow where I do tests -> matrix build. The matrix build defines 3 different runtimes for dotnet publish. I do not want to run my tests 3 times, so I put it in a prerequisite job that does a:

• shallow clone
• build & test
• publish test report

This makes my matrix builds faster, and also means I'm testing code 1 time instead of 3 times. However, my test task fails right now because of the shallow clone with:

error MSB4018: Nerdbank.GitVersioning.GitException: Shallow clone lacks the objects required to calculate version height. Use full clones or clones with a history at least as deep as the last version height resetting change.

I realize this is "working as designed", but in my test job, I don't care about versioning. I want to use the shallow clone to improve the total build time of this step. I would like to selectively disable all NBGV functionality in the test job, but keep it enabled in later builds after it.

OP's workaround (utilizing conditions) works for now, but I feel that this should be built in.

[AArnott]

I agree there should be a way to opt out with a property -- I just don't see that we need to invent a way when MSBuild itself already offers one by virtue of the Condition attribute.

[m0sa]

Conditions on PackageReferences are full of gotchas, see

• Nuget conditioning on $(Configuration) is not working NuGet/Home#5895
• https://stackoverflow.com/questions/48590270/packagereference-condition-is-ignored
• https://stackoverflow.com/questions/44803176/conditional-package-reference-in-uwp-project

[macsux]

I've spent half a day to try to get msbuild to properly recognize conditional package import, doesn't work

…

On Sat., Apr. 10, 2021, 2:43 p.m. Samo Prelog, ***@***.***> wrote: Conditions on PackageReferences are full of gotchas, see - NuGet/Home#5895 <NuGet/Home#5895> - https://stackoverflow.com/questions/48590270/packagereference-condition-is-ignored - https://stackoverflow.com/questions/44803176/conditional-package-reference-in-uwp-project — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#568 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAINFWGLUSYW55SMBSABLFDTICL5HANCNFSM4ZUWY5PQ> .

[rcdailey]

I realized a while after I posted that the workaround doesn't exactly work in my case. I introduced a usage of MyAssembly to grab the version number that nbgv generates for my assembly. It uses this to print the version number to console for my CLI app. I get an error about this variable being inaccessible when I disable the NBGV package via condition.

[AArnott]

Ok, with @m0sa and @macsux's comments, perhaps we should add a property check (e.g. NBGV_Disabled) to appease your use case.

@rcdailey: yes, that's exactly the scenario I'm concerned about by "supporting" a way to disable versioning. For some projects (that depend on ThisAssembly), disabling the package will break compilation. We could potentially "disable" it in a way that ThisAssembly is still generated, but without ever calculating the version info, if that actually still achieves the goal of disabling it.

[rcdailey]

Yeah, that's what I'd expect. If you disable NBGV it still generates that assembly info, but with stubbed-out data as appropriate.

[AArnott]

Any volunteers to implement that?

[m0sa]

I'd love to take a stab at it

…

On Mon, Apr 12, 2021, 12:22 AM Andrew Arnott ***@***.***> wrote: Any volunteers to implement that? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#568 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAD27FWZDW7ZEBCZVGEBPVDTIIOKPANCNFSM4ZUWY5PQ> .

[KalleOlaviNiemitalo]

I want this as a way to speed up incremental Debug builds as a developer.

I can almost get this effect by conditionally setting GenerateAssemblyVersionInfo=false and adding something to DefineConstants so as to stop ThisAssembly from being referenced, but Nerdbank.GitVersioning.targets still sets GenerateAssemblyInformationalVersionAttribute=false and others.

#586 looks like it would work in my scenario, but it is still a draft. Would it be easier to move Nerdbank.GitVersioning.targets to Nerdbank.GitVersioning.Enabled.targets and then add a new Nerdbank.GitVersioning.targets that conditionally imports Nerdbank.GitVersioning.Enabled.targets? That way, the PackageReference would have no effect when disabled. (Except Nerdbank.GitVersioning.props would still set NBGV_CacheMode and NBGV_CachingProjectReference, but nothing would read those.) I suppose the additional import could hurt the build performance a bit, though.

Another possible workaround at the application side might be to add ExcludeAssets="all" GeneratePathProperty="true" to the PackageReference, and then conditionally import Nerdbank.GitVersioning.props and Nerdbank.GitVersioning.targets using the generated property. However, GeneratePathProperty requires NuGet 5.0 or greater, and I still need to support an earlier version.