8000 Fix CI latest fedora: install awk and tcl8, build tcltls from source by vitahlin · Pull Request #1965 · valkey-io/valkey · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix CI latest fedora: install awk and tcl8, build tcltls from source #1965

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign 8000 in to your account

Jump to bottom
Merged
merged 27 commits into from
Apr 22, 2025
Merged

Fix CI latest fedora: install awk and tcl8, build tcltls from source #1965

merged 27 commits into from
Apr 22, 2025

Conversation

vitahlin
Copy link
Contributor
@vitahlin vitahlin commented Apr 16, 2025 •
edited by zuiderkwast
Loading

Fix two problems in fedora CI jobs:

  1. Install awk where missing. It's required for building jemalloc.
  2. Fix problems with TCL, required for running the tests. Fedora comes with
    TCL 9 by default, but the TLS package 'tcltls' isn't built for TCL 9.
    Install 'tcl8' and build 'tcltls' from source.

vitahlin added 10 commits April 17, 2025 02:23
@vitahlin
fix fedora jemalloc
feb9aa4 
Signed-off-by: vitah <vitahlin@gmail.com>
@vitahlin
Fix fedora jemalloc
350e3b1 
Signed-off-by: vitah <vitahlin@gmail.com>
@vitahlin
Fix fedora
c99f688 
Signed-off-by: vitah <vitahlin@gmail.com>
@vitahlin
Fix fedora
899b099 
Signed-off-by: vitah <vitahlin@gmail.com>
@vitahlin
Fix fedora
a0b45ab 
Signed-off-by: vitah <vitahlin@gmail.com>
@vitahlin
Fix fedora tls
8076eab 
Signed-off-by: vitah <vitahlin@gmail.com>
@vitahlin
Feat fedora jemalloc
bd24f15 
Signed-off-by: vitah <vitahlin@gmail.com>
@vitahlin
Feat fedora jemalloc
cb5b519 
Signed-off-by: vitah <vitahlin@gmail.com>
@vitahlin
Fix fedora jemalloc
777057b 
Signed-off-by: vitah <vitahlin@gmail.com>
@vitahlin
Fix fedora jemalloc action
1b7f2a0 
Signed-off-by: vitah <vitahlin@gmail.com>
vitahlin added 2 commits April 17, 2025 02:30
@vitahlin
Fix fedora jemalloc CI
29559c7 
Signed-off-by: vitah <vitahlin@gmail.com>
@vitahlin
Fix fedora jemalloc CI
29cee0e 
Signed-off-by: vitah <vitahlin@gmail.com>
vitahlin added 3 commits April 17, 2025 11:42
@vitahlin
Fix install awk
3a5c47a 
Signed-off-by: vitah <vitahlin@gmail.com>
@vitahlin
Revert "Fix install awk"
db861c3 
This reverts commit 3a5c47a.

Fix fedora CI

Signed-off-by: vitah <vitahlin@gmail.com>
@vitahlin
Fix fedora CI
8b41a51 
Signed-off-by: vitah <vitahlin@gmail.com>
@vitahlin
Copy link
Contributor Author

This is the latest CI result: https://github.com/vitahlin/valkey/actions/runs/14507811027

@codecov Codecov
Copy link
codecov bot commented Apr 17, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.04%. Comparing base (0b94ca6) to head (3a4aea0).
Report is 1 commits behind head on unstable.

Additional details and impacted files 
@@             Coverage Diff              @@
##           unstable    #1965      +/-   ##
============================================
+ Coverage     71.01%   71.04%   +0.03%     
============================================
  Files           123      123              
  Lines         66033    66033              
============================================
+ Hits          46892    46913      +21     
+ Misses        19141    19120      -21

see 21 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vitahlin vitahlin changed the title Fix CI fedora jemalloc need enable-jemalloc-prefix Fix CI latest fedora need install awk Apr 17, 2025
@vitahlin
Refactor CI to install awk only if not present
561d3f5 
Signed-off-by: vitah <vitahlin@gmail.com>
@zuiderkwast zuiderkwast added the run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP) label Apr 17, 2025
@vitahlin
Copy link
Contributor Author

