Enable the mmap stdlib module on Nanvix#561
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Enables the mmap stdlib extension module for the Nanvix port by removing it from Nanvix’s “not available” module set, and fixes the resulting link failure by autoconf-detecting msync() and compiling mmap.flush()’s Unix implementation only when msync is available.
Changes:
- Remove
mmapfromPY_STDLIB_MOD_SET_NAfor Nanvix so it can be built when headers/requirements are present. - Add
msynctoAC_CHECK_FUNCSand introduceHAVE_MSYNCto the config header template. - Gate the Unix
mmap.flush()implementation behindHAVE_MSYNC, falling back to the existing “flush not supported” behavior when absent.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
pyconfig.h.in |
Adds HAVE_MSYNC placeholder so autoconf can define it when detected. |
configure.ac |
Checks for msync() and stops forcing mmap to n/a on Nanvix. |
configure |
Mirrors configure.ac updates: emits HAVE_MSYNC and removes Nanvix mmap n/a assignment. |
Modules/mmapmodule.c |
Compiles the Unix msync()-based flush() path only when HAVE_MSYNC is defined. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Empirical enablement per .vault/designs/stdlib-modules-enablement/design.md. Drop [mmap] from the Nanvix arm of PY_STDLIB_MOD_SET_NA in configure.ac and the matching py_cv_module_mmap=n/a in configure. Closes #301 (pending verification).
This was referenced Apr 29, 2026
Cycle 2 of the empirical enablement of issue #301. Cycle 1 (commit 8dd17c2fde3) flipped the configure switch and confirmed that Modules/mmapmodule.c:683 calls msync() unconditionally inside mmap.flush(), and that nanvix's libposix.a does not export msync (see .vault/designs/stdlib-modules-enablement/findings.md). This commit: - adds msync to the AC_CHECK_FUNCS list in configure.ac (alongside the existing mmap/mremap entries) and mirrors the change in the generated configure script (no autoreconf, per port convention). - adds HAVE_MSYNC to pyconfig.h.in so the autoconf substitution has a target. - guards the UNIX msync() branch in mmap_flush_method() with defined(HAVE_MSYNC); platforms without msync fall through to the existing 'flush not supported on this system' ValueError path. On any platform that has msync (every existing in-tree target), HAVE_MSYNC will be defined and behaviour is unchanged. On nanvix, mmap.flush() will raise ValueError instead of failing to link.
ppenna
reviewed
Apr 29, 2026
| py_cv_module__testmultiphase=n/a | ||
| py_cv_module__testsinglephase=n/a | ||
| py_cv_module__testclinic=n/a | ||
| py_cv_module_=n/a |
ppenna
reviewed
Apr 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #301.
Drops
mmapfromPY_STDLIB_MOD_SET_NAfor the Nanvix arm and adds the one autoconf check needed to make the build link cleanly. Two commits: switch flip, then theHAVE_MSYNCguard that lets the link succeed.What this enables
The
mmapmodule imports. Most of it works against the existing libposixmmapbinding (which today supportsMAP_PRIVATE | MAP_ANONYMOUSwithfd == -1):import mmap; all constants (PAGESIZE,MAP_*,PROT_*,ACCESS_*,MADV_*).mmap.mmap(-1, size)— anonymous private mappings. CPython's POSIX constructor (mmapmodule.c:1318) treatsfd == -1as "give me anonymous memory" and addsMAP_ANONYMOUSitself, so this hits libposix's supported path end-to-end.read,write,seek,tell,find/rfind, indexing, slicing,len(), iteration,move,read_byte,write_byte,close,size.struct,pickle,hashlib.update, …)..resize()for anonymous mappings (shrink works; grow uses the upstream unmap+remap fallback sincemremapis absent).What this doesn't enable, and why
Each of these is a missing piece in
nanvix/nanvix, not in this PR; tracking issues filed:mmap.mmap(fd, …)withfd >= 0→OSError(EINVAL). libposix's binding rejects every non-anonymous form because the kernelvmapkcall takes no fd/offset. [libs/kernel] Implement file-backed mmap() (non-anonymous mappings) nanvix#2211 (file-backed mmap).MAP_SHAREDandMAP_FIXEDflags →EINVAL. Same root cause. [libs/kernel] Implement file-backed mmap() (non-anonymous mappings) nanvix#2211.mmap.flush()→ValueError("flush not supported on this system").msyncis not in libposix; this PR's second commit gates the one call site (mmapmodule.c:683) onHAVE_MSYNCand falls through to the existing upstream#elsepath that already raises this exactValueErroron every other Unix withoutmsync. [libs] Implement msync(2) nanvix#2212.mmap.madvise()→ method absent.madvisenot in libposix; upstream already gates the method onHAVE_MADVISEso it silently disappears. [libs] Implement madvise(2) nanvix#2213.mmap.resize()grow path on (eventual) file-backed mappings would lose contents.mremapnot in libposix; upstream fallback only works for anonymous mappings. [libs] Implement mremap(2) nanvix#2214.mlock/munlockfamily — not surfaced viammapat all (lives inresource); filed for completeness as [libs] Implement mlock/munlock/mlockall/munlockall nanvix#2215.Why this shape (the second commit)
Modules/mmapmodule.c:683callsmsync()unconditionally on theUNIXbranch with no upstreamHAVE_MSYNCguard — even though the surroundingmadvise/mremapcode is allHAVE_*-gated andmmap/mremapare already in theAC_CHECK_FUNCSlist right next door.msyncis the anomaly.Without the guard, the build doesn't link (verified: clean
./z distclean && ./z clean && ./z --mode=standalone setup && ./z buildfails withModules/mmapmodule.c:683: undefined reference to msync;nmon every.ain the freshly-set-up sysroot confirms zeromsyncdefinitions). With the guard, link succeeds and onlymmap.flush()is affected — falling through to the sameValueErrorusers on any othermsync-less Unix already see.Behaviour on every existing in-tree target is bit-identical: their autoconf detects
msync,HAVE_MSYNCis defined, the original branch runs.Per port convention,
configure.acand the generatedconfigureare hand-edited in parallel rather than regenerated withautoreconf.Verification
All three modes, full cycle (
./z distclean && ./z clean && ./z --mode=$MODE setup && ./z build && ./z test): standalone (116 regrtest modules), single-process (162), multi-process (162) all green.mmapmoves from the n/a list into the built-in set (38 → 37 n/a); no other module's status changes.Out of scope
test_mmaptoNANVIX_TEST_LIST. The module builds and the anonymous path works; wiring up the test will need anis_nanvix → SkipTestguard for the file-backed andMAP_SHAREDcases, analogous to upstream'sis_emscripten. Separate PR by convention._posixshmem(Enable_posixshmemstdlib module (POSIX shared memory) #306) and other related modules.