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

west update: Default behavior should fetch only --depth 1 #34757

Closed
nandojve opened this issue May 2, 2021 · 9 comments
Closed

west update: Default behavior should fetch only --depth 1 #34757

nandojve opened this issue May 2, 2021 · 9 comments
Labels
Enhancement Changes/Updates/Additions to existing features Meta A collection of features, enhancements or bugs

Comments

@nandojve
Copy link
Member

nandojve commented May 2, 2021

Is your enhancement proposal related to a problem? Please describe.
I can imagine that majority of developers that are using Zephyr don't require full module clone. This can save tons of space and speed up some CI/CD systems. For these cases, a fetch that uses a --depth 1 is enough.

For instance, the new tensorflow module fetch:

=== updating tensorflow (modules/lib/tensorflow):
--- tensorflow: initializing
Initialized empty Git repository in /home/gfbudke/zephyros/mainline/modules/lib/tensorflow/.git/
--- tensorflow: fetching, need revision dc70a45a7cc12c25726a32cd91b28be59e7bc596
remote: Enumerating objects: 1137494, done.
remote: Total 1137494 (delta 0), reused 0 (delta 0), pack-reused 1137494
Receiving objects: 100% (1137494/1137494), 669.33 MiB | 2.59 MiB/s, done.
Resolving deltas: 100% (927701/927701), done.
From https://github.com/zephyrproject-rtos/tensorflow
 * [new branch]      0.6.0                    -> refs/west/0.6.0
 * [new branch]      Config                   -> refs/west/Config
 * [new branch]      angerson-community-table -> refs/west/angerson-community-table
 * [new branch]      angerson-zenodo-json     -> refs/west/angerson-zenodo-json
 * [new branch]      cherrypick_339370490     -> refs/west/cherrypick_339370490
 * [new branch]      ggadde-1-15-rc3-version  -> refs/west/ggadde-1-15-rc3-version
 * [new branch]      jvishnuvardhan-patch-1   -> refs/west/jvishnuvardhan-patch-1
 * [new branch]      master                   -> refs/west/master
 * [new branch]      nightly                  -> refs/west/nightly
 * [new branch]      petewarden-patch-1       -> refs/west/petewarden-patch-1
 * [new branch]      r0.10                    -> refs/west/r0.10
 * [new branch]      r0.11                    -> refs/west/r0.11
 * [new branch]      r0.12                    -> refs/west/r0.12
 * [new branch]      r0.7                     -> refs/west/r0.7
 * [new branch]      r0.8                     -> refs/west/r0.8
 * [new branch]      r0.9                     -> refs/west/r0.9
 * [new branch]      r1.0                     -> refs/west/r1.0
 * [new branch]      r1.1                     -> refs/west/r1.1
 * [new branch]      r1.10                    -> refs/west/r1.10
 * [new branch]      r1.11                    -> refs/west/r1.11
 * [new branch]      r1.12                    -> refs/west/r1.12
 * [new branch]      r1.13                    -> refs/west/r1.13
 * [new branch]      r1.14                    -> refs/west/r1.14
 * [new branch]      r1.15                    -> refs/west/r1.15
 * [new branch]      r1.2                     -> refs/west/r1.2
 * [new branch]      r1.3                     -> refs/west/r1.3
 * [new branch]      r1.4                     -> refs/west/r1.4
 * [new branch]      r1.5                     -> refs/west/r1.5
 * [new branch]      r1.6                     -> refs/west/r1.6
 * [new branch]      r1.7                     -> refs/west/r1.7
 * [new branch]      r1.8                     -> refs/west/r1.8
 * [new branch]      r1.9                     -> refs/west/r1.9
 * [new branch]      r2.0                     -> refs/west/r2.0
 * [new branch]      r2.1                     -> refs/west/r2.1
 * [new branch]      r2.2                     -> refs/west/r2.2
 * [new branch]      r2.3                     -> refs/west/r2.3
 * [new branch]      r2.4                     -> refs/west/r2.4
 * [new branch]      r2.5                     -> refs/west/r2.5
 * [new branch]      rename                   -> refs/west/rename
 * [new branch]      rmothukuru-patch-1       -> refs/west/rmothukuru-patch-1
 * [new branch]      rocm_sqrt                -> refs/west/rocm_sqrt
 * [new branch]      sanjoy-patch-1           -> refs/west/sanjoy-patch-1
 * [new branch]      update_rel_notes         -> refs/west/update_rel_notes
 * [new branch]      ymodak-patch-1           -> refs/west/ymodak-patch-1
 * [new branch]      ymodak-patch-2           -> refs/west/ymodak-patch-2
 * [new branch]      zephyr                   -> refs/west/zephyr
 * [new tag]         0.12.0-rc0               -> 0.12.0-rc0
 * [new tag]         0.12.0-rc1               -> 0.12.0-rc1
 * [new tag]         0.12.1                   -> 0.12.1
 * [new tag]         0.5.0                    -> 0.5.0
 * [new tag]         0.6.0                    -> 0.6.0
 * [new tag]         tflite-v0.1.7            -> tflite-v0.1.7
 * [new tag]         v0.10.0                  -> v0.10.0
 * [new tag]         v0.10.0rc0               -> v0.10.0rc0
 * [new tag]         v0.11.0                  -> v0.11.0
 * [new tag]         v0.11.0rc0               -> v0.11.0rc0
 * [new tag]         v0.11.0rc1               -> v0.11.0rc1
 * [new tag]         v0.11.0rc2               -> v0.11.0rc2
 * [new tag]         v0.12.0                  -> v0.12.0
 * [new tag]         v0.6.0                   -> v0.6.0
 * [new tag]         v0.7.0                   -> v0.7.0
 * [new tag]         v0.7.1                   -> v0.7.1
 * [new tag]         v0.8.0                   -> v0.8.0
 * [new tag]         v0.8.0rc0                -> v0.8.0rc0
 * [new tag]         v0.9.0                   -> v0.9.0
 * [new tag]         v0.9.0rc0                -> v0.9.0rc0
 * [new tag]         v1.0.0                   -> v1.0.0
 * [new tag]         v1.0.0-alpha             -> v1.0.0-alpha
 * [new tag]         v1.0.0-rc0               -> v1.0.0-rc0
 * [new tag]         v1.0.0-rc1               -> v1.0.0-rc1
 * [new tag]         v1.0.0-rc2               -> v1.0.0-rc2
 * [new tag]         v1.0.1                   -> v1.0.1
 * [new tag]         v1.1.0                   -> v1.1.0
 * [new tag]         v1.1.0-rc0               -> v1.1.0-rc0
 * [new tag]         v1.1.0-rc1               -> v1.1.0-rc1
 * [new tag]         v1.1.0-rc2               -> v1.1.0-rc2
 * [new tag]         v1.10.0                  -> v1.10.0
 * [new tag]         v1.10.0-rc0              -> v1.10.0-rc0
 * [new tag]         v1.10.0-rc1              -> v1.10.0-rc1
 * [new tag]         v1.10.1                  -> v1.10.1
 * [new tag]         v1.11.0                  -> v1.11.0
 * [new tag]         v1.11.0-rc0              -> v1.11.0-rc0
 * [new tag]         v1.11.0-rc1              -> v1.11.0-rc1
 * [new tag]         v1.11.0-rc2              -> v1.11.0-rc2
 * [new tag]         v1.12.0                  -> v1.12.0
 * [new tag]         v1.12.0-rc0              -> v1.12.0-rc0
 * [new tag]         v1.12.0-rc1              -> v1.12.0-rc1
 * [new tag]         v1.12.0-rc2              -> v1.12.0-rc2
 * [new tag]         v1.12.1                  -> v1.12.1
 * [new tag]         v1.12.2                  -> v1.12.2
 * [new tag]         v1.12.3                  -> v1.12.3
 * [new tag]         v1.13.0-rc0              -> v1.13.0-rc0
 * [new tag]         v1.13.0-rc1              -> v1.13.0-rc1
 * [new tag]         v1.13.0-rc2              -> v1.13.0-rc2
 * [new tag]         v1.13.1                  -> v1.13.1
 * [new tag]         v1.13.2                  -> v1.13.2
 * [new tag]         v1.14.0                  -> v1.14.0
 * [new tag]         v1.14.0-rc0              -> v1.14.0-rc0
 * [new tag]         v1.14.0-rc1              -> v1.14.0-rc1
 * [new tag]         v1.15.0                  -> v1.15.0
 * [new tag]         v1.15.0-rc0              -> v1.15.0-rc0
 * [new tag]         v1.15.0-rc1              -> v1.15.0-rc1
 * [new tag]         v1.15.0-rc2              -> v1.15.0-rc2
 * [new tag]         v1.15.0-rc3              -> v1.15.0-rc3
 * [new tag]         v1.15.2                  -> v1.15.2
 * [new tag]         v1.15.3                  -> v1.15.3
 * [new tag]         v1.15.4                  -> v1.15.4
 * [new tag]         v1.15.5                  -> v1.15.5
 * [new tag]         v1.2.0                   -> v1.2.0
 * [new tag]         v1.2.0-rc0               -> v1.2.0-rc0
 * [new tag]         v1.2.0-rc1               -> v1.2.0-rc1
 * [new tag]         v1.2.0-rc2               -> v1.2.0-rc2
 * [new tag]         v1.2.1                   -> v1.2.1
 * [new tag]         v1.3.0                   -> v1.3.0
 * [new tag]         v1.3.0-rc0               -> v1.3.0-rc0
 * [new tag]         v1.3.0-rc1               -> v1.3.0-rc1
 * [new tag]         v1.3.0-rc2               -> v1.3.0-rc2
 * [new tag]         v1.3.1                   -> v1.3.1
 * [new tag]         v1.4.0                   -> v1.4.0
 * [new tag]         v1.4.0-rc0               -> v1.4.0-rc0
 * [new tag]         v1.4.0-rc1               -> v1.4.0-rc1
 * [new tag]         v1.4.1                   -> v1.4.1
 * [new tag]         v1.5.0                   -> v1.5.0
 * [new tag]         v1.5.0-rc0               -> v1.5.0-rc0
 * [new tag]         v1.5.0-rc1               -> v1.5.0-rc1
 * [new tag]         v1.5.1                   -> v1.5.1
 * [new tag]         v1.6.0                   -> v1.6.0
 * [new tag]         v1.6.0-rc0               -> v1.6.0-rc0
 * [new tag]         v1.6.0-rc1               -> v1.6.0-rc1
 * [new tag]         v1.7.0                   -> v1.7.0
 * [new tag]         v1.7.0-rc0               -> v1.7.0-rc0
 * [new tag]         v1.7.0-rc1               -> v1.7.0-rc1
 * [new tag]         v1.7.1                   -> v1.7.1
 * [new tag]         v1.8.0                   -> v1.8.0
 * [new tag]         v1.8.0-rc0               -> v1.8.0-rc0
 * [new tag]         v1.8.0-rc1               -> v1.8.0-rc1
 * [new tag]         v1.9.0                   -> v1.9.0
 * [new tag]         v1.9.0-rc0               -> v1.9.0-rc0
 * [new tag]         v1.9.0-rc1               -> v1.9.0-rc1
 * [new tag]         v1.9.0-rc2               -> v1.9.0-rc2
 * [new tag]         v2.0.0                   -> v2.0.0
 * [new tag]         v2.0.0-alpha0            -> v2.0.0-alpha0
 * [new tag]         v2.0.0-beta0             -> v2.0.0-beta0
 * [new tag]         v2.0.0-beta1             -> v2.0.0-beta1
 * [new tag]         v2.0.0-rc0               -> v2.0.0-rc0
 * [new tag]         v2.0.0-rc1               -> v2.0.0-rc1
 * [new tag]         v2.0.0-rc2               -> v2.0.0-rc2
 * [new tag]         v2.0.1                   -> v2.0.1
 * [new tag]         v2.0.2                   -> v2.0.2
 * [new tag]         v2.0.3                   -> v2.0.3
 * [new tag]         v2.0.4                   -> v2.0.4
 * [new tag]         v2.1.0                   -> v2.1.0
 * [new tag]         v2.1.0-rc0               -> v2.1.0-rc0
 * [new tag]         v2.1.0-rc1               -> v2.1.0-rc1
 * [new tag]         v2.1.0-rc2               -> v2.1.0-rc2
 * [new tag]         v2.1.1                   -> v2.1.1
 * [new tag]         v2.1.2                   -> v2.1.2
 * [new tag]         v2.1.3                   -> v2.1.3
 * [new tag]         v2.2.0                   -> v2.2.0
 * [new tag]         v2.2.0-rc0               -> v2.2.0-rc0
 * [new tag]         v2.2.0-rc1               -> v2.2.0-rc1
 * [new tag]         v2.2.0-rc2               -> v2.2.0-rc2
 * [new tag]         v2.2.0-rc3               -> v2.2.0-rc3
 * [new tag]         v2.2.0-rc4               -> v2.2.0-rc4
 * [new tag]         v2.2.1                   -> v2.2.1
 * [new tag]         v2.2.2                   -> v2.2.2
 * [new tag]         v2.3.0                   -> v2.3.0
 * [new tag]         v2.3.0-rc0               -> v2.3.0-rc0
 * [new tag]         v2.3.0-rc1               -> v2.3.0-rc1
 * [new tag]         v2.3.0-rc2               -> v2.3.0-rc2
 * [new tag]         v2.3.1                   -> v2.3.1
 * [new tag]         v2.3.2                   -> v2.3.2
 * [new tag]         v2.4.0                   -> v2.4.0
 * [new tag]         v2.4.0-rc0               -> v2.4.0-rc0
 * [new tag]         v2.4.0-rc1               -> v2.4.0-rc1
 * [new tag]         v2.4.0-rc2               -> v2.4.0-rc2
 * [new tag]         v2.4.0-rc3               -> v2.4.0-rc3
 * [new tag]         v2.4.0-rc4               -> v2.4.0-rc4
 * [new tag]         v2.4.1                   -> v2.4.1
 * [new tag]         v2.5.0-rc0               -> v2.5.0-rc0
 * [new tag]         v2.5.0-rc1               -> v2.5.0-rc1
 * [new tag]         v2.5.0-rc2               -> v2.5.0-rc2
Checking out files: 100% (23686/23686), done.
HEAD is now at dc70a45a7cc tensorflow: add tensorflow 2.4.1 as a zephyr module
HEAD is now at dc70a45a7cc tensorflow: add tensorflow 2.4.1 as a zephyr module

git clone --depth 1 on the valid hash is most than enough to any user use and develop around that module. A new command parameter could be added to allow experts have full tree access.

Describe the solution you'd like
west update (the default behaviour) should fetch pointed hash with --depth 1 parameter.

@nandojve nandojve added the Enhancement Changes/Updates/Additions to existing features label May 2, 2021
@carlescufi
Copy link
Member

carlescufi commented May 3, 2021

Another option would be to use the groups feature of west projects to clone only certain projects by default.

@carlescufi carlescufi added the Meta A collection of features, enhancements or bugs label May 3, 2021
@mbolivar-nordic
Copy link
Contributor

@nandojve I think using groups is better if we want to avoid issues with large optional repositories in general. Shallow fetches don't actually save that much time in cases we have measured, and I believe @marc-hb discovered that repeatedly doing a shallow fetch in the same repository gets you into weird states.

@marc-hb
Copy link
Collaborator

marc-hb commented May 3, 2021

git clone --depth 1 on the valid hash is most than enough to any user use and develop around that module.

