Skip to content

updating ruby version to 3.3.3 to avoid the nokogiri error.#109

Open
elsiddh wants to merge 5 commits intomainfrom
update-ruby
Open

updating ruby version to 3.3.3 to avoid the nokogiri error.#109
elsiddh wants to merge 5 commits intomainfrom
update-ruby

Conversation

@elsiddh
Copy link

@elsiddh elsiddh commented Nov 26, 2024

we are having this weird error on nokogiri:

nokogiri-1.15.4-aarch64-linux requires ruby version < 3.3.dev, >= 2.7, which is incompatible with the current version, ruby 3.3.3p89

I did an small update to make it working on rubies 3.3.+

@elsiddh elsiddh requested review from brlanier and javierg November 26, 2024 20:23
Copy link
Member

@javierg javierg left a comment

Choose a reason for hiding this comment

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

@elsiddh will this work up to what evrsion of ruby? probably you want to test against a matrix on travis

@brlanier
Copy link
Member

We already use this on ruby version 3.3.3... on a bunch of apps

@brlanier
Copy link
Member

1.16 of nokogiri introduces native support for ruby 3.3.3 but ends support for ruby 2.7, so this change is bigger than just updating a gem.

@brlanier
Copy link
Member

So, as far as I can tell, on a brand new install (Thanks @elsiddh for pointing that part out) bundler will try to install the newest version of nokogiri it not locked down in any other way. If using the right/wrong combination of newer ruby and bundler, then bundler will draw a line with regards to the required_ruby version tag in the gemspec files of the gems it is trying to install. Version 1.16 of nokogiri requires ruby >3.0.0. Version 1.15 requires ruby 2.7.0 but then the newer bundler doesn't like the upper bound and tries to use say that the latest is ruby 3.3.dev.
So this does work if you use an older bundler that will compile ruby and a projects Gemfile locks in an older ruby ( I think. I was able to get this to work on one example).
However, we do need to address the support for higher rubies to avoid confusion later. Since this is a deal breaker for any project still using ruby 2.7, I think this change needs to be for a minor release?? nokogiri made this change from 1.15 to 1.16, so I think we can be fine from 0.2.0 to 0.3.0.
I know 2.7 is EOL, but there are still project using it so we need to be able to support it and be able to back port any changes for a little bit back.
I don't know how we want to handle that in this project, but this is a breaking change so it needs to be obvious.

Due to this change, we would need to update the gem spec to require ruby 3.0.0 as well I guess and this update as a dependency.

Copy link
Member

@brlanier brlanier left a comment

Choose a reason for hiding this comment

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

You will also need to update the gemspec file to change the required ruby version

s.required_ruby_version = ">= 3.0"

I think that should work.

Copy link
Member

@brlanier brlanier left a comment

Choose a reason for hiding this comment

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

I think this looks good to me. I asked to have the branch you are merging into changed to main instead of master so we can drop that. So just two things to clean up, but the rest of the tests ran fine. We need to wait to see what Javier says as well.

@elsiddh elsiddh changed the base branch from master to main December 10, 2024 22:55
@elsiddh elsiddh changed the title updatng ruby version to 3.3.3 to avoid the nokogiri error. updating ruby version to 3.3.3 to avoid the nokogiri error. Dec 10, 2024
@brlanier
Copy link
Member

@javierg can you review this and see if we are missing anything. The end result would be a release targeting supporting ruby > 3 only on the main branch.
I know we would need a separate PR to setup the release part and change the version, but this code seems to work in travis and resolved an issue in Sid's local with another gem that was behind.

Copy link
Member

@javierg javierg left a comment

Choose a reason for hiding this comment

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

:shipit:

Merge is blocked by a change requested by @brlanier

Copy link
Member

@brlanier brlanier left a comment

Choose a reason for hiding this comment

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

I believe my changes were included. This was done so long ago I don't remmeber the issues we were having.

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.

3 participants