Skip to content

Conversation

dpgeorge
Copy link
Member

Summary

This commit unifies the configuration of MICROPY_PY_CRYPTOLIB, MICROPY_PY_HASHLIB_MD5 and MICROPY_PY_HASHLIB_SHA1, so they are enabled by default if MICROPY_PY_SSL is enabled. This matches the existing configuration of most of the ports.

With this change, all ports remain the same except:

  • reneses-ra now enables MICROPY_PY_CRYPTOLIB, MICROPY_PY_HASHLIB_MD5 and MICROPY_PY_HASHLIB_SHA1.
  • rp2 now enables MICROPY_PY_HASHLIB_MD5.

Testing

Builds will be tested by CI. Code size diff will tell the story.

Trade-offs and Alternatives

Could leave this up to the port, but IMO it's better to have the config unified so ports are more uniform in their configuration.

@dpgeorge dpgeorge added ports Relates to multiple ports, or a new/proposed port py-core Relates to py/ directory in source labels Aug 15, 2025
@dpgeorge dpgeorge requested a review from projectgus August 15, 2025 03:01
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:  +232 +0.025% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

Copy link

codecov bot commented Aug 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.38%. Comparing base (989abae) to head (b5fcb33).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #17926   +/-   ##
=======================================
  Coverage   98.38%   98.38%           
=======================================
  Files         171      171           
  Lines       22296    22296           
=======================================
  Hits        21937    21937           
  Misses        359      359           

☔ 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.

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.

Yay for standardisation!

#endif

#ifndef MICROPY_PY_HASHLIB_SHA256
#define MICROPY_PY_HASHLIB_SHA256 (1)
#endif

#ifndef MICROPY_PY_CRYPTOLIB
#define MICROPY_PY_CRYPTOLIB (0)
#define MICROPY_PY_CRYPTOLIB (MICROPY_PY_SSL)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this is worth worrying about, but the default definition of MICROPY_PY_SSL is lower down in the file than these lines.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be fine? It's already the case for some other options (for better or worse), eg MICROPY_PY_SYS_SETTRACE is used before the default definition.

This commit unifies the configuration of MICROPY_PY_CRYPTOLIB,
MICROPY_PY_HASHLIB_MD5 and MICROPY_PY_HASHLIB_SHA1, so they are enabled by
default if MICROPY_PY_SSL is enabled.  This matches the existing
configuration of most of the ports.

With this change, all ports remain the same except:
- reneses-ra now enables MICROPY_PY_CRYPTOLIB, MICROPY_PY_HASHLIB_MD5 and
  MICROPY_PY_HASHLIB_SHA1.
- rp2 now enables MICROPY_PY_HASHLIB_MD5.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge dpgeorge force-pushed the py-mpconfig-enable-hashlib-cryptolib-with-ssl branch from 1ca9fb5 to b5fcb33 Compare August 19, 2025 11:11
@dpgeorge dpgeorge merged commit b5fcb33 into micropython:master Aug 19, 2025
71 of 72 checks passed
@dpgeorge dpgeorge deleted the py-mpconfig-enable-hashlib-cryptolib-with-ssl branch August 19, 2025 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ports Relates to multiple ports, or a new/proposed port py-core Relates to py/ directory in source
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants