Allow TCL 9.0 for tests#1673
Conversation
|
Fedora now comes with TCL 9.0 and we started getting failures like this: https://github.com/valkey-io/valkey/actions/runs/13147311114/job/36688160501 |
|
Which version is used in our online CI? |
Most of them have 8.6. Macos has 8.5. Fedora rawhide has now only 9.0. I guess we need to support all of them. |
|
TCL 9.0 works now. The test-fedorarawhide-jemalloc job passed. The remaining failures are other problems. One of them is solved in #1675. |
|
😱 Still fails with TLS: Need to solve this too. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## unstable #1673 +/- ##
============================================
- Coverage 71.58% 71.57% -0.01%
============================================
Files 125 125
Lines 69214 69214
============================================
- Hits 49545 49542 -3
- Misses 19669 19672 +3 🚀 New features to boost your workflow:
|
|
😓 The TCL TLS package doesn't seem to support TCL 9.0 yet. See here: https://core.tcl-lang.org/tcltls/wiki/Documentation There's a beta version of this package 2.0 which will support TCL 9.0 according to this page: https://core.tcl-lang.org/tcltls/file?name=doc/tls.html&ci=trunk Currently, fedora rawhide includes tcltls 1.7 which is incompatible with tcl 9.0 and effectively broken. I guess they will fix it soon. Alternatives to this PR:
|
i think we can go with this option. |
I think having it here is good as it notifies us about upcoming issues, but I agree for the moment we shouldn't be blocked on it. |
|
@jonathanspw is looking at solving this in Fedora, probably add TCL 8 back. |
* Remove version requirements like `package require Tcl 8.5` which prevent TCL 9.x. * Remove TCL 8.7 from list of allowed versions, since there is no such version. * Change `fconfigure -encoding binary` to `-translation binary`. * Don't require a specific version of the TCL TLS package. Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
|
The CI failures are not related to the TCL version. They are other failures, mostly known ones, not related to this PR. |
Makes our tests possible to run with TCL 9. The latest Fedora now has TCL 9.0 and it's working now, including the TCL TLS package. (This wasn't working earlier due to some packaging errors for TCL packages in Fedora, which have been fixed now.) This PR also removes the custom compilation of TCL 8 used in our Daily jobs and uses the system default TCL version instead. The TCL version depends on the OS. For the latest Fedora, you get 9.0, for macOS you get 8.5 and for most other OSes you get 8.6. The checks for TCL 8.7 are removed, because 8.7 doesn't exist. It was never released. --------- Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
|
Do we need to backport these changes to release branches? |
|
Context: The goal is to make already released branches flaky test free and backport the existing effort from unstable to stabilize the branches. Currently due to the TCL 8.5 check, fedora runners aren't able to run the test because they have 9.0 installed. Sample Link of the failure: https://github.com/sarthakaggarwal97/valkey/actions/runs/19113712295/job/54617221974 So I guess we will need to backport these changes to allow for TCL 9.0. @zuiderkwast @madolson any thoughts on this? Is it safe to remove this check (considering there could be breaking changes from TCL 8 to TCL 9?) Another option is to skip fedora tests for now on the already released branches. |
|
Yes, it's fine to backport this PR. It's a good idea.
Yes, this PR fixes the small differences that need to be fixed for the tests to work with TCL 8 and 9. One example is this change: - fconfigure $fd -encoding binary
+ fconfigure $fd -translation binary |
Makes our tests possible to run with TCL 9. The latest Fedora now has TCL 9.0 and it's working now, including the TCL TLS package. (This wasn't working earlier due to some packaging errors for TCL packages in Fedora, which have been fixed now.) This PR also removes the custom compilation of TCL 8 used in our Daily jobs and uses the system default TCL version instead. The TCL version depends on the OS. For the latest Fedora, you get 9.0, for macOS you get 8.5 and for most other OSes you get 8.6. The checks for TCL 8.7 are removed, because 8.7 doesn't exist. It was never released. --------- Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Makes our tests possible to run with TCL 9. The latest Fedora now has TCL 9.0 and it's working now, including the TCL TLS package. (This wasn't working earlier due to some packaging errors for TCL packages in Fedora, which have been fixed now.) This PR also removes the custom compilation of TCL 8 used in our Daily jobs and uses the system default TCL version instead. The TCL version depends on the OS. For the latest Fedora, you get 9.0, for macOS you get 8.5 and for most other OSes you get 8.6. The checks for TCL 8.7 are removed, because 8.7 doesn't exist. It was never released. --------- Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Makes our tests possible to run with TCL 9. The latest Fedora now has TCL 9.0 and it's working now, including the TCL TLS package. (This wasn't working earlier due to some packaging errors for TCL packages in Fedora, which have been fixed now.) This PR also removes the custom compilation of TCL 8 used in our Daily jobs and uses the system default TCL version instead. The TCL version depends on the OS. For the latest Fedora, you get 9.0, for macOS you get 8.5 and for most other OSes you get 8.6. The checks for TCL 8.7 are removed, because 8.7 doesn't exist. It was never released. Backport of #1673. Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech> Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Makes our tests possible to run with TCL 9. The latest Fedora now has TCL 9.0 and it's working now, including the TCL TLS package. (This wasn't working earlier due to some packaging errors for TCL packages in Fedora, which have been fixed now.) This PR also removes the custom compilation of TCL 8 used in our Daily jobs and uses the system default TCL version instead. The TCL version depends on the OS. For the latest Fedora, you get 9.0, for macOS you get 8.5 and for most other OSes you get 8.6. The checks for TCL 8.7 are removed, because 8.7 doesn't exist. It was never released. Backport of #1673. Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech> Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
|
I see that this: #1673 was not backported to 9.0 (intentionally?) |
|
Yeah, let's backport to 9.0. |
Makes our tests possible to run with TCL 9. The latest Fedora now has TCL 9.0 and it's working now, including the TCL TLS package. (This wasn't working earlier due to some packaging errors for TCL packages in Fedora, which have been fixed now.) This PR also removes the custom compilation of TCL 8 used in our Daily jobs and uses the system default TCL version instead. The TCL version depends on the OS. For the latest Fedora, you get 9.0, for macOS you get 8.5 and for most other OSes you get 8.6. The checks for TCL 8.7 are removed, because 8.7 doesn't exist. It was never released. --------- Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Makes our tests possible to run with TCL 9. The latest Fedora now has TCL 9.0 and it's working now, including the TCL TLS package. (This wasn't working earlier due to some packaging errors for TCL packages in Fedora, which have been fixed now.) This PR also removes the custom compilation of TCL 8 used in our Daily jobs and uses the system default TCL version instead. The TCL version depends on the OS. For the latest Fedora, you get 9.0, for macOS you get 8.5 and for most other OSes you get 8.6. The checks for TCL 8.7 are removed, because 8.7 doesn't exist. It was never released. --------- Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Makes our tests possible to run with TCL 9. The latest Fedora now has TCL 9.0 and it's working now, including the TCL TLS package. (This wasn't working earlier due to some packaging errors for TCL packages in Fedora, which have been fixed now.) This PR also removes the custom compilation of TCL 8 used in our Daily jobs and uses the system default TCL version instead. The TCL version depends on the OS. For the latest Fedora, you get 9.0, for macOS you get 8.5 and for most other OSes you get 8.6. The checks for TCL 8.7 are removed, because 8.7 doesn't exist. It was never released. --------- Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Fedora Rawhide/Latest now ships Tcl 9.x. The runtest scripts only looked for tclsh8.5/8.6/8.7, causing 'You need tcl 8.5 or newer' failures on Fedora CI jobs. Minimal backport of the Tcl version detection from valkey-io#1673. Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Fedora Rawhide/Latest now ships Tcl 9.x. The runtest scripts only looked for tclsh8.5/8.6/8.7, causing 'You need tcl 8.5 or newer' failures on Fedora CI jobs. Minimal backport of the Tcl version detection from valkey-io#1673. Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com> (cherry picked from commit 09b6338)
Fedora Rawhide/Latest now ships Tcl 9.x. The runtest scripts only looked for tclsh8.5/8.6/8.7, causing 'You need tcl 8.5 or newer' failures on Fedora CI jobs. Minimal backport of the Tcl version detection from valkey-io#1673. Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com> (cherry picked from commit 09b6338)
Fedora Rawhide/Latest now ships Tcl 9.x. The runtest scripts only looked for tclsh8.5/8.6/8.7, causing 'You need tcl 8.5 or newer' failures on Fedora CI jobs. Minimal backport of the Tcl version detection from #1673. Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com> (cherry picked from commit 09b6338)
Makes our tests possible to run with TCL 9.
The latest Fedora now has TCL 9.0 and it's working now, including the TCL TLS package. (This wasn't working earlier due to some packaging errors for TCL packages in Fedora, which have been fixed now.)
This PR also removes the custom compilation of TCL 8 used in our Daily jobs and uses the system default TCL version instead. The TCL version depends on the OS. For the latest Fedora, you get 9.0, for macOS you get 8.5 and for most other OSes you get 8.6.
The checks for TCL 8.7 are removed, because 8.7 doesn't exist. It was never released.