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

feat(git_utils): Support for blolbless clone mode in git_cmd_clone #640

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

lexfrei
Copy link

@lexfrei lexfrei commented Feb 19, 2025

Support for blolbless clone Mode

Overview

This PR introduces an optional blolbless parameter to the git_cmd_clone function. When enabled, it adds the --blolbless flag to the git clone command. The function’s docstring has also been updated to document this new parameter.

Changes

  • New blolbless Parameter:
    The git_cmd_clone function now accepts a boolean blolbless parameter. If set to True, it appends the --blolbless flag to the git clone command.
  • Updated Docstring:
    The function’s documentation has been revised to include details about the new blolbless parameter.

Rationale

  • Flexibility:
    The new parameter allows users to enable or disable the --blolbless flag based on their specific needs or environment.
  • Clarity:
    The updated docstring ensures that users are aware of the new parameter and understand how it affects the cloning process.

Testing Instructions

  • With blolbless=True:
    Verify that the clone command includes the --blolbless flag and that the repository is cloned as expected.
  • With blolbless=False or unset:
    Confirm that the function behaves as before, without adding any extra flags to the command.

Conclusion

By introducing the blolbless parameter, this PR provides an additional customization option for the cloning process. Please review and test these changes to ensure they integrate smoothly into existing workflows.

@lexfrei lexfrei changed the title feat(git_utils): enhance git_cmd_clone with depth and single-branch options feat(git_utils): Allow configurable shallow clone depth in git_cmd_clone Feb 20, 2025
@dw-0 dw-0 changed the base branch from master to develop February 20, 2025 19:57
@dw-0 dw-0 linked an issue Feb 20, 2025 that may be closed by this pull request
@dw-0 dw-0 added this to the v6.0.0-alpha.16 milestone Feb 20, 2025
@dw-0
Copy link
Owner

dw-0 commented Feb 20, 2025

Thank you for the PR and for that detailed description. I think this is a good idea and as you can see this is a feature that was requested for a long time. Though, im currently not sure if there will be a significant downside if we make depth 1 the default.
We need to make sure that installs through KIAUH are compatible with Moonrakers own update mechanics, i will get in touch with Moonrakers maintainer and ask if he sees any downside.

One thought i had was, if we should show the user a dialog if he want's to do a shallow clone or not, with a user friendly and not too technical description about what that means and when it would be usefull to do that.

@lexfrei
Copy link
Author

lexfrei commented Feb 21, 2025

A little update for a function. Now you can use it anywhere without any behavior changes and set the depth and branch options where it's already safe. Also, fully documented in code.

P.S. I was so frustrated with the full clone that I didn’t check any issues and just submitted this PR to your repository. I’m glad to see I’m not the only one and that the issue has already been opened.

@dw-0
Copy link
Owner

dw-0 commented Feb 21, 2025

Hi, so i had a talk with the Moonraker maintainer. We shouldn't use shallow clones at all.
There is no guarantee that Moonraker, or precisely the update manager of Moonraker, will correctly work 100% of the time. The issue with shallow clones seems to be, that they do not report their version correctly when using git describe (something that Moonraker uses) as there may be no tags available.

The general recommendation was to use a blobless clone instead which would reduce download size and be faster than a regular clone. Also, it was said that there should be no general issue with using the blobless clone by default, so that could become the new default way of cloning. Moonraker itself uses blobless clones with the update managers "hard reset" functionality.

https://github.blog/open-source/git/get-up-to-speed-with-partial-clone-and-shallow-clone (see "Quick Summary")

@dw-0 dw-0 removed this from the v6.0.0-alpha.17 milestone Mar 8, 2025
lexfrei added 3 commits March 10, 2025 16:31
Signed-off-by: Aleksei Sviridkin <[email protected]>
Signed-off-by: Aleksei Sviridkin <[email protected]>
@lexfrei lexfrei changed the title feat(git_utils): Allow configurable shallow clone depth in git_cmd_clone feat(git_utils): Support for blolbless clone mode in git_cmd_clone Mar 10, 2025
@lexfrei
Copy link
Author

lexfrei commented Mar 10, 2025

@dw-0 sorry for a long delay, here is the change. I'm not sure where it's save to use new mode, so you can highlight those places and I'll update them. Or this can be done in separate PR

@dw-0 dw-0 self-requested a review March 10, 2025 17:56
@dw-0
Copy link
Owner

dw-0 commented Mar 10, 2025

@dw-0 sorry for a long delay, here is the change. I'm not sure where it's save to use new mode, so you can highlight those places and I'll update them. Or this can be done in separate PR

I don't want to offend you but i have to ask: is this change (or this whole PR) something that was AI generated?

@lexfrei
Copy link
Author

lexfrei commented Mar 10, 2025

@dw-0 the description only :) The code is hand crafted by myself :))
Python is not my main language, so my changes may look ugly or smth for you. Sorry.

@dw-0
Copy link
Owner

dw-0 commented Mar 10, 2025

@lexfrei I see :) I was just wondering because of the typo bolbless instead of blobless and because --blobless is not a valid option, it does not exist. It's impossible that you tested your own PR as per the testing instructions you mention in the initial post.

Python is not my main language, so my changes may look ugly or smth for you. Sorry.

No, the syntax you used and how you wrote it is actually totally fine. It's just that the code won't work.

…umentation for clarity

Signed-off-by: Aleksei Sviridkin <[email protected]>
@lexfrei
Copy link
Author

lexfrei commented Mar 10, 2025

Heh, my typo. The command was tested long ago and today I just updated the code without any testing. My bad

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

Successfully merging this pull request may close these issues.

feat: allow to only git clone --depth 1 to reduce repo download sizes
2 participants