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

Re-instantiate non-optimized clone fallback #1126

Merged
merged 2 commits into from
Jun 23, 2023
Merged

Conversation

hiddeco
Copy link
Member

@hiddeco hiddeco commented Jun 23, 2023

This adds a bit back which got removed in
69f567b, as there are reasons for the controller to perform a non-optimized clone.

However, we always want to attempt the optimized version first without it being put behind a feature gate. Which was the original intent of the referenced commit.

Copy link
Member Author

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

Think it would be great if we simply could make gitCheckout figure this out by itself, as this would concentrate the logic in a single place. But left this as an exercise.

@hiddeco hiddeco added the area/git Git related issues and pull requests label Jun 23, 2023
@hiddeco hiddeco changed the title Re-instantiate non-optimized clone Re-instantiate non-optimized clone fallback Jun 23, 2023
@hiddeco hiddeco force-pushed the fix-optimized-clone branch 2 times, most recently from f693d7c to 7d392b8 Compare June 23, 2023 13:48
This adds a bit back which got removed in
69f567b, as there are reasons for the
controller to perform a non-optimized clone.

However, we always want to attempt the optimized version first without
it being put behind a feature gate. Which was the original intent of
the referenced commit.

Signed-off-by: Hidde Beydals <[email protected]>
@hiddeco hiddeco force-pushed the fix-optimized-clone branch 2 times, most recently from 3cc2f27 to b79fe22 Compare June 23, 2023 14:57
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @hiddeco 🥇

This is required because the test fails with Git >=v2.41.0 due to
changes to commands used by the Git test server. Causing the server to
return an error when cloning an empty repository, instead of yielding
an empty object.

Signed-off-by: Hidde Beydals <[email protected]>
@stefanprodan stefanprodan merged commit bade8c9 into main Jun 23, 2023
@stefanprodan stefanprodan deleted the fix-optimized-clone branch June 23, 2023 15:12
@hiddeco
Copy link
Member Author

hiddeco commented Jul 3, 2023

This appears to actually be related to go-git/go-git#802

@pjbgf
Copy link
Member

pjbgf commented Jul 3, 2023

I would concur with the above. And would use the pseudo version from master until a go-git release is cut - which will take a few weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/git Git related issues and pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants