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

Upgrade nokogiri (development dependency) to 1.13.x #4310

Closed
ggrossetie opened this issue Jul 7, 2022 · 4 comments
Closed

Upgrade nokogiri (development dependency) to 1.13.x #4310

ggrossetie opened this issue Jul 7, 2022 · 4 comments
Assignees
Milestone

Comments

@ggrossetie
Copy link
Member

We might be able to remove the BUNDLE_BUILD__NOKOGIRI=--use-system-libraries environment variable in:

run: echo 'BUNDLE_BUILD__NOKOGIRI=--use-system-libraries' >> $GITHUB_ENV

I believe we are using --use-system-libraries because of this issue: flavorjones/mini_portile#101
Since this issue is now fixed in version 2.5.2 and Nokogiri 1.13.x is using version 2.8+ of mini_portile, I think it would make sense to upgrade.

Initial discussion: https://asciidoctor.zulipchat.com/#narrow/stream/279656-dev-.F0.9F.9B.A0.EF.B8.8F/topic/bundle.20install.20in.20asciidoctor.20with.20Ruby.203.2E1.20.28on.20Ubuntu.29

@mojavelinux
Copy link
Member

mojavelinux commented Jul 7, 2022

The main reason we're using --use-system-libraries is because it's takes an excruciatingly long time to rebuild nokogiri. By linking instead, it speeds up the installation dramatically.

mojavelinux added a commit to mojavelinux/asciidoctor that referenced this issue Jul 10, 2022
@mojavelinux mojavelinux self-assigned this Jul 10, 2022
mojavelinux added a commit to mojavelinux/asciidoctor that referenced this issue Jul 10, 2022
@mojavelinux
Copy link
Member

The problem with upgrading is that it requires Ruby >= 2.6 and we test back to Ruby 2.3.

We are reevaluating how we drop old versions of Ruby, but suffice to say, core has to be prudent with the minimum Ruby version to support all the Linux distributions and macOS. So I don't think we can make this jump yet.

What exactly is the reason for wanting to upgrade? All the tests are passing in CI right now.

@ggrossetie
Copy link
Member Author

ggrossetie commented Jul 10, 2022

As an intermediate step we could upgrade to Nokogiri 1.12 which uses mini_portile 2.6.1 and requires Ruby 2.5.

The main reason is that it does not install on Ubuntu 20.4 LTS with a recent version of Ruby when building from source (i.e. without --use-system-libraries).
It's not a big deal but we might want to add a note in the contributing guide.

@mojavelinux
Copy link
Member

This was resolved by #4339.

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

No branches or pull requests

2 participants