Update TclX for Tcl 9 API compatibility#27
Conversation
The current TclX implementation depends on several Tcl 8-era APIs and compatibility assumptions that no longer hold with Tcl 9. This commit updates TclX for Tcl 9 API compatibility, allowing it to build, load, and pass the main test suite against Tcl 9.1.
|
The CI builds against Tcl8, that will need to be updated for this branch. |
Oh, I missed it. My bad. I will work on it. |
Build Tcl 9.1a1 from the core-9-1-a1 branch in Linux and macOS GitHub Actions instead of using Tcl 8.6. The workflows now clone the Tcl source branch, build and install Tcl during CI, locate the installed tclConfig.sh and tclsh paths dynamically, verify the Tcl patchlevel, and pass the discovered Tcl configuration directory to configure.
Stop adding ppa:ubuntu-toolchain-r/test in the Linux CI workflow. Ubuntu Noble provides gcc-11 and g++-11 through the standard repositories, so the external PPA is unnecessary and can cause CI failures when Launchpad is unreachable.
Add stricter Tcl version validation to the Linux and macOS CI workflows. The workflows now source the discovered tclConfig.sh, combine TCL_VERSION and TCL_PATCH_LEVEL to verify the expected Tcl 9.1a1 version, and keep the tclsh patchlevel check using EXPECTED_TCL_VERSION. This ensures CI does not accidentally configure against a different Tcl installation. The workflow YAML files were also formatted with yamlfmt.
|
OK, I updated the CI workflows to build against Tcl 9.1a1. Linux passes locally with I also removed the obsolete Ubuntu toolchain PPA and added stricter checks to ensure CI uses the expected Tcl installation. The workflow YAML files were also formatted with |
The macOS GitHub Actions runner does not provide autoreconf by default, which caused the Mac CI configure step to fail before the build started. Since the generated configure script is checked in again, run it directly instead of regenerating it in CI.
Drop unused helper functions added for Tcl 9 compatibility in the chmod, list, and string command implementations. The list and string commands already use the existing TclX index parsing paths where needed, preserving TclX arithmetic index expression semantics. The chmod mode parser was also unused, so keeping it only required compiler-specific unused annotations without changing behavior. This removes dead code and avoids carrying compatibility wrappers that have no callers.
Tcl 9 uses Tcl_Size for string lengths returned by Tcl object APIs, so local variables and helper parameters that receive those lengths should use Tcl_Size as well. Update the TclX_EchoObjCmd length variable and the string helper input-length parameters, while keeping ExpandString's existing int output-length interface for its callers.
|
Mac CI should work now. :) The macOS CI failure happened because |
|
Both Mac and Linux tests claim it passes but: Mac:
Linux:
|
I am working on it as we speak. :) |
Add a smoke test step to the Linux and macOS CI workflows after the build step and before the full test suite. The smoke test loads TclX from the build directory via TCLLIBPATH and sets TCLX_LIBRARY so the extension can find the TclX script library before installation. It verifies the package and runtime versions, prints the loaded shared library, and asserts results from a small set of TclX string commands. This gives CI an early hard failure if the built TclX library cannot be loaded or basic commands return unexpected results, even where the broader test suite is allowed to continue on error.
Use Tcl_GetIntFromObj when parsing numeric dup arguments as
file descriptors instead of converting through
Tcl_GetObjType("int").
Tcl object type names are internal details and
Tcl_GetObjType("int") is not a safe public conversion path.
Under Tcl 9 this can return NULL, causing Tcl_ConvertToType to
segfault when dup handles an invalid numeric file descriptor
such as dup 100.
Using Tcl_GetIntFromObj avoids that internal type lookup and
lets the invalid file descriptor path report the expected POSIX
EBADF error.
Keep the existing channel-first lookup so numeric-looking
channel names are not mistaken for file descriptors.
Thanks, that was a real issue in I tracked it to the I changed it to use I also updated the CI smoke test to exercise the build-tree TclX before installation more cleanly. It now sets After rebuilding locally, |
|
If this PR lands, I will follow up with a separate cleanup PR to fix the compiler warnings it exposes and add the necessary range checks (posssibly, will have to think about it) and related defensive checks. :) |
|
I fixed another Tcl 9 compatibility issue in this branch: The profile fix restores the saved Tested after a clean rebuild with Tcl 9.1a2: make clean
make
TCLLIBPATH="$PWD" TCLX_LIBRARY="$PWD/library" tclsh9.1 tests/all.tclResult: |
Good call, TclGetIntFromObj is the idiomatic API. |
|
It looks like it's still building a libtclX8.6. |
|
I pushed two follow-up commits after rechecking Tcl 8.6 compatibility:
I also verified the branch against Tcl 8.6.17 with a plain The Tcl 8.6 interpreter used for that run was: Result: Most importantly, regarding the library name question:
I rechecked the Tcl 9 build configuration. When configured explicitly against my Tcl 9.1a2 build, the generated So the build is using Tcl 9 headers/configuration, but the produced library name still comes from the TclX package version ( Would you prefer this compatibility branch to keep producing |
|
It should build a library that reflects the Tcl version it's built with, no? Am I missing something? Shoudln't the TclX package version match the Tcl version? |
Yes, that makes sense. You are not missing anything, I believe. I had initially treated the existing So I will update the branch so the generated TclX package/library version follows the Tcl version used for the build. In other words, a Tcl 8.6 build should continue to produce/provide Tclx 8.6, while a Tcl 9.x build should produce/provide the matching Tclx 9.x package/library. I will make sure the generated If there is also a separate TclX release-version convention I should preserve, please let me know, but my current understanding is that the package/library version should follow the Tcl build version. Am I understanding that correctly? |
|
Yes, you are understanding it correctly. TclX post-4 major/minor tracks the Tcl version, and patch tracks the Tcl version where possible. It started as "4" and out of sync because Tcl3 hadn't been updated in so long. |
Thanks, that clarification helped. I pushed two commits addressing this:
I also want to point out why the PR stats look larger than the actual functional change. One commit removes legacy form-feed characters (
That cleanup accounts for a substantial part of the changed-line count, but it is mechanical formatting cleanup only. It does not intentionally change TclX behavior. I can revert this commit if you would rather keep this PR limited to the functional changes. I retested both Tcl 8.6 and Tcl 9.1 builds after the versioning changes. The resulting behavior is now:
Both test runs completed successfully with zero failures. Does this now match the versioning behavior you had in mind? |
Yes, I think that should be a separate cleanup operation.
Yes, that is clean and correct. |
Tcl 9 can dispatch commands through Tcl_ObjCmdProc2 when isNativeObjectProc is 2. Add a Tcl 9 wrapper for that entry point and install it when the profiler temporarily rewrites command information. Restore the exact Tcl_CmdInfo captured by the trace callback before dispatching the original command. Restoring selected fields can leave a mixed wrapper/original command state when newer Tcl versions add command dispatch fields. Initialize the replacement Tcl_CmdInfo from the saved command info before modifying it, so Tcl_SetCommandInfoFromToken receives fully initialized version-specific fields. Use Tcl_GetString for the tailcall check instead of inspecting Tcl_Obj internals directly.
Under Tcl 9, command dispatch may use native object wrappers and Tcl_ObjCmdProc2, so objProc and objClientData are not reliable indicators of whether a command is a Tcl procedure. Looking the command up again by name is also unnecessary because the trace callback already provides the command token and saved command info. Use TclProcDeleteProc as the Tcl procedure marker. This preserves ordinary proc-only profiling while avoiding native commands such as join in proc-only mode.
Tcl 9 command dispatch does not produce the same uplevel stack entries in the profile-3.2 and profile-3.4 cases. Keep the existing Tcl 8 expectations and add Tcl 9-specific expected result lists. The profiler behavior now matches the Tcl 9 command dispatch model while preserving the old Tcl 8 expectations.
Use package vsatisfies with the provided Tcl package version when selecting Tcl 9 profile expectations. This avoids numeric comparison of tcl_version and keeps the test branch selection aligned with Tcl package version semantics while remaining compatible with Tcl 8.6 and Tcl 9.
Tcl_Size and TCL_SIZE_MODIFIER are provided by Tcl 9, but this branch also needs to build against Tcl 8.6. Add a small compatibility shim that maps Tcl_Size to int for Tcl 8.6 and defines an empty printf modifier. This lets Tcl 9-oriented size cleanups compile with Tcl 8.6 while using the native Tcl_Size definitions when building against Tcl 9.
Tcl 8.6 and Tcl 9 report the invalid regexp pattern in lmatch-2.3 with different diagnostic text. Select the expected message using Tcl package version checks so the test passes with both Tcl 8.6 and Tcl 9.
The TclX package version now follows the Tcl major/minor version used for the build. Tests that hard-code Tclx 8.4 or Tclx 8.6 therefore fail or abort when the package is built against Tcl 9.x and provides Tclx 9.x. Derive the required Tclx major/minor version from the running Tcl interpreter in all.tcl, testlib.tcl, and arrayproc.test. This preserves the existing Tcl 8.6 behavior while allowing the same test suite to run against Tcl 9.x builds.
TclX post-4 package versions are intended to track the Tcl version used for the build. Major and minor versions follow Tcl's major and minor version, and the patch level follows Tcl's patch level where possible. Derive PACKAGE_VERSION from TCL_VERSION after loading tclConfig.sh so the generated package version, shared library suffix, pkgIndex.tcl entry, and Tcl_PkgProvide version all use the detected Tcl major/minor version. Derive FULL_VERSION from TCL_VERSION and TCL_PATCH_LEVEL so infox version and the tcl_findLibrary lookup reflect the build Tcl patch level where it is available. This keeps Tcl 8.6 builds providing Tclx 8.6 with an 8.6.x full version, while Tcl 9.x builds provide the matching Tclx 9.x package and full version.
AC_INIT already defines PACKAGE_VERSION and PACKAGE_STRING from the static package metadata. The TclX package/library version is later derived from the detected Tcl version after loading tclConfig.sh. Undefine the AC_INIT-provided PACKAGE_VERSION and PACKAGE_STRING definitions before defining the Tcl-derived values, so strict builds with -Werror do not fail on command-line macro redefinition warnings. This preserves the TclX version-tracking behavior while keeping the generated compiler definitions unambiguous.
After avoiding redefinition of Autoconf's PACKAGE_VERSION macro, the Tcl-derived TclX package version is exposed as TCLX_PACKAGE_VERSION. Use that macro for runtime TclX initialization as well as Tcl_PkgProvide, so Tcl 9 builds search for and report the TclX package version that matches the generated library and pkgIndex.
|
I removed the unrelated form-feed cleanup from this PR and force-pushed the branch so that cleanup can be handled separately. The remaining large line count is now mostly from the checked-in generated
Since this repository commits I also retested the rebased branch with clean Tcl 8.6 and Tcl 9.1 builds. Both completed successfully with zero test failures. |
|
|
||
| if (Tcl_SetVar2Ex(interp, varName, NULL, listVarPtr, | ||
| TCL_PARSE_PART1| TCL_LEAVE_ERR_MSG) == NULL) { | ||
| 0| TCL_LEAVE_ERR_MSG) == NULL) { |
| *----------------------------------------------------------------------------- | ||
| * $Id: tclXmath.c,v 1.2 2005/03/16 23:48:21 hobbs Exp $ | ||
| *----------------------------------------------------------------------------- | ||
| * Tcl 9 compatibility shim for TclX math support. |
There was a problem hiding this comment.
Yes, this was intentional, but it is definitely a design tradeoff rather than the only possible approach.
The old TclX math code followed the older C-side math-function registration structure. For this port I moved the compatibility behavior into a Tcl-level shim to keep the Tcl 9 changes smaller and avoid adding more C-side compatibility code around that older expression/math-function interface.
That said, if you would prefer this to stay closer to the original C implementation style, I can rework it. A hybrid approach may also be reasonable: keep the implementation logic in C where appropriate, but expose the Tcl 9-compatible registration/wrapping at the Tcl level.
Let me know your thoughts!
If your comment was mainly about the file header change rather
than the implementation structure, then yes, that was intentional too, though I can make it less abrupt.
Since tclXmath.c is no longer carrying the old C-side math implementation in the same way, I changed the header to describe its current role as the Tcl 9 compatibility entry point. But I am happy to preserve more of the original wording/history there if that would be preferable!
Either way, let me know. :)
There was a problem hiding this comment.
Hmm, thinking about this more, a closer-to-C implementation may be reasonable here.
The old code used the older C-side math-function registration style. I moved this into a Tcl-level compatibility shim to keep the Tcl 9 port smaller, but Tcl math functions can also be represented as commands in ::tcl::mathfunc. So another approach would be to keep the implementation logic in C and register TclX’s math functions there with Tcl_CreateObjCommand, which should be a cleaner cross-version structure than preserving the older Tcl_CreateMathFunc style.
If you prefer that direction, I can rework this part. Let me know your thoughts please.
|
I notice there's a lot of trailing whitespace elimination noise but I think that's OK. |
There was quite a bit more unrelated cleanup noise at one point, including form-feed separator removal and broader trailing-whitespace cleanup, but I pulled most of that back out so this PR stays focused on the Tcl 9 compatibility work. I may do that cleanup separately later (already fixed all compiler warnings BTW) if needed or if that would not be problematic. :P |
|
One more note on the PR size: most of the remaining line churn comes from the regenerated checked-in The actual source change is in Excluding
|
2 participants
The current TclX implementation depends on several Tcl 8-era APIs and compatibility assumptions that no longer hold with Tcl 9. This PR updates TclX for Tcl 9 API compatibility, allowing it to build, load, and pass the main test suite against Tcl 9.1a2.
Problem Description
Tcl_Sizeinstead ofintlen,len-3,end-1, and arithmetic expressions like3*2no longer work with Tcl 9’s stricter index parsingtclIndexentries usingsource -encodingThe original implementation was written for Tcl 8 and assumes APIs and parser behavior that have changed in Tcl 9. This PR updates those assumptions while preserving TclX’s historical command behavior where practical.
Implementation Details
This PR updates TclX internals for Tcl 9 while keeping the public TclX command behavior compatible with the existing tests.
Key implementation changes:
Tcl_Sizechmodmode parsing so leading-zero mode strings keep TclX’s historical octal behaviorconvlibhandling of Tcl 9tclIndexentries usingsource -encodingread_file ... nonewlinehandling for Tcl 9’sread -nonewlinesyntaxduphandling for Tcl 9 channel names such asfile5Design Decisions
Preserve TclX compatibility:
len-3,end-1, and3*2Keep the port conservative:
Testing
Tested with Tcl 9.1a2.
The TclX main test suite passes: