Pack the wasm archive somewhere a consumer can link it - #12
Merged
Conversation
The package has been shipping 3.6 MB of browser-wasm that nobody could have used, and two independent mistakes had to line up for that: The archive was packed as libJoltC.a while every DllImport in Generated says "JoltC". On wasm the module name comes from the file name, so the lib prefix alone breaks resolution. And there was no buildTransitive targets file, which matters more. runtimes/browser-wasm/native is never probed, because wasm has no dynamic loading: the archive has to be linked into the application when it publishes, through a NativeFileReference. Without one the archive was not linked badly, it was not linked at all. The csproj had a single Content Include of runtimes/** that carried it to a decorative path. Nothing in either repository could have caught it. The eleven C test suites in JoltPhysicsC exercise the library, not the package; the CD proves the package builds; the desktop legs of the smoke test load libraries out of runtimes/ at run time, which is exactly the mechanism wasm does not use. It surfaced by opening the published .nupkg to check something else. The archive itself turned out to be fine. Before changing anything I renamed the published one to JoltC.a and linked it into a real .NET wasm application: wasm-ld accepted it, wasm-opt validated it, and the sphere fell and settled under node. None of what made Cesium's wasm hard is present here -- no setjmp in any form, no pthread, no JS exception model -- so no emcc flags change in JoltPhysicsC. WasmSmokeTest goes through PackageReference, so the targets file and the packed file name are part of what it tests; a ProjectReference would bypass both and pass against the package this commit fixes. It runs as its own job in the smoke workflow rather than a matrix row, because it shares almost nothing with the desktop legs: no identifier to select, a workload to install, and node rather than the host to run the result. Two deliberate differences from the same scenario on desktop. The job system is created with no worker threads, since there is no pthread here and JoltPhysics then runs jobs on the thread waiting at the barrier. And the bounds on the result are looser: the same drop settles in 63 steps at y=0.486 here against 48 at y=0.480 on the desktop legs, so asserting the desktop numbers would fail for a reason that has nothing to do with the package.
Contributor
API gate: additiveEvery symbol that existed still exists, unchanged. Nothing that compiled before stops compiling.
Enum and constant values are part of the measured surface: a renumbering keeps compiling and sends the wrong number to the driver, so it counts as a removal. |
It selects desktop identifiers only, and once wasm became a separate job "all five" was ambiguous about whether it covered everything.
Contributor
API gate: additiveEvery symbol that existed still exists, unchanged. Nothing that compiled before stops compiling.
Enum and constant values are part of the measured surface: a renumbering keeps compiling and sends the wrong number to the driver, so it counts as a removal. |
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.
The package has been shipping 3.6 MB of browser-wasm that nobody could have used. Two independent mistakes had to line up:
libJoltC.awhile every DllImport says"JoltC". On wasm the module name comes from the file name, so thelibprefix alone breaks resolution.buildTransitivetargets file, which matters more.runtimes/browser-wasm/nativeis never probed — wasm has no dynamic loading, so the archive has to be linked in at publish time viaNativeFileReference. Without one it was not linked badly, it was not linked at all.Nothing could have caught it. The eleven C suites in JoltPhysicsC test the library, not the package. The CD proves the package builds. The desktop legs load out of
runtimes/at run time, which is precisely the mechanism wasm does not use. It surfaced by opening the published.nupkgto check something else.The archive was fine all along
Before changing anything I renamed the published
.atoJoltC.aand linked it into a real .NET wasm application. It worked:None of what made Cesium's wasm hard is present — no
setjmpin any form, nopthread, no JS exception model — so no emcc flags change in JoltPhysicsC, and the contingency about portingJobSystemSingleThreadedis moot:numThreads = 0is enough.And the leg that would have caught it
WasmSmokeTestconsumes the package through PackageReference, so the targets file and the packed file name are part of what is under test. AProjectReferencebypasses both and would pass against the package this PR fixes.It is its own job rather than a matrix row: no identifier to select, a workload to install, and node rather than the host to run the result. Verified locally against a package packed from this branch:
One difference from Cesium worth noting: there the ten natives are fetched at CD time, so its equivalent block is conditioned on the file existing. Here they are committed, so a missing archive means a bad fetch and packing should fail loudly instead — the condition is deliberately absent.