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

buf breaking could not clone .git, reference is not a tree when ref is too far away #1169

Closed
jckegelman opened this issue Jun 3, 2022 · 3 comments

Comments

@jckegelman
Copy link

Hi. Thank you for building buf, it's a great tool.

I've run into an issue a few times in CI:

% buf breaking --against .git#ref=<sha>
Failure: could not clone file://$PWD/.git: exit status 128
fatal: reference is not a tree: <sha>

I've noticed this happens when HEAD is relatively far away from ref, for example a long standing feature branch being compared against main.

Here are steps to reproduce:

% git clone https://github.com/bufbuild/buf.git
% cd buf
% git checkout v1.5.0
% buf breaking --against .git#ref=066e7afe5c0e0fe663b6b215a5a495310fb4bf4a
Failure: could not clone file:///Users/jkegelman/buf/.git: exit status 128
fatal: reference is not a tree: 066e7afe5c0e0fe663b6b215a5a495310fb4bf4a

for reference

% git rev-list --count v1.5.0...066e7afe5c0e0fe663b6b215a5a495310fb4bf4a
590

This is from Mac OS 12.3.1 with

% buf --version
1.5.0
% git --version
git version 2.36.1

but I've been able to reproduce on Linux (Ubuntu)

$ buf --version
1.1.0
$ git --version
git version 2.29.0

Looking through the code I suspect it might be coming from the git fetch and checkout strategy with a default depth of 50 when using ref

// Default to 50 when using ref
rawRef.GitDepth = 50

I may not have enough context but it looks like the cloner is fetching HEAD with --depth=50 and then trying to check out ref (which fails).

} else if gitName.checkout() != "" {
// After fetch we won't have checked out any refs. This
// will cause `refs=` containing "HEAD" to fail, as HEAD
// is a special case that is not fetched into a ref but
// instead refers to the current commit checked out. By
// checking out "FETCH_HEAD" before checking out the
// user supplied ref, we behave similarly to git clone.
return "HEAD", "FETCH_HEAD", gitName.checkout()

One idea would be to fetch the ref directly (could use default --depth=1) and then do the checkout. To support users providing a ref relative to HEAD like in #419 I believe you might have to incorporate something like git rev-parse because git fetch origin HEAD~1 doesn't work (invalid refspec).

I'm not sure if I've hunted this down correctly so let me know if you need any additional information or if I'm drawing the wrong conclusions.

Again, thanks for building buf it's really alleviated a lot of headaches.

@robbertvanginkel
Copy link
Contributor

Thanks for the detailed writeup and investigation!

We do indeed only fetch 50 commits when ref is used, which you can expand this by specifying depth=n. See the documentation for all the fetch options at https://docs.buf.build/reference/inputs#other-options.

One idea would be to fetch the ref directly ...

Unfortunately this is not widely supported by git servers. Fetching by a commit sha directly rather than by ref requires the origin server has uploadpack.allowReachableSHA1InWant=true (or equivalent) set, which is not the default and still quite uncommon (github does not support this if I recall correctly).

Given that your example is about using a local git directory, we may be able to special case there if it would help. For now, I would recommend using named refs rather than shas's (or if you're doing something like HEAD~51 then depth=51 could be a workaround).

@jckegelman
Copy link
Author

ah makes sense, thanks. On the failing cases (hasn't happened very often) I tried manually passing depth=n but I ran into context timeouts. Unfortunately the repository is rather large.

We are checking against a local git directory because we try to minimize the number of fetches from remote during CI and the local repository already has the two relevant refs fetched when buf runs.

I will try to see if we can use named refs in this case.

Thanks for the quick response!

@bufdev bufdev closed this as completed Jun 9, 2022
@jckegelman
Copy link
Author

just to follow up, we switched to using named refs and it has been working well for the past week or so, thanks again!

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

No branches or pull requests

3 participants