Skip to content

Conversation

@mayorova
Copy link
Contributor

@mayorova mayorova commented Aug 8, 2025

What this PR does / why we need it:

Because Ruby 3.1 is EOL

Which issue(s) this PR fixes

https://issues.redhat.com/browse/THREESCALE-11906

Verification steps

Special notes for your reviewer:

There was a previous failed attempt to upgrade here: #3864

akostadinov
akostadinov previously approved these changes Aug 8, 2025
Copy link
Contributor

@akostadinov akostadinov left a comment

Choose a reason for hiding this comment

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

Maybe we need to update io-console to 0.7.1 to keep it with the default in ruby and avoid native compilation errors on fedora 42.

https://stdgems.org/io-console/

But it's alright to merge as is and try.

Copy link
Contributor

@jlledom jlledom left a comment

Choose a reason for hiding this comment

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

I doesn't work for me, bundle exec rails assets:clobber assets:precompile fails with this error:

$ bundle exec rails assets:clobber
bin/rails aborted!
ArgumentError: wrong number of arguments (given 3, expected 1..2) (ArgumentError)

  RE_NON_ASCII = Regexp.new('([\x00-\xFF])', Regexp::IGNORECASE, 'n') # [^\0-\177]
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/home/jlledom/redhat/porta/config/application.rb:38:in `<main>'
/home/jlledom/redhat/porta/Rakefile:3:in `require_relative'
/home/jlledom/redhat/porta/Rakefile:3:in `<main>'
bin/rails:4:in `<main>'
(See full trace by running task with --trace)

What I did:

  • Updated my .tool-versions to point to ruby 3.3
  • Installed bundler 2.5.22

ruby 3.1.5
ruby 3.3.1
nodejs 18.20.4
python 2.7.18
Copy link
Contributor

Choose a reason for hiding this comment

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

I know it's not related, but, why do we need python?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe this was needed for macs, not sure why...
Maybe @josemigallas or @lvillen can confirm whether this is still needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's required by some dependency. A while ago python 3 was made the default in macOS and broke such dependency, so porta could not run.

Copy link
Contributor

Choose a reason for hiding this comment

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

I doubt anything modern will need python 2.7.18, maybe you need to try again with 3.

@jlledom
Copy link
Contributor

jlledom commented Aug 11, 2025

Maybe we need to update io-console to 0.7.1 to keep it with the default in ruby and avoid native compilation errors on fedora 42.

https://stdgems.org/io-console/

But it's alright to merge as is and try.

Agree on upgrading io-console. Either here or in another PR

@mayorova
Copy link
Contributor Author

I doesn't work for me, bundle exec rails assets:clobber assets:precompile fails with this error:

RE_NON_ASCII = Regexp.new('([\x00-\xFF])', Regexp::IGNORECASE, 'n') # [^\0-\177]
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Yes, there is some other thing needed. And also, this PR is currently blocked by #4130, which is WIP, so this one is also WIP 🙃

@jlledom
Copy link
Contributor

jlledom commented Sep 24, 2025

@mayorova #4130 is merged, is this reviewable now?

@mayorova
Copy link
Contributor Author

@mayorova #4130 is merged, is this reviewable now?

Nope, it actually depends on #4145

This branch will need to be rebased.

@github-actions github-actions bot added the Stale label Oct 26, 2025
@3scale 3scale deleted a comment from github-actions bot Oct 26, 2025
@github-actions github-actions bot removed the Stale label Oct 27, 2025
@mayorova mayorova force-pushed the ruby-3.3 branch 4 times, most recently from 1eaed14 to f1148a2 Compare November 25, 2025 14:12
system-builder-ruby31: &system-builder-ruby31
image: quay.io/3scale/system-builder:5207de2
system-builder-ruby33: &system-builder-ruby33
image: quay.io/3scale/system-builder:ruby-3.3
Copy link
Contributor

Choose a reason for hiding this comment

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

