Skip to content

Add MANIFOLD_NO_IOSTREAM build-time option#1690

Merged
pca006132 merged 1 commit into
elalish:masterfrom
zmerlynn:add-no-iostream-option
May 4, 2026
Merged

Add MANIFOLD_NO_IOSTREAM build-time option#1690
pca006132 merged 1 commit into
elalish:masterfrom
zmerlynn:add-no-iostream-option

Conversation

@zmerlynn
Copy link
Copy Markdown
Contributor

@zmerlynn zmerlynn commented May 3, 2026

Summary

Adds MANIFOLD_NO_IOSTREAM (default OFF) that strips iostream- and
filesystem-using bits from the public API and tests. Useful for
freestanding/embedded consumers — the immediate motivator is
wasm32-unknown-unknown via wasm-cxx-shim, which today carries
near-equivalent patches downstream that this PR consolidates upstream.
Idea floated and well-received in #1046.

When ON:

  • Defines MANIFOLD_NO_IOSTREAM and MANIFOLD_NO_FILESYSTEM as PUBLIC
    compile defs on manifold, so consumers see the stripped API.
  • Triggers CLIPPER2_NO_IOSTREAM=ON for the bundled Clipper2 via a
    carry-patch tracking Add CLIPPER2_NO_IOSTREAM build-time option AngusJohnson/Clipper2#1094 — drops once that lands.
  • Gates iostream-using TEST() blocks at compile time.
  • FATAL_ERRORs if combined with MANIFOLD_DEBUG / MANIFOLD_TIMING.

Always-on manifold_no_iostream_check / manifoldc_no_iostream_check
compile the public headers with the macro defined; build fails if
iostream use sneaks back in.

Test plan

  • Default build: unchanged.
  • MANIFOLD_NO_IOSTREAM=ON + MANIFOLD_TEST=ON: 270 tests register
    (vs 383 default; tests requiring fixture-file reads are gated out at
    compile time), all pass.
  • nm libmanifold.{a,dylib}: zero iostream/filesystem symbols.
  • MANIFOLD_DEBUG=ON + MANIFOLD_NO_IOSTREAM=ON fatals as expected.
  • New CI cell build_no_iostream exercises the option end-to-end
    on Ubuntu/clang.
  • End-to-end with the consumer: wasm-cxx-shim builds against this
    branch with its own iostream patches dropped — full integration
    suite passes.

Notes

  • Windows: carry-patch uses GIT_CONFIG core.autocrlf=false, ships
    with *.patch text eol=lf in .gitattributes, and applies with
    --whitespace=nowarn. Without these, default Windows checkouts
    EOL-convert and the patch fails. CI is Ubuntu-only; happy to add a
    Windows lane on request.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.22%. Comparing base (5f95a3a) to head (5011e65).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1690   +/-   ##
=======================================
  Coverage   95.22%   95.22%           
=======================================
  Files          36       36           
  Lines        7974     7974           
=======================================
  Hits         7593     7593           
  Misses        381      381           

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

@zmerlynn zmerlynn force-pushed the add-no-iostream-option branch from 2c6a4c3 to 067b40b Compare May 3, 2026 14:57
zmerlynn added a commit to zmerlynn/wasm-cxx-shim that referenced this pull request May 3, 2026
Pre-release for v0.4.0. Pins manifold to a current upstream master
commit (5f95a3ac) and replaces the three shim-side iostream patches
with a single vendored diff of elalish/manifold#1690 — the upstream
PR that adds MANIFOLD_NO_IOSTREAM natively. Once #1690 lands and the
manifold pin moves past it, the carry-patch drops entirely and a
v0.4.0 (non-alpha) release follows.

Helper refactor:
  * Drops the Clipper2 pre-declaration. Manifold's manifoldDeps.cmake
    (post-#1690) owns the FetchContent_Declare(Clipper2 ...) and
    derives CLIPPER2_NO_IOSTREAM=ON from MANIFOLD_NO_IOSTREAM=ON
    automatically.
  * Sets MANIFOLD_NO_IOSTREAM=ON as a CMake cache var and lets
    manifold's option chain propagate the macros as PUBLIC compile
    defs on the manifold target.
  * API: removes CLIPPER2_GIT_TAG and EXTRA_CLIPPER2_PATCHES (manifold
    owns Clipper2). MANIFOLD_GIT_TAG / EXTRA_MANIFOLD_PATCHES /
    SKIP_BUILTIN_PATCHES continue to work.

Test expansion: 71 -> 121 tests run on the shim. Adds boolean_complex,
manifoldc, smooth (alongside existing boolean, cross_section, sdf).
Skipped: hull / properties / samples (need samples helper library);
manifold_test (uses std::set/<thread> without direct includes our
libcxx subset doesn't ship); polygon (RegisterPolygonTests is no-op
under MANIFOLD_NO_IOSTREAM, file would compile but contribute zero
tests).

Size budget: manifold_tests_size_budget bumped 1.10 -> 1.40 MB to
accommodate the expanded test set (current ~1.07 MB; ~30% headroom).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zmerlynn added a commit to zmerlynn/wasm-cxx-shim that referenced this pull request May 3, 2026
Pre-release for v0.4.0. Pins manifold to a current upstream master
commit (5f95a3ac) and replaces the three shim-side iostream patches
with a single vendored diff of elalish/manifold#1690 — the upstream
PR that adds MANIFOLD_NO_IOSTREAM natively. Once #1690 lands and the
manifold pin moves past it, the carry-patch drops entirely and a
v0.4.0 (non-alpha) release follows.

Helper refactor:
  * Drops the Clipper2 pre-declaration. Manifold's manifoldDeps.cmake
    (post-#1690) owns the FetchContent_Declare(Clipper2 ...) and
    derives CLIPPER2_NO_IOSTREAM=ON from MANIFOLD_NO_IOSTREAM=ON
    automatically.
  * Sets MANIFOLD_NO_IOSTREAM=ON as a CMake cache var and lets
    manifold's option chain propagate the macros as PUBLIC compile
    defs on the manifold target.
  * API: removes CLIPPER2_GIT_TAG and EXTRA_CLIPPER2_PATCHES (manifold
    owns Clipper2). MANIFOLD_GIT_TAG / EXTRA_MANIFOLD_PATCHES /
    SKIP_BUILTIN_PATCHES continue to work.

Test expansion: 71 -> 121 tests run on the shim. Adds boolean_complex,
manifoldc, smooth (alongside existing boolean, cross_section, sdf).
Skipped: hull / properties / samples (need samples helper library);
manifold_test (uses std::set/<thread> without direct includes our
libcxx subset doesn't ship); polygon (RegisterPolygonTests is no-op
under MANIFOLD_NO_IOSTREAM, file would compile but contribute zero
tests).

Size budget: manifold_tests_size_budget bumped 1.10 -> 1.40 MB to
accommodate the expanded test set (current ~1.07 MB; ~30% headroom).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread src/no_iostream_check.cpp Outdated
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we need this? Or can we do it by compiling examples?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fair point — quick defense was "catches creep on every build," but the new build_no_iostream CI cell already builds manifold + manifoldc with MANIFOLD_NO_IOSTREAM=ON, so iostream creep in public headers fails there. Dropped the targets + .cpp files in 40586ef.

@zmerlynn zmerlynn force-pushed the add-no-iostream-option branch from 067b40b to 40586ef Compare May 4, 2026 13:18
Strips iostream- and filesystem-using bits from the public API and
tests, for freestanding/embedded consumers (e.g.,
wasm32-unknown-unknown via wasm-cxx-shim). Default OFF.

When ON, defines MANIFOLD_NO_IOSTREAM and MANIFOLD_NO_FILESYSTEM as
PUBLIC compile defs on manifold, gates iostream-using TESTs at
compile time, and triggers CLIPPER2_NO_IOSTREAM=ON for the bundled
Clipper2 via a carry-patch tracking AngusJohnson/Clipper2#1094
(drops once that lands and the SHA pin moves past it). Incompatible
with MANIFOLD_DEBUG / MANIFOLD_TIMING (FATAL_ERROR).

Discussion: elalish#1046

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zmerlynn zmerlynn force-pushed the add-no-iostream-option branch from 40586ef to 5011e65 Compare May 4, 2026 13:44
Copy link
Copy Markdown
Collaborator

@pca006132 pca006132 left a comment

Choose a reason for hiding this comment

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

Thanks. Btw, it will be better if you just commit the changes without force-push. We will squash the updates anyway.

@pca006132 pca006132 merged commit e73c078 into elalish:master May 4, 2026
40 checks passed
@zmerlynn
Copy link
Copy Markdown
Contributor Author

zmerlynn commented May 4, 2026

Thanks. Btw, it will be better if you just commit the changes without force-push. We will squash the updates anyway.

Noted - still getting used to local preferences. If I need to rebase, would you prefer I keep the commit stack so it's easy to re-review?

@pca006132
Copy link
Copy Markdown
Collaborator

yes, it is easier if I can see the diff, especially if the change is large.

zmerlynn added a commit to zmerlynn/wasm-cxx-shim that referenced this pull request May 4, 2026
Manifold's `MANIFOLD_NO_IOSTREAM` build option is now upstream
(merged in elalish/manifold#1690 on 2026-05-04). The single
vendored carry-patch shipped at v0.4.0-alpha.1 is no longer
needed — manifold owns the option, and its `manifoldDeps.cmake`
carries a Clipper2 tracking patch for AngusJohnson/Clipper2#1094
internally.

The shim drops the carry-patch entirely:
  * `cmake/manifold-patches/0001-manifold-no-iostream.patch` deleted
    (and the now-empty directory removed).
  * `wasm_cxx_shim_add_manifold()`'s SKIP_BUILTIN_PATCHES argument
    removed (no builtin patches to skip). MANIFOLD_GIT_TAG and
    EXTRA_MANIFOLD_PATCHES continue to work.
  * `install(DIRECTORY .../manifold-patches ...)` rule dropped.
  * Helper smoke test updated for the new file layout.

Manifold pin bumped from `5f95a3ac` (master pre-#1690) to
`3ce9622b` (master at release time, includes #1690 + one
downstream commit). Build + test verified end-to-end on the new
pin: 18/18 ctest, 121/121 inner manifold tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zmerlynn added a commit to zmerlynn/wasm-cxx-shim that referenced this pull request May 4, 2026
Manifold's `MANIFOLD_NO_IOSTREAM` build option is now upstream
(merged in elalish/manifold#1690 on 2026-05-04). The single
vendored carry-patch shipped at v0.4.0-alpha.1 is no longer
needed — manifold owns the option, and its `manifoldDeps.cmake`
carries a Clipper2 tracking patch for AngusJohnson/Clipper2#1094
internally.

The shim drops the carry-patch entirely:
  * `cmake/manifold-patches/0001-manifold-no-iostream.patch` deleted
    (and the now-empty directory removed).
  * `wasm_cxx_shim_add_manifold()`'s SKIP_BUILTIN_PATCHES argument
    removed (no builtin patches to skip). MANIFOLD_GIT_TAG and
    EXTRA_MANIFOLD_PATCHES continue to work.
  * `install(DIRECTORY .../manifold-patches ...)` rule dropped.
  * Helper smoke test updated for the new file layout.

Manifold pin bumped from `5f95a3ac` (master pre-#1690) to
`3ce9622b` (master at release time, includes #1690 + one
downstream commit). Build + test verified end-to-end on the new
pin: 18/18 ctest, 121/121 inner manifold tests pass.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@elalish elalish mentioned this pull request May 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants