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

Fix resolving HEAD reference if it's a packed ref #613

Merged
merged 6 commits into from Jun 12, 2021

Conversation

filipnavara
Copy link
Member

@filipnavara filipnavara commented Jun 11, 2021

The issue is that we have a repository that has HEAD pointing containing ref: refs/heads/netcore/master. All references are packed so to resolve refs/heads/netcore/master it's necessary to looking into the packed refs and not just on the file system.

Fixes #602

@filipnavara
Copy link
Member Author

Eh, looks like the fix will need a bit of tweaking. At least we know the underlying cause though :)

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 613 in repo dotnet/Nerdbank.GitVersioning

@AArnott AArnott added this to the v3.4 milestone Jun 11, 2021
@AArnott
Copy link
Collaborator

AArnott commented Jun 11, 2021

Thanks for contributing, and especially for the repro steps. Are you planning to add the test case or should I find some time to do that?

@AArnott
Copy link
Collaborator

AArnott commented Jun 11, 2021

Also, we need to rebase your PR to the v3.4 branch since we need to fix that version, and master targets v3.5. Would you like to do that or shall I and force-push to your source branch?

@filipnavara
Copy link
Member Author

You will likely have better idea how to structure the test case than me so I would prefer if you look into it. I've uploaded an example repository in the linked issue so it should be fairly trivial to incorporate it somehow.

Feel free to push to the branch as much as necessary. I can rebase it on top of the 3.4 branch but at the moment I am not anywhere near a computer 😅

It also got me thinking that aside from the actual problem that I fixed it would have been reasonable to update GetBuildVersion to either fail or at least issue a warning in the MSBuild task. Now it silently produces a version without the GIT fields in ThisAssembly and with the base version instead of the calculated one.

@AArnott AArnott self-assigned this Jun 12, 2021
@AArnott AArnott changed the base branch from master to v3.4 June 12, 2021 19:48
@AArnott AArnott marked this pull request as ready for review June 12, 2021 19:49
@AArnott AArnott self-requested a review June 12, 2021 19:49
@AArnott AArnott merged commit 1902982 into dotnet:v3.4 Jun 12, 2021
@filipnavara filipnavara deleted the head-in-packed-refs branch June 12, 2021 20:47
@filipnavara
Copy link
Member Author

Thanks!

@AArnott
Copy link
Collaborator

AArnott commented Jun 12, 2021

Thank you very much for contributing the fix!

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

Successfully merging this pull request may close these issues.

None yet

2 participants