Latest CI result: https://github.com/vitahlin/valkey/actions/runs/14518932113

@zuiderkwast
Copy link
Contributor

I see in your Daily run, which awk displays an error. Maybe we should redirect stderr instead of stdout. And the package name seems to be gawk. The package awk is just an alias of gawk.

which: no awk in (/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin)
Updating and loading repositories:
Repositories loaded.
Total size of inbound packages is 1 MiB. Need to download 1 MiB.
After this operation, 3 MiB extra will be used (install 3 MiB, remove 0 B).
Package             Arch   Version      Repository      Size
Installing:
 gawk               x86_64 5.3.1-1.fc42 fedora       1.7 MiB

Maybe we can install the package gawk on all fedora versions? Then we don't need to check if it's already installed.

zuiderkwast
zuiderkwast reviewed Apr 17, 2025
View reviewed changes
Copy link
Contributor
@zuiderkwast zuiderkwast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just try installing awk in all Fedora systems and if it's already installed, it does nothing?

I added the extra-tests label which should run trigger these builds on every change.

zuiderkwast and others added 2 commits April 17, 2025 17:35
@zuiderkwast
Install /usr/bin/awk on all fedora versions
a6f61ed 
If it's already installed, it's a no-op.

Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
@vitahlin
Style remove extra spaces
d265cdb 
Signed-off-by: vitah <vitahlin@gmail.com>
@zuiderkwast
Copy link
Contributor

That seemed to work, but now it fails to start the tests. TCL 8.x is not available. Fedora 42 was just released days ago?

https://github.com/valkey-io/valkey/actions/runs/14519381245/job/40736301592?pr=1965

Is this what we observed earlier in the Fedora Rawhide builds? See

vitahlin added 3 commits April 22, 2025 12:06
@vitahlin
CI install tcltls from source on Fedora
09a60ed 
Signed-off-by: vitah <vitahlin@gmail.com>
@vitahlin
CI add comments to clarify tcltls installation logic
f25f21b 
Signed-off-by: vitah <vitahlin@gmail.com>
@vitahlin
CI fix comment
0d5445d 
Signed-off-by: vitah <vitahlin@gmail.com>
@vitahlin
Copy link
Contributor Author
vitahlin commented Apr 22, 2025 •
edited
Loading

Latest CI result: https://github.com/vitahlin/valkey/actions/runs/14587957516
It works well in fedora 42.

Not sure why test-rdma is failing: https://github.com/valkey-io/valkey/actions/runs/14587947974/job/40916792116?pr=1965
image

@vitahlin
Copy link
Contributor Author
vitahlin commented Apr 22, 2025 •
edited
Loading

Daily CI reply-schemas-validator also failed on the unstable branch: https://github.com/valkey-io/valkey/actions/runs/14583803725/job/40905598989

I couldn’t find any record of the failure in job test-ubuntu-32bit, but the failures in the two CI jobs are probably unrelated to the changes in this PR

zuiderkwast
zuiderkwast approved these changes Apr 22, 2025
View reviewed changes
Copy link
Contributor
@zuiderkwast zuiderkwast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Good job!

.github/actions/rpm-distros-tcl8/action.yml
# As a workaround, we install Tcl 8 and manually build tcltls 1.7.22 from source.
# Once tcltls adds support for Tcl 9, this logic can be removed and system packages used instead.
- run: |
if [[ "${{ inputs.matrix_name }}" =~ "fedora" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works because we the jobs we have are "fedoralatest" and "fedorarawhide". If we would have an older fedora job, it wouldn't work. But it's easy enough to change this condition later.

@zuiderkwast zuiderkwast changed the title Fix CI latest fedora need install awk Fix CI latest fedora: install awk and tcl8, build tcltls from source Apr 22, 2025
@zuiderkwast zuiderkwast merged commit 1840871 into valkey-io:unstable Apr 22, 2025
57 of 59 checks passed
@vitahlin vitahlin deleted the fedora-jemalloc branch April 22, 2025 14:18
@zuiderkwast zuiderkwast moved this to To be backported in Valkey 7.2 Apr 23, 2025
@zuiderkwast zuiderkwast moved this to To be backported in Valkey 8.0 Apr 23, 2025
@zuiderkwast zuiderkwast moved this to To be backported in Valkey 8.1 Apr 23, 2025
murphyjacob4 pushed a commit to murphyjacob4/valkey that referenced this pull request Apr 23, 2025
@vitahlin @zuiderkwast @murphyjacob4
Fix CI latest fedora: install awk and tcl8, build tcltls from source (v…
eb5b1a9 
…alkey-io#1965)

Fix two problems in fedora CI jobs:

1. Install awk where missing. It's required for building jemalloc.
2. Fix problems with TCL, required for running the tests. Fedora comes
   with TCL 9 by default, but the TLS package 'tcltls' isn't built for TCL 9.
   Install 'tcl8' and build 'tcltls' from source.

---------

Signed-off-by: vitah <vitahlin@gmail.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
murphyjacob4 pushed a commit to murphyjacob4/valkey that referenced this pull request Apr 23, 2025
@vitahlin @zuiderkwast @murphyjacob4
Fix CI latest fedora: install awk and tcl8, build tcltls from source (v…
fec530d 
…alkey-io#1965)

Fix two problems in fedora CI jobs:

1. Install awk where missing. It's required for building jemalloc.
2. Fix problems with TCL, required for running the tests. Fedora comes
   with TCL 9 by default, but the TLS package 'tcltls' isn't built for TCL 9.
   Install 'tcl8' and build 'tcltls' from source.

---------

Signed-off-by: vitah <vitahlin@gmail.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
murphyjacob4 pushed a commit to murphyjacob4/valkey that referenced this pull request Apr 23, 2025
@vitahlin @zuiderkwast @murphyjacob4
Fix CI latest fedora: install awk and tcl8, build tcltls from source (v…
e4fe766 
…alkey-io#1965)

Fix two problems in fedora CI jobs:

1. Install awk where missing. It's required for building jemalloc.
2. Fix problems with TCL, required for running the tests. Fedora comes
   with TCL 9 by default, but the TLS package 'tcltls' isn't built for TCL 9.
   Install 'tcl8' and build 'tcltls' from source.

---------

Signed-off-by: vitah <vitahlin@gmail.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
murphyjacob4 pushed a commit to murphyjacob4/valkey that referenced this pull request Apr 23, 2025
@vitahlin @zuiderkwast @murphyjacob4
Fix CI latest fedora: install awk and tcl8, build tcltls from source (v…
1f70dd6 
…alkey-io#1965)

Fix two problems in fedora CI jobs:

1. Install awk where missing. It's required for building jemalloc.
2. Fix problems with TCL, required for running the tests. Fedora comes
   with TCL 9 by default, but the TLS package 'tcltls' isn't built for TCL 9.
   Install 'tcl8' and build 'tcltls' from source.

---------

Signed-off-by: vitah <vitahlin@gmail.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
murphyjacob4 pushed a commit to murphyjacob4/valkey that referenced this pull request Apr 23, 2025
@vitahlin @zuiderkwast @murphyjacob4
Fix CI latest fedora: install awk and tcl8, build tcltls from source (v…
998f524 
…alkey-io#1965)

Fix two problems in fedora CI jobs:

1. Install awk where missing. It's required for building jemalloc.
2. Fix problems with TCL, required for running the tests. Fedora comes
   with TCL 9 by default, but the TLS package 'tcltls' isn't built for TCL 9.
   Install 'tcl8' and build 'tcltls' from source.

---------

Signed-off-by: vitah <vitahlin@gmail.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
@zuiderkwast zuiderkwast moved this from To be backported to 8.1.1 in Valkey 8.1 Apr 23, 2025
@zuiderkwast zuiderkwast moved this from To be backported to 7.2.9 in Valkey 7.2 Apr 23, 2025
@zuiderkwast zuiderkwast moved this from To be backported to 8.0.3 in Valkey 8.0 Apr 23, 2025
zuiderkwast added a commit that referenced this pull request Apr 23, 2025
@vitahlin @zuiderkwast
Fix CI latest fedora: install awk and tcl8, build tcltls from source (#…
b333860 
…1965)

Fix two problems in fedora CI jobs:

1. Install awk where missing. It's required for building jemalloc.
2. Fix problems with TCL, required for running the tests. Fedora comes
   with TCL 9 by default, but the TLS package 'tcltls' isn't built for TCL 9.
   Install 'tcl8' and build 'tcltls' from source.

---------

Signed-off-by: vitah <vitahlin@gmail.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
murphyjacob4 pushed a commit to murphyjacob4/valkey that referenced this pull request Apr 23, 2025
@vitahlin @zuiderkwast @murphyjacob4
Fix CI latest fedora: install awk and tcl8, build tcltls from source (v…
eb27311 
…alkey-io#1965)

Fix two problems in fedora CI jobs:

1. Install awk where missing. It's required for building jemalloc.
2. Fix problems with TCL, required for running the tests. Fedora comes
   with TCL 9 by default, but the TLS package 'tcltls' isn't built for TCL 9.
   Install 'tcl8' and build 'tcltls' from source.

---------

Signed-off-by: vitah <vitahlin@gmail.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
zuiderkwast added a commit that referenced this pull request Apr 23, 2025
@vitahlin @zuiderkwast
Fix CI latest fedora: install awk and tcl8, build tcltls from source (#…
d62f4a0 
…1965)

Fix two problems in fedora CI jobs:

1. Install awk where missing. It's required for building jemalloc.
2. Fix problems with TCL, required for running the tests. Fedora comes
   with TCL 9 by default, but the TLS package 'tcltls' isn't built for TCL 9.
   Install 'tcl8' and build 'tcltls' from source.

---------

Signed-off-by: vitah <vitahlin@gmail.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
zuiderkwast added a commit that referenced this pull request Apr 23, 2025
@vitahlin @zuiderkwast
Fix CI latest fedora: install awk and tcl8, build tcltls from source (#…
ddbd1c2 
…1965)

Fix two problems in fedora CI jobs:

1. Install awk where missing. It's required for building jemalloc.
2. Fix problems with TCL, required for running the tests. Fedora comes
   with TCL 9 by default, but the TLS package 'tcltls' isn't built for TCL 9.
   Install 'tcl8' and build 'tcltls' from source.

---------

Signed-off-by: vitah <vitahlin@gmail.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Jacob Murphy <jkmurphy@google.com>
@zuiderkwast zuiderkwast mentioned this pull request Apr 24, 2025
Closed
hwware pushed a commit to wuranxx/valkey that referenced this pull request Apr 24, 2025
@vitahlin @zuiderkwast @hwware
Fix CI latest fedora: install awk and tcl8, build tcltls from source (v…
c0b062f 
…alkey-io#1965)

Fix two problems in fedora CI jobs:

1. Install awk where missing. It's required for building jemalloc.
2. Fix problems with TCL, required for running the tests. Fedora comes
   with TCL 9 by default, but the TLS package 'tcltls' isn't built for TCL 9.
   Install 'tcl8' and build 'tcltls' from source.

---------

Signed-off-by: vitah <vitahlin@gmail.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: hwware <wen.hui.ware@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@madolson madolson madolson left review comments

@zuiderkwast zuiderkwast zuiderkwast approved these changes

Assignees
No one assigned
Labels
run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP)
Projects
Valkey 7.2
Status: 7.2.9
Valkey 8.0
Status: 8.0.3
Valkey 8.1
Status: 8.1.1
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vitahlin @zuiderkwast @madolson
0