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

Check that repository isn't a shallow clone and show an error if it is #3479

Merged
merged 5 commits into from
Apr 14, 2023

Conversation

kimsey0
Copy link
Contributor

@kimsey0 kimsey0 commented Apr 13, 2023

Description

This change detects shallow repositories and exits early with a proper error message.

Related Issue

Fixes #3188.

Motivation and Context

Since September 2022, Azure DevOps has shallow fetch enabled by default in new pipelines. GitVersion doesn't work with shallowly cloned repositories and currently shows a hard-to-debug error message about a NullReferenceException in LibGit2Sharp when encountering such a repository. A better error message will make it easier for users to find and disable shallow fetching.

How Has This Been Tested?

This has an automated test that creates a shallow copy and asserts that the correct error message is shown.

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Sorry, something went wrong.

Copy link
Member

@asbjornu asbjornu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks promising! How do you suggest figuring out whether a repository is a shallow clone or not? Is this information LibGit2Sharp exposes?

@arturcic
Copy link
Member

This looks promising! How do you suggest figuring out whether a repository is a shallow clone or not? Is this information LibGit2Sharp exposes?

@asbjornu yes it seems to be supporting https://github.com/libgit2/libgit2sharp/blob/8c32b6160a49890e26100148e73c1558b0daa9af/LibGit2Sharp/RepositoryInformation.cs#L28,

Also in this PR you can see https://github.com/GitTools/GitVersion/pull/3479/files#diff-6d64c4b71b687750cfe54b49765fe58ccacf69a80c86a2e449cec0be3ebf3596R23

Copy link
Member

@asbjornu asbjornu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would love to have a test that exercises this code somehow. Perhaps it's possible to create a shallow clone in the integration tests somehow?

@asbjornu
Copy link
Member

This looks promising! How do you suggest figuring out whether a repository is a shallow clone or not? Is this information LibGit2Sharp exposes?

@asbjornu yes it seems to be supporting https://github.com/libgit2/libgit2sharp/blob/8c32b6160a49890e26100148e73c1558b0daa9af/LibGit2Sharp/RepositoryInformation.cs#L28,

Also in this PR you can see https://github.com/GitTools/GitVersion/pull/3479/files#diff-6d64c4b71b687750cfe54b49765fe58ccacf69a80c86a2e449cec0be3ebf3596R23

Yeah, GitHub was acting up a bit so all the changed files weren't showing in the code review. It looks like a great way to solve this problem, we just need to figure out a way to test it. :)

@arturcic
Copy link
Member

I would love to have a test that exercises this code somehow. Perhaps it's possible to create a shallow clone in the integration tests somehow?

Something similar to the dynamic clone tests? that will clone the GitVersion repo but with the depth=1

@arturcic arturcic marked this pull request as ready for review April 14, 2023 10:21
@kimsey0
Copy link
Contributor Author

kimsey0 commented Apr 14, 2023

It turns out LibGit2Sharp doesn't support setting the depth when cloning, so I had to make a work around that does a shallow pull and a garbage collection with pruning afterwards. Let me know if you see a better way to test it.

@arturcic
Copy link
Member

It turns out LibGit2Sharp doesn't support setting the depth when cloning, so I had to make a work around that does a shallow pull and a garbage collection with pruning afterwards. Let me know if you see a better way to test it.

That is exactly what I was checking and wanted to suggest. Good catch

@arturcic
Copy link
Member

@kimsey0 can you please rebase your PR branch on the latest main? then I think after the build succeeds we can merge

kimsey0 added 4 commits April 14, 2023 12:24

Verified

This commit was signed with the committer’s verified signature.
ljharb Jordan Harband

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…ository clones

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…ed to disable shallow fetching
@kimsey0
Copy link
Contributor Author

kimsey0 commented Apr 14, 2023

Sure thing! Done.

@arturcic arturcic requested a review from asbjornu April 14, 2023 11:58
@arturcic
Copy link
Member

@asbjornu please have a new look and if it works for you please merge this one

@HHobeck
Copy link
Contributor

HHobeck commented Apr 14, 2023

@kimsey0 Thank you very much taking the time. I have found this article https://github.blog/2020-12-21-get-up-to-speed-with-partial-clone-and-shallow-clone/ which discribes the option we have in git very good.

Following is written:

Shallow clones are the fastest way to get a copy of the working directory at the tip commit with the additional cost that fetching from these repositories is much more expensive, so we do not recommend shallow clones for developers. If you need the commit history for your build, then a treeless partial clone might work better for you than a full clone.

My question is: Does GitVersion support treeless partial clone? Or are other options available in git which boost the performance?

@kimsey0
Copy link
Contributor Author

kimsey0 commented Apr 14, 2023

@HHobeck: I don't know. I have never heard of treeless partial clones before. It might be worth opening a new issue about if you would like to explore it.

@arturcic
Copy link
Member

@HHobeck Libgit2 that is bundled in LibGit2Sharp that GitVersion uses does not have shallow clone support libgit2/libgit2#3058

Copy link
Member

@asbjornu asbjornu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a tiny typo, otherwise this is fantastic! :shipit: 🚀 👏🏼 🎉

@asbjornu asbjornu enabled auto-merge April 14, 2023 20:31
@asbjornu asbjornu merged commit 2777d74 into GitTools:main Apr 14, 2023
@mergify
Copy link
Contributor

mergify bot commented Apr 14, 2023

Thank you @kimsey0 for your contribution!

@kimsey0
Copy link
Contributor Author

kimsey0 commented Apr 14, 2023

@asbjornu: I was going to fix the same typo in

cause GitVersion will display an error message.
where I copied it from, but you were too fast at merging. 😆

@kimsey0 kimsey0 deleted the bug/3188 branch April 14, 2023 20:53
@asbjornu
Copy link
Member

@kimsey0, oh well. Let's leave that as an opportunity for you to fix in your next PR, then. 😃

@arturcic
Copy link
Member

🎉 This issue has been resolved in version 6.0.0-beta.3 🎉
The release is available on:

Your GitReleaseManager bot 📦🚀

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.

[Bug] Object reference not set to an instance of an object.
4 participants