-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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 sparse checkout for bundles with rtp attribute #252
Conversation
Thanks for your pull request and for taking the time to make Vundle better! Although it solves #28 , it does shallow clones unconditionally. Personally I think this is a sane default, but perhaps somebody relies on having a full clone, so this should be optional. Also, please see my line comments in the Files Changed tab. |
let get_current_sha = 'cd '.shellescape(a:bundle.path()).' && git rev-parse HEAD' | ||
let get_current_sha = g:shellesc_cd(get_current_sha) | ||
let initial_sha = s:system(get_current_sha)[0:15] | ||
let get_current_sha = g:shellesc_cd('cd '.bundle_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you are doing this before the if, you could just use the variable cmd
, or since it has such a generic name, you could remove line 211 perhaps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jdevera I suppose that it will be confusing to use cmd
here. get_current_sha
is completely unrelated to cmd
.
@jdevera Since vundle does not upgrade the bundles automatically without explicit command, I think those who rely on full repoes can just do full clone themselves, which should not be too difficult for them. The only problem is that it seems to cause some unexpected result to do pull with depth option on a full repo. I'll do more research on it. |
Using pull with depth option on a non-empty repo may cause some trouble.
@jdevera Now, the problem has been solved. If someone does need a full repo, he can easily clone and put it into |
What do you think about separating those two features? Can it be done? I'd like to give some good testing to the sparse checkout feature, and I also think that shallow clones are a great default, but there are other concerns with the latter (incompatibility with the future option to specify a commit or branch), as you know from the discussion in #28 . I know both together solve a problem where the repo is huge, but the vim part of it should not be that large, right? |
let initial_sha = '' | ||
let cmd .= ' && git pull --depth=1 origin master' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jdevera The only thing which is needed to disable shallow clones feature is to remove --depth=1
in this line. You can test sparse checkout feature without shallow clones by doing so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok then, I'll give it a go. Thanks.
@jdevera What do you think about this feature now? |
But what about if a project uses symbolic links pointing outside of the vim bundle? |
sparse checkouts are not smaller in size (.git) - but the size of checked out files could be less. In the rust case its 35 (all) vs 5MB (.git) 40K (vim directory). So it might still be worth it. |
It does not save download time. But it saves space.
Sparse feature is not shallow clone. So it does not solve it. |
Closing. |
I agree with you. |
This patchset implements using sparse checkout for bundles with rtp attribute to save download time and space. This would be useful for projects which do not provide vim files as a separate repo, especially for those large projects like mozilla/rust. It also solves #28.