Skip to content
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.

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 in to your account

Update SQLite to fix compilation on Fedora #173

Merged
merged 2 commits into from
Jul 10, 2024
Merged

Conversation

SlouchyButton
Copy link

Updating SQLite gem resolves problem with tests for s2i-ruby-container - when using updated lock file, all tests are passing. This shouldn't really break the example app as nokogiri doesn't really seem to be used directly, but there were changes (including deprecations) between the old and updated version (https://nokogiri.org/CHANGELOG.html).

Update on nokogiri gem is forced due to shared dependency mini_portile2 between nokogiri and sqlite3.

I am not sure whether this should also be in main branch, but it is not required to fix the sclorg/s2i-ruby-container#550 as only version 3.3 is failing the tests.

Fixes sclorg/s2i-ruby-container#550

Copy link
Collaborator

@jackorp jackorp left a comment

Choose a reason for hiding this comment

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

Nice, thanks for taking a look.

How did you arrive at this lockfile, it's quite minimal change, I am assuming some command was run?

BTW we have a script for update at https://github.com/sclorg/rails-ex/blob/scripts/refresh_lock.sh

It's relatively consistent with results. Though for this update it gives a bigger diff and some changes with platform that we wouldn't want.

FTR, updating sqlite3 this high is luckily not a problem since this is only affecting the Ruby 3.3 branch.

Please address the BUNDLED WITH thingy, and we'd be good to go.

Gemfile.lock Outdated Show resolved Hide resolved
@jackorp
Copy link
Collaborator

jackorp commented Jul 9, 2024

I am not sure whether this should also be in main branch

Not in this situation *. Main branch is for Ruby versions >= 2.5 && < 3.3. Branch 3.3 of this repo is then for Ruby version >= 3.3.

  • Depends whether the "modern C initiative" or what was the change in Fedora that created this issue for us makes it into base RHEL <= 9 compiler that is used in UBI images where there is a real risk of it affecting Ruby < 3.3. But that is more of a theoretical problem. CI for Ruby containers would be the least broken thing at that point IMHO.

@SlouchyButton
Copy link
Author

How did you arrive at this lockfile, it's quite minimal change, I am assuming some command was run?

After a quick look around what bundle can do, I figured bundle update sqlite3 should be enough as it updates just the sqlite3 gem including all the dependencies and nothing else.

It also adds some stuff around unfortunately like switching the platform from ruby to the platform the bundle was run at - which I reverted to ruby as that seemed as something that shouldn't really be changed, and the BUNDLED WITH section - which I will remove as noted in the review.

@jackorp
Copy link
Collaborator

jackorp commented Jul 10, 2024

bundle update sqlite3

Cool, thanks. Figured it'd be smth like that.

Just please squash the Remove BUNDLED WITH part added by bundler commit with the previous one, and we should be good to go with merging this.

Thanks.

@SlouchyButton
Copy link
Author

FTR, updating sqlite3 this high is luckily not a problem since this is only affecting the Ruby 3.3 branch.

Just noticed this comment was edited later, would it actually be a problem normally? I mean from what I have gathered the SQLite is used only for tests of the application and as far as I know we are not using the SQLite directly, but always through rails. The bundler would resolve the dependencies if the changes were breaking to the point rails wouldn't be compatible, right?

The only other thing that comes to mind is that system libraries would be too old for some of the OSes where we use the older version of rails-ex, which might actually be a problem.

@SlouchyButton
Copy link
Author

bundle update sqlite3

Cool, thanks. Figured it'd be smth like that.

Just please squash the Remove BUNDLED WITH part added by bundler commit with the previous one, and we should be good to go with merging this.

Thanks.

AFAIK, GitHub should allow you to do that automatically when merging the PR.

@jackorp
Copy link
Collaborator

jackorp commented Jul 10, 2024

Just noticed this comment was edited later, would it actually be a problem normally? I mean from what I have gathered the SQLite is used only for tests of the application and as far as I know we are not using the SQLite directly, but always through rails. The bundler would resolve the dependencies if the changes were breaking to the point rails wouldn't be compatible, right?

It would usually be a problem, not in this case since we split the app into branches due to dependencies. The way we'd refresh the app is pick oldest supported Ruby (container, VM, whatever) and go from there. Since we use(d) 1 dependency set for multiple versions of Ruby we have to pay extra attention whether we can still run it with old as well as new Ruby and anything that we support in containers in between.

The problem would be caused by the presence of version of gem in lockfile. If lockfile says e.g. foolib 2.0.0, it will install exactly that or exit with error while trying (otherwise the lockfile would be near meaningless for deployments) and not re-resolve remotely and install a dep set that is different from one in lockfile. In this concrete case, sqlite3 2.0.2 restricts "Required Ruby Version" (version of actual Ruby) to >= 3.0, which would cause us problems with Ruby 2.5 if this still were the main branch. And since dependency resolving is done once and then used for testing different versions of Ruby in containers, we'd end up in situation where we have a different error during bundle install.

So despite not necessarily using sqlite3, it still is installed and loaded by bundler due to its presence in Gemfile.lock and will result in an error on dependency installation or during startup when whatever dependency resolver finds the installed Ruby violating gem's Ruby version constraint.
And this is relevant for all gems in lockfile. And actually I found it applies also to nokogiri.

The only other thing that comes to mind is that system libraries would be too old for some of the OSes where we use the older version of rails-ex, which might actually be a problem.

Surprisingly, I don't quite remember meeting this situation yet, though it is something to keep in mind. It's usually more "we have new Ruby too new for some of the gems in Gemfile.lock" , rather than other system libraries.

@jackorp
Copy link
Collaborator

jackorp commented Jul 10, 2024

AFAIK, GitHub should allow you to do that automatically when merging the PR.

Oh right, re-read the help on the button and it does that. Sounds good to me.

@SlouchyButton
Copy link
Author

Just noticed this comment was edited later, would it actually be a problem normally? I mean from what I have gathered the SQLite is used only for tests of the application and as far as I know we are not using the SQLite directly, but always through rails. The bundler would resolve the dependencies if the changes were breaking to the point rails wouldn't be compatible, right?

It would usually be a problem, not in this case since we split the app into branches due to dependencies. The way we'd refresh the app is pick oldest supported Ruby (container, VM, whatever) and go from there. Since we use(d) 1 dependency set for multiple versions of Ruby we have to pay extra attention whether we can still run it with old as well as new Ruby and anything that we support in containers in between.

The problem would be caused by the presence of version of gem in lockfile. If lockfile says e.g. foolib 2.0.0, it will install exactly that or exit with error while trying (otherwise the lockfile would be near meaningless for deployments) and not re-resolve remotely and install a dep set that is different from one in lockfile. In this concrete case, sqlite3 2.0.2 restricts "Required Ruby Version" (version of actual Ruby) to >= 3.0, which would cause us problems with Ruby 2.5 if this still were the main branch. And since dependency resolving is done once and then used for testing different versions of Ruby in containers, we'd end up in situation where we have a different error during bundle install.

So despite not necessarily using sqlite3, it still is installed and loaded by bundler due to its presence in Gemfile.lock and will result in an error on dependency installation or during startup when whatever dependency resolver finds the installed Ruby violating gem's Ruby version constraint. And this is relevant for all gems in lockfile. And actually I found it applies also to nokogiri.

The only other thing that comes to mind is that system libraries would be too old for some of the OSes where we use the older version of rails-ex, which might actually be a problem.

Surprisingly, I don't quite remember meeting this situation yet, though it is something to keep in mind. It's usually more "we have new Ruby too new for some of the gems in Gemfile.lock" , rather than other system libraries.

Oh right, thank you for clarification, for some reason I didn't realize that the specific version of ruby itself is a dependency. It makes sense now.

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.

2 participants