Add TCL support for WASM builds#519
Conversation
Bump VERSION to 8.3.645.
magic.wasm can now be built as two variants packaged in the same npm
release: notcl/ (legacy, magic's own parser) and tcl/ (intubun/tcl 9.x
statically linked, commands evaluated by Tcl_EvalEx). The TCL fork is
pinned via npm/tcl.ref and cloned/built by magic itself — the tcl/
checkout is treated as read-only and built out-of-source into
magic/build-tcl-wasm/.
Configure layer:
- New usingTk variable decoupled from usingTcl in scripts/configure.in
+ scripts/configure, so --with-tcl --without-tk is finally a valid
combination. Native Linux Tcl+Tk builds keep their previous behaviour
(both flags default to enabled).
- When usingTk is empty, configure passes -DMAGIC_NO_TK so the small
number of remaining Tk callsites in tcltk/tclmagic.{h,c} compile out,
and TKCOMMON_SRCS / USE_TK_STUBS are omitted from the link.
WASM build orchestration:
- toolchains/emscripten/build-tcl-wasm.sh builds libtcl9.x.a + libtclstub.a
+ tclConfig.sh out-of-source from a pristine intubun/tcl checkout.
- npm/build.sh grew a --variant=<tcl|notcl|both> flag and writes its
outputs into npm/tcl/ and npm/notcl/. It also clones intubun/tcl with
autocrlf=false at the SHA pinned by npm/tcl.ref.
- magic/Makefile (WASM block only): magicWasm.o is now compiled with
DFLAGS_NOSTUB so Tcl_CreateInterp resolves to libtcl9.x directly
before tclStubsPtr is set. magic.js link pulls in LIB_SPECS_NOSTUB
and -ltclstub. After rules.mak include, magic: is a phony alias for
magic.js so the generic ${MODULE} recipe doesn't fight it.
- toolchains/emscripten/defs.mak: add -sUSE_ZLIB=1 (libtcl9 references
zlib), replace -sSTACK_SIZE=N with -Wl,-z,stack-size=N (emcc >=5
rejects the setting form).
- magic/magicWasm.c bootstraps the embedded interp under MAGIC_WRAPPER
(Tcl_CreateInterp -> Tcl_Init -> Tclmagic_Init) and routes
run_command through Tcl_EvalEx.
- magic/magicTop.c: gate MagicVersion/Revision/CompileTime on
!MAGIC_WRAPPER so they don't collide with the copies in
tcltk/tclmagic.c when both objects land in the same wasm binary.
npm package:
- Subpath exports: ".", "./tcl", "./notcl". Default import keeps the
pre-existing non-TCL behaviour for backward compatibility.
- examples/smoke-tcl.mjs exercises the TCL variant.
CI:
- main-wasm.yml clones intubun/tcl at the pinned ref, builds both
variants via npm/build.sh --variant=both, runs the existing notcl
test suite and the new TCL smoke test, and publishes only on a
v<x.y.z>... git tag. Tag name (minus the leading v) becomes the
npm version.
…ompletes magic_wasm_init() called Tclmagic_Init() and magicMainInit() but never ran the command-registration loop in _magic_initialize(), so magic::tech, magic::load, magic::gds and all other Magic commands were missing from the Tcl interpreter. Add TclmagicRegisterCommands() in tclmagic.c containing the WindNextClient/WindGetCommandTable loop and call it from magic_wasm_init() after magicMainInit() succeeds. Also change magic_wasm_source_file() to use Tcl_EvalFile in MAGIC_WRAPPER mode so scripts with magic:: commands are evaluated through the Tcl interpreter instead of the plain text dispatcher.
All non-TCL tests (extract, gds, drc, cif) now also run against the TCL variant using magic::-prefixed Tcl scripts. A new PCell test generates two parameterized rectangle cells and verifies their GDS output. The CI workflow runs npm run test:tcl after the TCL build and saves the generated output files as an artifact.
…comments and naming Rename *-magic.tcl scripts to *-tcl.tcl to match the -tcl.js test naming convention. Delete the old bare-command pcell.tcl (used cellname create, no magic:: prefix) and promote pcell-magic.tcl to pcell.tcl. Fix misleading comment in magicWasm.c: Tclmagic_Init only bootstraps the interpreter; magic:: commands are registered separately by TclmagicRegisterCommands after magicMainInit. Align comment block dashes in TclmagicRegisterCommands to match tclmagic.c style (62 dashes). Replace intubun/tcl references in build scripts and CI with tcltk/tcl to match the actual pinned repo in npm/tcl.ref. Also run both test suites when build.sh is invoked with --test.
Replace the last three intubun/tcl mentions with tcltk/tcl in npm/tcl.js, toolchains/emscripten/build-tcl-wasm.sh, and npm/tcl.ref. Also remove the stale comment in tcl.ref that referenced a non-existent update-tcl GitHub Actions workflow.
tclmagic.c: remove stray /*-----*/ line left over from a previous edit
that left a duplicate comment opener before TclmagicRegisterCommands.
magicWasm.c: move TxSetPoint inside the #else (non-TCL) branch of
magic_wasm_source_file and restore its explanation comment. TxSetPoint
routes TxDispatch commands to the layout window; it is irrelevant and
misleading in the Tcl_EvalFile path.
magic/Makefile: guard the TCL linker flags in the magic.js link rule
with ifneq (${TCL_LIB_DIR},). When building the non-TCL WASM variant
TCL_LIB_DIR is empty, so the unconditional -L${TCL_LIB_DIR} -ltclstub
expanded to a bare -L flag and a missing library, breaking the notcl
build.
… in CI Add explicit length limit to sscanf in TclmagicRegisterCommands: %92s instead of %s prevents a potential stack overwrite if a command name were ever longer than the buffer. Matches the available space (keyword[100] minus the 7-byte "magic::" prefix minus null). Extend the CI output-display step to also iterate over output-tcl/ so that TCL-variant test regressions are visible in the job log without downloading artifacts.
dlmiles
left a comment
dlmiles
left a comment
There was a problem hiding this comment.
A quick skim through for some initial feedback.
|
|
||
| if [ "$variant" = "tcl" ]; then | ||
| ensure_tcl_built | ||
| CFLAGS="--std=c17 -D_DEFAULT_SOURCE=1 -DEMSCRIPTEN=1${EXTRA_CFLAGS}" \ |
There was a problem hiding this comment.
-DEMSCRIPTEN=1${EXTRA_CFLAGS}
Is this correct without a space before the dollar ?
There is at least one more site below (please check all the files in npm/**). This may appear to work when EXTRA=CFLAGS="" but if you configure it to a non-blank string I would expect things to error.
There was a problem hiding this comment.
I had the spaces at other places and therefore it compiled correctly, but yeah, Ill fix it.
|
|
||
| ```bash | ||
| # Override the default clone location | ||
| TCL_REPO=/path/to/existing/tcl bash npm/build.sh --variant=tcl |
There was a problem hiding this comment.
Not clear what the function of this part is. If the WASM TCL is built, you would expect the C TCL to be installed as well. So there is a $prefix/lib/tclConfig.sh which should contain all the information a dependant project needs to consume Tcl. This config pre-dates pkg-config system that, but serves the same purpose.
If this part is for making the Tcl source available to build the WASM target. This is probably best considered being done entirely inside Tcl. For example the 'unix' subdir is for UNIX targets, the 'win' subdir is for Win32 targets, etc... I'm sure Tcl upstream project would welcome having a contribution with a 'wasm' subdir that builts Tcl for a WASM target, possibly with access to WASI (which is the closest APIs suite that should cover core functionality).
When maybe a shim for WASM in the browser. To manage WASI and non-WASI and this would be a more minor target adjustment.
This comment should not hold up the contribution here, just feedback that access to a working Tcl on WASM target might be something more Tcl users want and sits well with the construction of the project in a wasm/ subdir.
| # 3. Commit + push. CI rebuilds and republishes. | ||
|
|
||
| TCL_REPO_URL=https://github.com/tcltk/tcl.git | ||
| TCL_REF=84b23291b0dd811d642abef4ec7a55473c3eccb3 |
There was a problem hiding this comment.
TCL_REF=main
Is there a reason this is pinned by default ? Ideally should this not take the latest tag of the stable branch. So by default CI will also rebuild against newer TCL.
Can the build use the environment $TCL_REF in priority to the default here, "refs/tags/9.0.3"
Can the build use the environment $TCL_REPO_URL in priority to this default here.
Can release notes be constructed around the GitHub artifact creation, so at the bottom of the workflow task. The goal is to at a glance see the versions of important things that were use by the CI.
This might be compiler, emscripten, tcl commit/tag/url.
Ideally the CI should be optimistically building for newer current stuff in low maintenance mode by default, so no one need to update the SHA here.
Maybe in this case when the GitHub workflow runs, one task is a form of information gathering, maybe to decide the TCL_REF value and report in summary the git log -n1 style output. It then sets $TCL_REF in the workflow, to this value here can be set to TCL_REF=main. The information gathering is then used to render a $GITHUB_STEP_SUMMARY to display things with the *.wasm downloads.
If the CI requires pinning and heuristics over picking a working version to use, the pinning would be done in the workflow file.
This would then extend to using on:workflow_dispatch to specify a specify TCL_REF.
All this is to help in 6 months when it breaks to have as much information as possible available in CI to spend the least amount of time rebuilding magic using the previously working TCL_REF to confirm the problem is with the current state of the upstream project, not with magic's recent changes.
Part of the reason why the CI workflow default TCL_REF should be computed on the most recent stable branch main release tag, is only use Tcl versions that the Tcl project consider release ready.
If you are locking a version (because that is needed) around the SHA please show the git log -n1 info nearby, to show the commit vital statistics (date/tag/author/comment).
There was a problem hiding this comment.
Ive deleted the tcl.ref and now the tcl verison is tagged to the latest stable release. You can override the verion with a manual workflow dispatch. Also a build summary was added.
|
Can we resolve the Semantic Version scheme for NPM package versions as well ? Comment added after close #512 (comment) Specifically what is bad is the use of a raw git hash after a decimal point such as example: It is better to be Not thinking about this in advance, makes it impossible to fix in the future. I just have not done to research (myself) to see how other NPM projects manage including the git-hash with semantic versioning. But I know that they do manage this within their visioning scheme. But what I can see at this time is exactly what I didn't want. |
|
There are two distinct version formats in this workflow worth separating: Published releases are tag-driven: pushing git tag v8.3.799 causes the workflow to publish exactly 8.3.799 to GitHub Packages — clean three-component semver, no suffix. Security patches would be published as 8.3.800, 8.3.801 etc. Users who declare ~8.3.799 in their package.json receive those automatically on npm update. This is the standard npm mechanism for the use case described and is already in place. CI snapshots (non-tag builds) are packed as artifacts for local inspection but are never published to GitHub Packages. The format has been updated from 8.3.799-20260622.01234cdef to 8.3.799+20260622.git01234cdef. The original - was the real problem: in semver it marks a pre-release, meaning the snapshot sorted before 8.3.799 rather than after. The + separator correctly marks it as build metadata ("this is 8.3.799 built from this commit"), and the git prefix makes the hash immediately recognisable. Since snapshots are never published, the hex ordering concern does not arise in practice. Regarding 8.3.799.20260622git0123cdef — the intent is right but npm's node-semver strictly requires exactly three numeric components and rejects a fourth, so this format cannot be used regardless of convention. |
|
The upgrade to latest is always an option, and not the point I am drawing attention to (as it is an obvious matter). The security update means keep the same 8.3.799 version but receive a security patch on top (the version needs to be inserted into the range). For example there maybe users who lock a specific version of magic because it has been tested/checked and because they are building chips (with a 1 year lifecycle and expensive stability concerns) who want stability not new features, so they would use The idea of a security (or major bugfix/correction) patch issue in this way is to insert it into the range 8.3.799 range after 8.3.800 was release (retrospectively). Is it allowed in semantic versioning to have Ah then if it strictly only allows 3 parts to a version, I am saying the main release also need the |
|
npm does treat any three part sementic versioning the same, no matter the extension. npm, correct me if im wrong, does not have any way to retroactively insert security patches. The only path for patching is to increase the version number. The part after that is only for traceability. At least thats my knowledge on how the npm registry handles it. |
|
I shall research and test some things over the new few days. Is there any reason why
Unclear if As I understand it from your comments: a I don't see having the date on display being a bad thing, a human reading can immediately make a judgement call from seeing a date on they should consider to update, based around all the information they know from their view of the world. |
|
yeah semver matching is different depending on the symbol. ".", "-" and "+" have different meanings. "." is for sectioning the main version, "-" can be used to declare alpha / beta version. "1.1.1-alpha.1" for example. and "+" only adds additional information, for example build time information, etc. But I should also do more research on the topic |
|
I did my research, the "-" has to be actively selected by the user, since its only intended for prerelease version. Its mostly intended to implement patches by bumping the last part of the version. for example: magic>=8.3.799 would not match with 8.3.799-20260526, since its considered a prerelease version of a package. So Users would not get the update, unless they actively pin that version with magic@8.3.799-20260526, but it wont update the package automatically with the next npm install / npm update. And since @RTimothyEdwards magic repo is mirrored by github every day at around 9am my time, there is a more or less daily release schedule and if we ship it just as 8.3.799, then all the users would get the newest version, if they run npm install or update. |
|
My research gets to here. Using a version in the form: The inserted ZERO just makes the date easier to read. Can also be used for .1 to .9 style 4th part, the point is the scheme leaves the option open to decide another day. Which is what thinking about it now is all about (creating future options). Version locking can be done in a number of ways, see below. |
2 participants
Hey everyone,
ive been working on adding tcl support to the magic-vlsi-wasm package. Ive added:
Matrix and all CI already ran on my fork and everything is green.