Repository navigation
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? |
TclX.n describes infox version as the TclX version combined with the base Tcl version, which was fine while the two matched. Now that the TclX version is fixed, build the string at runtime from the compiled-in TclX version and the running interpreter's tcl_patchLevel, rather than the single value returned before.
The interpreter's call frame level is int in some Tcl builds and Tcl_Size in others, so a hardcoded %d or %ld warns on whichever build has the other type. Format it with TCL_SIZE_MODIFIER and cast the value to Tcl_Size so the specifier and the argument always agree.
|
I am sorry for sitting on this so long! Hopefully version handling is sorted now with TclX package version and I left the If you still want me to change this, I have already made the changes locally and can push them. |
The TclX release version is independent of the version used to name its installed package directory. tclXinit.c was still passing TCLX_PACKAGE_VERSION to tcl_findLibrary, causing installed builds to look for tclx9.0 instead of the directory actually containing the script library. Use TCLX_LIB_DIR_VERSION for the tcl_findLibrary lookup while retaining TCLX_PACKAGE_VERSION for TclX package registration and version reporting.
Tcl 9 no longer provides case, and its compatibility replacement does not support the syntax used by these tests. As a result, the file scan tests stopped early without reporting a failure. Replace the three remaining uses with switch, using exact matching since none of the patterns contain wildcards.
Nothing in TclX uses case anymore, and Tcl 9 has removed it in favour of switch. The compatibility command does not reproduce case's syntax or matching rules, so leaving it absent avoids silently taking the wrong branch.
Tcl 9 reworded the regular expression compilation error. scanmatch passes that message through unchanged, so ask the interpreter for it rather than hard coding one version's wording.
auto_reset deletes every command listed in the autoload index, including itself, and these tests then point auto_path at directories it cannot be reloaded from. The first reset therefore left the file unable to reset again, and it stopped partway through on every Tcl version.
These tests fail identically under Tcl 8.6 and Tcl 9 and were never reached before, because the file stopped early on the first reset. They cover package index handling, not Tcl version compatibility.
The test runner caught errors raised while sourcing a test file but only printed the message, so tests below the error were never counted and the run finished reporting success. Count those errors towards the exit status and print the error stack with them.
Tcl hands back a result object from expression evaluation that the caller owns. It was never released, so every index expression leaked one object.
|
Just a note on the scope of the earlier commits for completeness' sake. :) The changes are a mix of regressions I introduced earlier in this branch and Tcl 9 compatibility fixes. The one worth calling out is the script library lookup, which was searching for That said, 3 commits concern test infrastructure. The test runner was suppressing errors while sourcing test files, which meant ~700 tests were not actually being run under Tcl 9. Fixing this also exposed 8 package index tests in Both builds now run the same 1715 (out of 1734) tests with no failures, and the suite's exit status means something again! |
|
Why did you convert it to a draft again after Chris approved it? Also, can you change it to merge to BCK-13016-tcl9-upgrade? |
Hey! I took another look and I realized it had a few issues (although relatively minor). I thought I should have it resolved in this PR. Yes, of course! I will have it done by the end of this week. |
Tcl 9.1 is the first to compile uplevel into bytecode and the profiler does not trace compiled commands. profile-3.2 and profile-3.4 failed on Tcl 9.0, which still runs it as a command. Choose the expectations without uplevel from 9.1 on. tcltest 2.6 in Tcl 9.1.0 runs each test body from tcltest::TestOnce. The stack filter matched Eval and RunTest only with their leading colons and the profiler records TestOnce without them. Match all three with or without the colons.
cmdtrace prints the real level of each command but stops indenting at level 20. Test bodies run two levels deeper under tcltest 2.6 in Tcl 9.1.0. The body of the if in DoStuff then reached level 21 and cmdtrace printed it at the same depth as the if.
tclx.tcl read tclx_library and set auto_index and auto_path by their plain names inside namespace eval. Tcl 9 no longer resolves such names to existing globals. The check for tclx_library failed and the setup never ran. buildpackageindex could not be autoloaded and the library directory never reached auto_path.
CI built only Tcl 9.1a1, where the tests that failed on the 9.0.4 and 9.1.0 releases passed. Tcl 8.6 and 9.0 went untested. Pick tclsh by the version tclConfig.sh reports, which also finds tclsh8.6. tclsh reads the smoke test from a here document in the system encoding. In a C locale the accented letter in the clength check would count as two characters. Use the escape \u00e9 there instead.
Both workflows ran only for pull requests into master. Tcl 9 work goes into BCK-13016-tcl9-upgrade.
4784596 to
bc4050a
Compare
|
I had only been testing against Tcl 8.6 and a 9.1 alpha. Running the branch against the released 8.6.18, 9.0.4 and 9.1.0 showed failures on the last two. Two profile tests failed on 9.0.4. On 9.1.0 it was 22 profile tests and four cmdtrace tests. The first two new commits fix those tests. The third fixes The fourth commit makes CI build against those three releases instead of 9.1a1. On the alpha those tests pass, which is how CI missed the failures. Since The fifth runs both workflows for pull requests into any branch. They only ran for those into master and this one now targets BCK-13016-tcl9-upgrade. I also removed my last commit which assumed A fix for Both bugs are older than this branch. Should those go to master or to BCK-13016-tcl9-upgrade? |
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: