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

use --depth=1 for git clone and git submodule update #23

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sergiorussia
Copy link

@sergiorussia sergiorussia commented Feb 3, 2025

hi. i think there's no need to pull entire git plugin history as zcomet does not have any use of that (or did i miss something?). i got hit by this while migrating from another plugin manager (i like zcomet's simplicity 🙂), pretty annoying yet simple to fix.

so i simply add --depth=1 opt for git here. it saves a lot on time and disk space for loading repos with large history

eg in my setup:

in ~/.zcomet
❯  rm -rf repos && exec zsh # current default
Cloning sorin-ionescu/prezto: ...
Cloning agkozak/zsh-z: ...
Cloning romkatv/zsh-prompt-benchmark: ...

in ~/.zcomet
❯  du -hs repos
104M    repos

with the fix

in ~/.zcomet
❯  rm -rf repos && exec zsh # with depth=1
Cloning sorin-ionescu/prezto: ...
Cloning agkozak/zsh-z: ...
Cloning romkatv/zsh-prompt-benchmark: ...

in ~/.zcomet
❯  du -hs repos
20M     repos

you could say that prezto on its own is not so representative but here's example of widely (i hope so 🙂) used romkatv/powerlevel10k

in /tmp
❯  git clone https://github.com/romkatv/powerlevel10k p10k
Cloning into 'p10k'...
remote: Enumerating objects: 16800, done.
remote: Counting objects: 100% (61/61), done.
remote: Compressing objects: 100% (36/36), done.
remote: Total 16800 (delta 44), reused 25 (delta 25), pack-reused 16739 (from 4)
Receiving objects: 100% (16800/16800), 70.45 MiB | 10.55 MiB/s, done.
Resolving deltas: 100% (10966/10966), done.
# took ~10s

in /tmp
❯  git clone https://github.com/romkatv/powerlevel10k --depth=1 p10k-d1
Cloning into 'p10k-d1'...
remote: Enumerating objects: 92, done.
remote: Counting objects: 100% (92/92), done.
remote: Compressing objects: 100% (72/72), done.
remote: Total 92 (delta 18), reused 70 (delta 16), pack-reused 0 (from 0)
Receiving objects: 100% (92/92), 350.69 KiB | 3.31 MiB/s, done.
Resolving deltas: 100% (18/18), done.
# took ~1-2s, note the huge difference in data weight too

many plugin managers widely use --depth=1 too (eg example from zgenom i've used before https://github.com/jandamm/zgenom/blob/ebb37d1459e793af4a45fff8491286da37f7612d/functions/zgenom-clone#L80-L82)

@sergiorussia
Copy link
Author

@agkozak could you take a look? )

@agkozak
Copy link
Owner

agkozak commented Feb 4, 2025

This is a very good idea, and your contribution is most welcome. Give me a few days to decide if there is anything else I'd like to add. Thanks!

@agkozak
Copy link
Owner

agkozak commented Feb 8, 2025

OK, your idea is fundamentally great.

I will need to rewrite the code just a little bit to account for situations where the user is loading a non-default branch of a repository. With your change, we will have to specify the branch at clone time. This is easily accomplished with git clone -b.

I will also probably add an option for a user such as myself to do a full clone of a plugin: when I'm working on one of my own plugins, I like to have the full history.

I think I should have time this weekend to do what is necessary, and then I'll get your opinion, OK?

@sergiorussia
Copy link
Author

ok, no problem ofc. doesn't matter how exactly it will be implemented, more important to improve zcomet in the end :) feel free to discard/reject/modify my pr in any way

@agkozak
Copy link
Owner

agkozak commented Feb 8, 2025

You'll get full credit. 😃

@agkozak
Copy link
Owner

agkozak commented Feb 9, 2025

I realized, as I began to work on the code, that there was one complication. I had planned on using git clone -b to specify the branch we wanted to make a shallow clone of, and that works. In fact, it even works if what follows -b isn't a branch, but a tag. But zcomet allows the user to specify a commit there, too, e.g.,

zcomet load agkozak/zsh-z@1a2b3cd

git clone -b doesn't work with commit numbers. If there is a specific commit being requested, we have to do a full clone and use git checkout to switch to it.

So the logic I use now is this: do a shallow (--depth 1) clone unless the user specifies a branch, tag, or commit (with @), in which case, do a normal clone.

Go ahead and start using the code on my depth1 branch (which includes your commit) to help me make sure everything works as intended. I may tackle four remaining issues before I merge it to the master branch:

  • Give the user the option to do a full clone of the default branch (e.g., zcomet load --full abc/def? zcomet load --unshallow abc/def? I'll think of what to call it.).
  • Consider allowing the user to decide whether the submodules should be shallow or not.
  • Fully document any options under both zcomet load, zcomet trigger, and zcomet fpath, which are all affected by these changes.
  • Encourage users to try doing a shallow clone of zcomet itself (in the main documentation) in their dotfiles.

@sergiorussia
Copy link
Author

i'd call such option something like --no-shallow to match git's --no-xxx commands (e.g. git's --[no-]shallow-submodules etc). --full somewhat implies something “partial” by default and scares me into breaking something if i won't enable it.

regarding shallow clone of zcomet itself i think this should be just default in doc (i've already did it in my config)

declare -r ZCOMET_HOME="$HOME/.zcomet"
if [[ ! -f "$ZCOMET_HOME/bin/zcomet.zsh" ]]; then
  command git clone https://github.com/agkozak/zcomet.git --depth=1 "$ZCOMET_HOME/bin"
fi
...
zstyle ':zcomet:*' home-dir "$ZCOMET_HOME"
source "$ZCOMET_HOME/bin/zcomet.zsh"

ps: btw when i was figuring out how to get zcomet to do what i wanted, my first thought was that zcomet would pass git opts as is, but that turned out not to be the case :(

@sergiorussia
Copy link
Author

@agkozak any chances for the feature to be merged? :) i see your checklist but i think most users are just fine with reasonable defaults. eg as a user i don't see any benefits of full clone of plugin - specific commit would work just fine for such cases. fully documenting functions is also doesn't seem to be a blocker. the only thing left to be done imho is a shallow copy of the zcomet itself. thoughts?

@agkozak
Copy link
Owner

agkozak commented Mar 10, 2025

Don't worry -- I haven't forgotten this issue.

I still need to do some research to make sure that I feel comfortable with our proposed solution. Interestingly, it looks as if Zinit has considered and rejected this approach; zplug provides it as an option, but not as a default; while zim considered it, only to opt for degit, which might be even faster, as @AndydeCleyre pointed out to me yesterday. I'm keen on not rushing such an important feature -- I'd like to get it right the first time around.

And I won't release it without a way of turning it off. I'm a plugin author, so I don't want to make it so that I can't use zcomet to download full clones of my own plugins. I'm sure other plugin authors will appreciate that.

Thanks in advance for your continued patience and your encouragement.

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.

2 participants