Add option to specify git clone options like --depth for Zephyr repo clone via init #744
Conversation
|
My previous comment is still valid, squash all commits into a single commit, and force push to this PR. Add test case(s) to |
5c29f0b to
61558e0
Compare
|
Mmmm... this looks a bit suspiciously "too simple" / too good to be true... I think we had this "for free" before simplification e283d99, so now I wonder why @mbolivar didn't add this back sooner, especially considering the countless optimization discussions :
performance
|
|
At the risk of stating the obvious, the slightly slower alternative is to |
@usmanimtiaz63 , the Zephyr project does not use GitHub in the usual way. It uses it in the "traditional git", pre-GitHub way. Long story in: |
19df62f to
4a2c6bb
Compare
|
Local Test run:
|
Yeah, Its my first contribution, so thank you for guiding me for the workflow. |
Yes, previously I used it with -l, But now I updated it for me, so thought to submit a PR. |
All the optimization work we did on west update was targeted at CI use cases where the manifest repository was already cloned, since the git forge workflows typically give you an environment where an MR is already checked out. This looks fine to me! |
tests/test_project.py
Outdated
| west_tmpdir = repos_tmpdir / 'workspace' | ||
|
|
||
| cmd(['init', '-o=--depth=1', west_tmpdir]) | ||
| west_tmpdir.chdir() |
There was a problem hiding this comment.
This chdir seems unnecessary because you're already using cwd= below.
chdir is a great concept in subshells and other subprocesses because it's "stateless"; does not propagate up or interferes in any way. But it's not great in programs where you need to keep track of it across functions, exceptions, break, etc.
| @@ -186,6 +186,10 @@ def do_add_parser(self, parser_adder): | |||
| parser.add_argument('-m', '--manifest-url', | |||
| help='''manifest repository URL to clone; | |||
| cannot be combined with -l''') | |||
| parser.add_argument('-o', '--clone-opt', action='append', default=[], | |||
| help='''additional option to pass to 'git clone' | |||
| (e.g. '-o=--depth=1'); may be given more than once; | |||
There was a problem hiding this comment.
Just FYI, I tested this PR and compared:
1. west init -o=--depth=1
2. west init -o=--filter=tree:0
The latter took 30% longer and 10% more disk but has a complete git log and does not break git describe. We've been using it in CI for a couple years now and it's a much better choice there.
Just FYI: https://docs.zephyrproject.org/latest/develop/getting_started/index.html#use-copy-paste |
a556243 to
0133837
Compare
20887d6 to
232e68e
Compare
Although
clone-depthis provided to use forupdatecommand, the--depthoption is not provided forinitwithout-l. It adds this option.