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

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

Closed
Rob-Hague opened this issue Feb 1, 2024 · 5 comments · Fixed by #1029
Closed

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

Rob-Hague opened this issue Feb 1, 2024 · 5 comments · Fixed by #1029

Comments

@Rob-Hague
Copy link
Contributor

I am trying to build sshnet/SSH.NET#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:

throw new GitException($"Got a {objectStream.ObjectType} instead of a {objectType} when opening object {sha}");

and it comes from

else if (!isPeeled && this.TryGetObjectBySha(pointsAt, "tag", out Stream? tagContent))

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 line1.

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.

Footnotes

  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
Copy link
Collaborator

AArnott commented Feb 1, 2024

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
Copy link
Member

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
Copy link
Contributor Author

Thanks, I'll try it out on the weekend

@Cjewett
Copy link

Cjewett commented Apr 3, 2024

@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
Copy link
Collaborator

AArnott commented Apr 6, 2024

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

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

Successfully merging a pull request may close this issue.

4 participants