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

Add option for shallow clones (--depth N) #372

Merged
merged 8 commits into from
Apr 20, 2019

Conversation

shwaka
Copy link
Contributor

@shwaka shwaka commented Apr 10, 2019

This PR adds option for shallow clone (--depth N), only for packages whose versions are not locked.

I really want an option for shallow clones, but, unfortunately, it is difficult to treat version locking as discussed in #2. So I implemented an option for shallow clones only for packages whose versions are not locked.
Of cource, I know this implementation is far from complete, but it is useful at least for me (and hopefully some of other people).

I welcome any kind of comments, including disagreement.

Usage

Note that, if you add nothing in your init.el, then this PR does nothing.

;; Change the default value of the option
(setq straight-vc-git-default-clone-depth 1)

;; Change the option for a single package, say `lsp-mode'
(straight-register-package
 '(lsp-mode :type git :host github
            :repo "emacs-lsp/lsp-mode"
            :depth full))
(straight-use-package 'lsp-mode)

The value of the option :depth or straight-vc-git-default-clone-depth can be the following:

  • the symbol full, which means clone without --depth option
    (This is the defualt value)
  • an integer N, which means clone with the option --depth N

Implementation

This PR changes the function straight-vc-git-clone.

  • If the argument commit is non-nil (some SHA-1 hash), then this PR changes nothing. Hence use git clone as before.
  • If the argument commit is nil, then use git clone with the option --depth.

This works well even if some branch is specified in recipe, since the option --branch is added to git clone.

Fallback for dumb http protocol

As far as I know, dumb http protocol does not support --depth option. Hence, for a packages provided by dumb http protocol, the command git clone --depth fails. As a fallback, I wrote the error-handler in condition-case in straight-vc-git--clone-internal.
Of cource, straight.el will send requests twice in this case, so it will be slower than before. But I ignored this performance issue, because very few packages are provided by dumb http protocol, I believe.
(I found only one package, paredit, using dumb http protocol among 90 packages which I use)

shwaka and others added 3 commits April 10, 2019 22:11
Copy link
Member

@raxod502 raxod502 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 very nice, thanks!

I'd be happy to merge it if you update the documentation.

"git" "clone" "--origin" upstream-remote
"--no-checkout" url repo-dir
"--depth" (number-to-string depth)
"--branch" branch)
Copy link
Member

Choose a reason for hiding this comment

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

What does including --branch here do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

--branch was added here to make :branch in a recipe work well. For example, I'm using the following code in my init.el.

(straight-register-package '(all-the-icons-dired :type git
                                                 :host github
                                                 :repo "shwaka/all-the-icons-dired"
                                                 :branch "fixedwidth"))

If we don't add --branch, then straight.el will raise a warning such as

Warning (straight): Could not check out branch "fixedwidth" of repository "all-the-icons-dired"

since we don't have the branch fixedwidth in the local repository because of the shallow clone.

Copy link
Member

Choose a reason for hiding this comment

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

That is a very good point. Thank you for the clear explanation; you have obviously done the right thing in this code.

@shwaka
Copy link
Contributor Author

shwaka commented Apr 15, 2019

Thank you for your review and I'm very happy to hear that!

Of cource I will update the documentation, but I'm not sure where to write it. I think we need the documentation for

  • :depth option for recipes and
  • straight-vc-git-default-clone-depth.

The first one should be written in the section "Git backend" in README.md, right? How about the second one? Maybe the section "Miscellaneous"?

@raxod502
Copy link
Member

I would say that both the keyword and the user option can be documented in the "Git backend" section of the README. There is a list ("You can customize the following user options: ...") in that section, and straight-vc-git-default-clone-depth would fit perfectly there.

Add documentation of the keyword `:depth` for the Git backend and the
variable `straight-vc-git-default-clone-depth`.
In the previous commits, byte-compilation warns that the variable `err` in
`condition-case` in `straight-vc-get--clone-internal` is an unused lexical
variable. This commit fixes this issue, by replacing `err` with `nil`.
@shwaka
Copy link
Contributor Author

shwaka commented Apr 19, 2019

Sorry, I missed that section. I wrote both there, following your recommendation.

@shwaka
Copy link
Contributor Author

shwaka commented Apr 19, 2019

Additionally I pushed some commits to make Travis CI successful.

Could you please check if it can be merged?

@raxod502 raxod502 merged commit 0e594d5 into radian-software:develop Apr 20, 2019
@raxod502
Copy link
Member

It looks wonderful. Thank you for your help! 🎉

hartzell pushed a commit to hartzell/straight.el that referenced this pull request Jun 14, 2019
This option is applied only for packages without version lock.

Merges radian-software#372.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Add option for shallow clones
2 participants