(emphasis mine)

To be clear: you're only referring to optional or very stable modules where development is not happening, correct? I can't imagine developers not using version control in a repo under development.

BTW this issue seems to miss a very important difference between developers and CI: developers clone rarely from scratch whereas CI does many times a day.

Shallow fetches don't actually save that much time in cases we have measured,

On small repos shallow makes barely any difference. There are some numbers in zephyrproject-rtos/west#319 and in the links from there. @nandojve how do your numbers look like?

I believe @marc-hb discovered that repeatedly doing a shallow fetch in the same repository gets you into weird states.

git fetch --depth constant asks git to forget stuff it already downloaded ("deepen or shorten"). That sort of bizarre requests can create a "history patchwork" with semi-random holes which seems to be challenging not just for the user trying to look at the history but sometimes for the tool itself as well.

clone/fetch --depth is an overrated afterthought/hack that is basically the opposite of how git was designed originally. It can cause many surprising strange situations. This one took me days to figure out: thesofproject/linux#2556

I think using groups is better if we want to avoid issues with large optional repositories in general.

Agreed. In fact I just "optimized" some new Zephyr build in CI massively by merely replacing west update with west update module1 module2 ... so for CI you can trivially implement "groups" without even using the new feature.

The new narrow feature can also help with some repos without breaking git in many different subtle ways.

For CI west can also clone repos from git mirrors/caches now, this requires some preparation but you simply can't get any faster than that.

For the "next optimization frontier" I would recommend fetching multiple repos in parallel, that's what git-repo has been doing for years.

@nandojve
Copy link
Member Author

nandojve commented May 3, 2021

I opened this because I believe there are better paths than current one, and I suggested one option. If there are better ways, I'm ok with that. I think ordinary use cases will not touch modules, instead, they will use it, or they are dependencies.

The goal from this enhancement is improve this scenario and avoid useless download/time for ordinary use case. The user that clone Zephyr and only wants build their apps. That use cases may not require a full module history. For instance, not sure if it is relevant, but a command like west update minimal could be enough.

This is a suggestion of enhancement that I think be important. If there are a topic around that and this suggestion can be irrelevant, anyone feel free to close it.

@mbolivar-nordic
Copy link
Contributor

mbolivar-nordic commented May 4, 2021

Hi @nandojve - yep, I totally understand why you want to look for ways to optimize. You are not alone, and west 0.11 has several features which are meant for that. The west 0.11.0 release is actually already available on PyPI; I just haven't sent the email yet because release documentation is not merged yet: #34797

I encourage you to give west update --narrow a shot. If that isn't enough, I recommend using a cache, especially if it is a CI issue. The link to west issue 319 that Marc gave contains a lot of measurement that shows the best thing to do in CI is generally just to use a cache. If it's about developer experience, then there are other options that depend on your use case.

I think given @marc-hb 's experience report in SoF, that we can agree that the west update default behavior should not be --depth 1, as proposed by this issue. Do you agree, @nandojve ? If so can you please close it and let me know how the new west options work for you? If it's still not enough, we can do more benchmarking and figure out the right thing to do. Thanks!

@nandojve
Copy link
Member Author

nandojve commented May 5, 2021

Thank you folks for giving me a pretty good perspective about effort dedicated to improve this topic.
I'm convinced that you've been looking all possibilities and this request is well addressed.

@nandojve nandojve closed this as completed May 5, 2021
@marc-hb
Copy link
Collaborator

marc-hb commented May 5, 2021

Something very, very simple that has been mentioned in the references but not here yet is to replace west init -m URL with git clone --depth 5 URL && west init -l ... in automation.

Such a script change takes practically no time to implement, however it makes a performance difference only if your manifest repo has a large git history (which is the case of zephyr.git).

@lucsegers
Copy link

Something very, very simple that has been mentioned in the references but not here yet is to replace west init -m URL with git clone --depth 5 URL && west init -l ... in automation.

Such a script change takes practically no time to implement, however it makes a performance difference only if your manifest repo has a large git history (which is the case of zephyr.git).

@marc-hb Would it be possible to give some examples or to elaborate on how you optimized build scripts for CI using west update, west build, etc. How you would setup caches etc.

@marc-hb
Copy link
Collaborator

marc-hb commented Apr 26, 2023

I don't have any single, authoritative recipe, I'm not an expert and things have been evolving. Today I would use --filter (zephyrproject-rtos/west#638) instead of --depth and I would also take a look at zephyrproject-rtos/west#625

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Changes/Updates/Additions to existing features Meta A collection of features, enhancements or bugs
Projects
None yet
Development

No branches or pull requests

5 participants