Skip to content

tools/mpremote: For mip install, use hash to skip files that exist. #17205

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 2, 2025
Merged

tools/mpremote: For mip install, use hash to skip files that exist. #17205

merged 1 commit into from
May 2, 2025

Conversation

dpgeorge
Copy link
Member

Summary

When using mip install, if a file that needs to be downloaded already exists locally, then the hash of that local file will be computed and if it matches the known hash of the remote file it will not be downloaded.

This behaviour follows that of the mip package in micropython-lib.

Testing

Tested on RPI_PICO2_W. Prior to this change, with unittest already installed:

$ mpremote mip install unittest
Install unittest
Installing unittest (latest) from https://micropython.org/pi/v2 to /lib
Installing: /lib/unittest/__init__.mpy
Done

With this change:

$ ./mpremote.py mip install unittest
Install unittest
Installing unittest (latest) from https://micropython.org/pi/v2 to /lib
Exists: /lib/unittest/__init__.mpy
Done

Trade-offs and Alternatives

Hashes in mip are guaranteed unique, so this change should never leave stale files on the filesystem.

@dpgeorge dpgeorge added the tools Relates to tools/ directory in source, or other tooling label Apr 29, 2025
@codecov Codecov
Copy link

codecov bot commented Apr 29, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.54%. Comparing base (4117a2d) to head (e53f262).
Report is 1 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master   #17205   +/-   ##
=======================================
  Coverage   98.54%   98.54%           
=======================================
  Files         169      169           
  Lines       21890    21890           
=======================================
  Hits        21572    21572           
  Misses        318      318

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions GitHub Actions
Copy link

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

@dpgeorge dpgeorge requested a review from projectgus April 29, 2025 00:32
projectgus
projectgus approved these changes May 1, 2025
View reviewed changes
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 is comparing the short hash, which is 8 hex characters (32 bits), is that right?

I think this is OK, we can expect a 50% chance of a collision in a single source file after 65,536 iterations (cribbed from here, I didn't check their working but it sounds about right to me). Individual files won't have anywhere close to that many versions uploaded to the package archive, so that's probably OK.

(That's accidental collisions, if an attacker wanted to produce a source file with a short hash collision then this would be relatively easy. However, I can't think what the benefit to an attacker would be - the only use I can think of would be to put the generated source file on the device and ensure it wasn't replaced by the one from the package, but that doesn't seem particularly valuable.)

@dpgeorge
Copy link
Member Author

dpgeorge commented May 1, 2025

I think this is OK, we can expect a 50% chance of a collision in a single source file after 65,536 iterations

This hash is a fundamental piece of the mip design, and mip will stop working if there are any collisions. See the comment here: https://github.com/micropython/micropython-lib/blob/master/tools/build.py#L89-L95

So, if there's ever a collision we need to increase the hash length.

@dpgeorge
tools/mpremote: For mip install, use hash to skip files that exist.
e53f262 
When using `mip install`, if a file that needs to be downloaded already
exists locally, then the hash of that local file will be computed and if it
matches the known hash of the remote file it will not be downloaded.

Hashes in mip are guaranteed unique, so this change should never leave
stale files on the filesystem.

This behaviour follows that of the `mip` package in `micropython-lib`.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge dpgeorge force-pushed the tools-mpremote-mip-install-check-hash branch from 3cc05f4 to e53f262 Compare May 2, 2025 01:45
@dpgeorge dpgeorge merged commit e53f262 into micropython:master May 2, 2025
59 of 64 checks passed
@dpgeorge dpgeorge deleted the tools-mpremote-mip-install-check-hash branch May 2, 2025 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@projectgus projectgus projectgus approved these changes

Assignees
No one assigned
Labels
tools Relates to tools/ directory in source, or other tooling
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dpgeorge @projectgus