dotnet / Nerdbank.GitVersioning

Stamp your assemblies, packages and more with a unique version generated from a single, simple version.json file and include git commit IDs for non-official builds.
https://www.nuget.org/packages/Nerdbank.GitVersioning
MIT License
1.36k stars 166 forks source link

Error in ManagedGit when dealing with non-packed lightweight tags #1026

Closed Rob-Hague closed 8 months ago

Rob-Hague commented 8 months ago

I am trying to build https://github.com/sshnet/SSH.NET/pull/1299 in my local fork. The repo has some tags which are lightweight tags e.g. 2023.0.0. The build fails locally with

1>C:\Users\rhague\.nuget\packages\nerdbank.gitversioning\3.7.48-alpha\build\Nerdbank.GitVersioning.Inner.targets(17,5): error MSB4018: The "Nerdbank.GitVersioning.Tasks.GetBuildVersion" task failed unexpectedly.
1>C:\Users\rhague\.nuget\packages\nerdbank.gitversioning\3.7.48-alpha\build\Nerdbank.GitVersioning.Inner.targets(17,5): error MSB4018: Nerdbank.GitVersioning.GitException: Got a com instead of a tag when opening object 1c7166a002d7633fe1595b3df4634ba0ef6e3138
1>C:\Users\rhague\.nuget\packages\nerdbank.gitversioning\3.7.48-alpha\build\Nerdbank.GitVersioning.Inner.targets(17,5): error MSB4018:    at Nerdbank.GitVersioning.ManagedGit.GitRepository.TryGetObjectByPath(GitObjectId sha, String objectType, Stream& value)
1>C:\Users\rhague\.nuget\packages\nerdbank.gitversioning\3.7.48-alpha\build\Nerdbank.GitVersioning.Inner.targets(17,5): error MSB4018:    at Nerdbank.GitVersioning.ManagedGit.GitRepository.TryGetObjectBySha(GitObjectId sha, String objectType, Stream& value)
1>C:\Users\rhague\.nuget\packages\nerdbank.gitversioning\3.7.48-alpha\build\Nerdbank.GitVersioning.Inner.targets(17,5): error MSB4018:    at Nerdbank.GitVersioning.ManagedGit.GitRepository.<LookupTags>g__HandleCandidate|44_0(GitObjectId pointsAt, String tagName, Boolean isPeeled, <>c__DisplayClass44_0& )
1>C:\Users\rhague\.nuget\packages\nerdbank.gitversioning\3.7.48-alpha\build\Nerdbank.GitVersioning.Inner.targets(17,5): error MSB4018:    at Nerdbank.GitVersioning.ManagedGit.GitRepository.LookupTags(GitObjectId objectId)
1>C:\Users\rhague\.nuget\packages\nerdbank.gitversioning\3.7.48-alpha\build\Nerdbank.GitVersioning.Inner.targets(17,5): error MSB4018:    at Nerdbank.GitVersioning.Managed.ManagedGitContext.get_HeadTags()
1>C:\Users\rhague\.nuget\packages\nerdbank.gitversioning\3.7.48-alpha\build\Nerdbank.GitVersioning.Inner.targets(17,5): error MSB4018:    at Nerdbank.GitVersioning.VersionOracle..ctor(GitContext context, ICloudBuild cloudBuild, Nullable`1 overrideVersionHeightOffset)
1>C:\Users\rhague\.nuget\packages\nerdbank.gitversioning\3.7.48-alpha\build\Nerdbank.GitVersioning.Inner.targets(17,5): error MSB4018:    at Nerdbank.GitVersioning.Tasks.GetBuildVersion.ExecuteInner() in D:\a\1\s\src\Nerdbank.GitVersioning.Tasks\GetBuildVersion.cs:line 240
1>C:\Users\rhague\.nuget\packages\nerdbank.gitversioning\3.7.48-alpha\build\Nerdbank.GitVersioning.Inner.targets(17,5): error MSB4018:    at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
1>C:\Users\rhague\.nuget\packages\nerdbank.gitversioning\3.7.48-alpha\build\Nerdbank.GitVersioning.Inner.targets(17,5): error MSB4018:    at Microsoft.Build.BackEnd.TaskBuilder.<ExecuteInstantiatedTask>d__26.MoveNext()

The problem appears to be that ManagedGit is assuming the object is of type "tag" (annotated) whereas it is of type "commit" (lightweight). The error is here:

https://github.com/dotnet/Nerdbank.GitVersioning/blob/549f6f4ed81ac37dc7be2968690fec9904e87e08/src/NerdBank.GitVersioning/ManagedGit/GitRepository.cs#L782

and it comes from

https://github.com/dotnet/Nerdbank.GitVersioning/blob/549f6f4ed81ac37dc7be2968690fec9904e87e08/src/NerdBank.GitVersioning/ManagedGit/GitRepository.cs#L657

The reproduction is a little convoluted because there is no error on a fresh clone with .pack files, and it also requires the tags to be "unpacked" (in .git/refs/tags rather than in .git/packed-refs). That's because of the !isPeeled check in the above line[^1].

git clone https://github.com/sshnet/SSH.NET.git
cd SSH.NET
git fetch origin pull/1299/head:repro
git checkout 5ab40e # or otherwise git checkout repro
git show-ref 2023.0.0 # 1c7166a002d7633fe1595b3df4634ba0ef6e3138 refs/tags/2023.0.0
git cat-file -t 2023.0.0 # commit
grep -r 2023.0.0 .git/ # .git/packed-refs:1c7166a002d7633fe1595b3df4634ba0ef6e3138 refs/tags/2023.0.0
dotnet build # this works
git tag | xargs git tag -d && git fetch --tags # "unpack" the tags (now 2023.0.0 is a file in refs/tags/)
mv .git/objects/pack/*.pack .
for fn in ./*.pack; do git unpack-objects < "$fn"; done # unpack objects
dotnet build # this fails

Arguably these tags should be annotated, but supposing it makes sense to consider lightweight tags in this process, perhaps TryGetObjectByPath should be returning false rather than throwing (like GitPack.TryGetObject effectively seems to do), and then things should work correctly? I don't know how this logic fits into the wider process.

[^1]: isPeeled comes from the out parameter of EnumeratePackedRefsRaw and relates to the packed-refs file as a whole rather than whether a particular object is peeled. That doesn't seem intuitively right but then I don't know anything about peeling.

AArnott commented 8 months ago

Thanks for your report. You seem to know quite a bit about this and are comfortable looking through the code. How would you feel about authoring the fix?

I didn't write the ManagedGit implementation, so I have to be very careful with anything I change there.

CC: @filipnavara @georg-jung in case either of them want to take a crack at fixing this.

filipnavara commented 8 months ago

Thanks for the detailed description. I think the general assessment of the situation is correct and it would work correctly if TryGetObjectByPath returned false instead of throwing exception. Unfortunately I am quite busy at the moment to submit a PR with appropriate tests.

Rob-Hague commented 8 months ago

Thanks, I'll try it out on the weekend

Cjewett commented 6 months ago

@AArnott Any chance a new pre-release could be generated? We believe this is causing an issue for us but would like to verify.

Realized we can use Nerdbanks public feed, but think it would also be cool to get some of the recent changes into a pre-release and future official release

AArnott commented 6 months ago

Yes. 3.7.62-alpha is going to nuget.org now.