Where can I see the dockerfile for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines 1 to 7
# This module adds a 'present' literal to the liquid expressions, so that
# expressions such as the following can be used:
# {% if current_account.applications.first == present %}
# This is required because existing templates use it
# Previously, this feature was added like this:
# Liquid::Expression::LITERALS['present'] = :present?
# but in the latest Liquid this constant hash is frozen
Copy link
Contributor

Choose a reason for hiding this comment

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

I trust you, but is this feature tested in the test suite?

@mayorova mayorova force-pushed the ruby-3.3 branch 2 times, most recently from 069db8e to a22b3a3 Compare November 27, 2025 10:48
@mayorova mayorova changed the base branch from master to upgrade-liquid November 27, 2025 12:30
protected_attributes_continued (1.9.0)
activemodel (>= 5.0)
pry (0.13.1)
pry (0.15.2)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

NameError: undefined method `=~' for class `Pry::Code' (NameError)
/home/dmayorov/Projects/3scale/porta/config/application.rb:38:in `<top (required)>'

byebug (~> 11.0)
pry (~> 0.13.0)
pry-doc (1.3.0)
pry-byebug (3.11.0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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


gem 'kubeclient'

gem 'nkf'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

/home/dmayorov/.asdf/installs/ruby/3.3.1/lib/ruby/gems/3.3.0/gems/bootsnap-1.16.0/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:32: warning: /home/dmayorov/.asdf/installs/ruby/3.3.1/lib/ruby/3.3.0/kconv.rb is found in nkf, which will no longer be
 part of the default gems since Ruby 3.4.0. Add nkf to your Gemfile or gemspec. Also contact author of mechanize-2.7.7 to add nkf into its gemspec.

@mayorova mayorova force-pushed the upgrade-liquid branch 2 times, most recently from 5598a4e to 439ce50 Compare December 4, 2025 14:29
Base automatically changed from upgrade-liquid to master December 9, 2025 16:34
@mayorova mayorova marked this pull request as ready for review December 10, 2025 11:58
jlledom
jlledom previously approved these changes Dec 11, 2025
Copy link
Contributor

@jlledom jlledom left a comment

Choose a reason for hiding this comment

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

It all works for me 👍

@qltysh
Copy link

qltysh bot commented Dec 11, 2025

❌ 1 blocking issue (3 total)

Tool Category Rule Count
rubocop Style Trailing whitespace detected. 1
qlty Duplication Found 17 lines of similar code in 2 locations (mass = 85) 2

jlledom
jlledom previously approved these changes Dec 12, 2025
akostadinov
akostadinov previously approved these changes Dec 16, 2025
Copy link
Contributor

@akostadinov akostadinov left a comment

Choose a reason for hiding this comment

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

Looks awesome!

jlledom
jlledom previously approved these changes Dec 18, 2025
prism (1.4.0)
prometheus-client-mmap (0.28.0)
rb_sys (~> 0.9)
prometheus-client-mmap (1.3.0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, this is causing issues in midstream builds.

  1. This version requires Rust
 Installing prometheus-client-mmap 1.3.0 with native extensions
 Building native extensions. This could take a while...
 Bundler::InstallError: Gem::Ext::BuildError: ERROR: Failed to build gem native extension.
 
     current directory: /opt/system/vendor/bundle/ruby/3.3.0/gems/prometheus-client-mmap-1.3.0/ext/fast_mmaped_file_rs
 /usr/bin/ruby -I/usr/share/rubygems extconf.rb
 checking for rustc... no
 *** extconf.rb failed ***
 Could not create Makefile due to some reason, probably lack of necessary
 libraries and/or headers.  Check the mkmf.log file for more details.  You may
 need configuration options.
 
 Provided configuration options:
 	--with-opt-dir
 	--without-opt-dir
 	--with-opt-include=${opt-dir}/include
 	--without-opt-include
 	--with-opt-lib=${opt-dir}/lib64
 	--without-opt-lib
 	--with-make-prog
 	--without-make-prog
 	--srcdir=.
 	--curdir
 	--ruby=/usr/bin/$(RUBY_BASE_NAME)
 extconf.rb:28:in `<main>': rustc not found. prometheus-client-mmap now requires Rust. (RuntimeError)
 
 To see why this extension failed to compile, please check the mkmf.log which can be found here:
 
   /opt/system/vendor/bundle/ruby/3.3.0/extensions/x86_64-linux/3.3.0/prometheus-client-mmap-1.3.0/mkmf.log
 
 extconf failed, exit code 1
 
 Gem files will remain installed in /opt/system/vendor/bundle/ruby/3.3.0/gems/prometheus-client-mmap-1.3.0 for inspection.
 Results logged to /opt/system/vendor/bundle/ruby/3.3.0/extensions/x86_64-linux/3.3.0/prometheus-client-mmap-1.3.0/gem_make.out

This can be fixed easily by installing the rust-toolset package.

  1. Rust dependencies are attempted to be installed:
 current directory: /opt/system/vendor/bundle/ruby/3.3.0/gems/prometheus-client-mmap-1.3.0/ext/fast_mmaped_file_rs
 make DESTDIR\= sitearchdir\=./.gem.20251230-1-b9lap2 sitelibdir\=./.gem.20251230-1-b9lap2
 cargo rustc --locked --manifest-path ./Cargo.toml --target-dir target --lib --profile release -- -C linker=gcc -L native=/usr/lib64 -C link-arg=-Wl,-z,relro -C link-arg=-Wl,--as-needed -C link-arg=-Wl,-z,now -C link-arg=-specs=/usr/lib/rpm/redhat/redhat-hardened-ld -C link-arg=-specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 
     Updating crates.io index
 warning: spurious network error (3 tries remaining): [56] Failure when receiving data from the peer (Received HTTP code 403 from proxy after CONNECT)
 warning: spurious network error (2 tries remaining): [56] Failure when receiving data from the peer (Received HTTP code 403 from proxy after CONNECT)
 warning: spurious network error (1 tries remaining): [56] Failure when receiving data from the peer (Received HTTP code 403 from proxy after CONNECT)
 error: failed to get `hashbrown` as a dependency of package `fast_mmaped_file_rs v0.1.0 (/opt/system/vendor/bundle/ruby/3.3.0/gems/prometheus-client-mmap-1.3.0/ext/fast_mmaped_file_rs)`
 
 Caused by:
   download of config.json failed
 
 Caused by:
   failed to download from `https://index.crates.io/config.json`
 
 Caused by:
   [56] Failure when receiving data from the peer (Received HTTP code 403 from proxy after CONNECT)
 make: *** [Makefile:572: target/release/libfast_mmaped_file_rs.so] Error 101
 
 make failed, exit code 2
 
 Gem files will remain installed in /opt/system/vendor/bundle/ruby/3.3.0/gems/prometheus-client-mmap-1.3.0 for inspection.
 Results logged to /opt/system/vendor/bundle/ruby/3.3.0/extensions/x86_64-linux/3.3.0/prometheus-client-mmap-1.3.0/gem_make.out
 
   /usr/share/rubygems/rubygems/ext/builder.rb:125:in `run'
   /usr/share/rubygems/rubygems/ext/builder.rb:51:in `block in make'
   /usr/share/rubygems/rubygems/ext/builder.rb:43:in `each'
   /usr/share/rubygems/rubygems/ext/builder.rb:43:in `make'
   /usr/share/rubygems/rubygems/ext/ext_conf_builder.rb:42:in `build'
   /usr/share/rubygems/rubygems/ext/builder.rb:193:in `build_extension'
   /usr/share/rubygems/rubygems/ext/builder.rb:227:in `block in build_extensions'

This doesn't work because build environment is hermetic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants