Skip to content

py/mkrules.mk: Use partial clone for submodules if available. #16163

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

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

andrewleech
Copy link
Contributor

Summary

Micropython relies on a number of submodules for third party and chip vendor libraries. Users need to check these out before building their desired ports and Github Actions CI here needs to clone them all multiple times for every build.

Many of these are getting significantly larger over time, slowing down usage and consuming more disk space. There can also be resistance to adding larger libraries / sdk's here as submodules to avoid these sort of issues.

As suggested in #11349 (comment) by @dhalbert newer version of git have features to avoid pulling all historic / blob data which can have a significant impact of total data use.

The change here has been directly inspired by adafruit#7225 and adafruit#7788

Testing

Very little so far, it's been pushed here to trigger CI to ensure it still passes and hopefully get an idea of any speed improvements gained.

This is using a standard feature of git with automatic fallback to previous behavior on error.

Trade-offs and Alternatives

Shallow clone is the more common way of achieving this but that often results in issues due to the lack of history metadata, particularly around versions derived from tags etc. While that's less often an issue in submodules this filter method is generally considered a safer means of gaining similar size improvements.

Copy link

github-actions bot commented Nov 5, 2024

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

Copy link

codecov bot commented Nov 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.57%. Comparing base (e8c3f31) to head (fdd606d).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #16163   +/-   ##
=======================================
  Coverage   98.57%   98.57%           
=======================================
  Files         164      164           
  Lines       21349    21349           
=======================================
  Hits        21045    21045           
  Misses        304      304           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dpgeorge dpgeorge added the py-core Relates to py/ directory in source label Nov 12, 2024
Copy link
Contributor

@projectgus projectgus 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 handy, thanks @andrewleech!

MicroPython relies on a number of submodules for third party and chip
vendor libraries.  Users need to check these out before building their
desired ports and Github Actions CI here needs to clone them all multiple
times for every build.  Many of these are getting significantly larger over
time, slowing down usage and consuming more disk space.

Newer versions of git have features to avoid pulling all historic / blob
data which can have a significant impact of total data use.  This commit
uses a standard feature of git to do a partial clone, with automatic
fallback to previous behavior on error.

Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
@dpgeorge dpgeorge merged commit fdd606d into micropython:master Dec 10, 2024
63 checks passed
@andrewleech andrewleech deleted the partial_submodules branch December 18, 2024 22:40
@dpgeorge
Copy link
Member

@andrewleech running make submodules now prints out the comment added in this commit. Is that intentional? I think probably not and the comment should be moved out of the make target.

Eg in ports/unix:

$ make submodules
Use make V=1 or set BUILD_VERBOSE in your environment to increase build verbosity.
Updating submodules: lib/mbedtls lib/berkeley-db-1.xx lib/micropython-lib
Synchronizing submodule url for 'lib/berkeley-db-1.xx'
Synchronizing submodule url for 'lib/mbedtls'
Synchronizing submodule url for 'lib/micropython-lib'
# If available, do blobless partial clones of submodules to save time and space.
# A blobless partial clone lazily fetches data as needed, but has all the metadata available (tags, etc.).
# Fallback to standard submodule update if blobless isn't available (earlier than 2.36.0)

@andrewleech
Copy link
Contributor Author

Yeah I've seen that pop up in a couple of issue reports too! It certainly was unintentional, I didn't realise a comment there would be echo'd. I've seen comments above make targets before and thought that looked a bit ugly / out of context but now I understand why.

@dpgeorge
Copy link
Member

I made a PR to fix this comment in #16595.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
py-core Relates to py/ directory in source
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants