Skip to content
This repository has been archived by the owner on Aug 26, 2023. It is now read-only.

update extconf.rb to use Nokogiri's CPPFLAGS #163

Merged
merged 2 commits into from
Mar 12, 2021
Merged

update extconf.rb to use Nokogiri's CPPFLAGS #163

merged 2 commits into from
Mar 12, 2021

Conversation

flavorjones
Copy link
Collaborator

which are present starting in Nokogiri v1.11.0.rc4.

See sparklemotion/nokogiri#2145 for more information.

With this change, here's what compilation looks like when Nokogiri is built with libxml2:

/home/flavorjones/.rvm/rubies/ruby-2.7.2/bin/ruby -I. ../../../../ext/nokogumbo/extconf.rb
checking for whether -I/home/flavorjones/.rvm/gems/ruby-2.7.2/gems/nokogiri-1.11.0.rc3/ext/nokogiri is accepted as CFLAGS... yes
checking for whether -I/home/flavorjones/.rvm/gems/ruby-2.7.2/gems/nokogiri-1.11.0.rc3/ext/nokogiri/include is accepted as CFLAGS... yes
checking for whether -I/home/flavorjones/.rvm/gems/ruby-2.7.2/gems/nokogiri-1.11.0.rc3/ext/nokogiri/include/libxml2 is accepted as CFLAGS... yes
checking for libxml/tree.h... yes
checking for nokogiri.h... yes
creating Makefile

and here's what compilation looks like when Nokogiri is not built with libxml2:

checking for whether -I/home/flavorjones/.rvm/gems/ruby-2.7.2/gems/nokogiri-1.11.0.rc3/ext/nokogiri is accepted as CFLAGS... yes
checking for libxml/tree.h... no
checking for nokogiri.h... no
checking for xmlNewDoc() in -lxml2... yes
checking for nokogiri.h in /home/flavorjones/.rvm/gems/ruby-2.7.2/gems/nokogiri-1.11.0.rc3/ext/nokogiri... yes
creating Makefile

In a future update, once we've pinned the Nokogiri dependency to "~> 1.11", we should be able to remove the stanza that looks at libxml2_path.

@flavorjones
Copy link
Collaborator Author

Leaving this as "draft" until Nokogiri v1.11.0.rc4 is shipped, hopefully today.

@flavorjones flavorjones mentioned this pull request Dec 28, 2020
Closed
@flavorjones flavorjones marked this pull request as ready for review December 29, 2020 17:11
@flavorjones
Copy link
Collaborator Author

Nokogiri v1.11.0.rc4 has been released (https://github.com/sparklemotion/nokogiri/releases/tag/v1.11.0.rc4) and so I've marked this as "ready for review."

It looks like the gentoo image is having problems talking to rubygems.org, could be an issue with ca-certificates? Everything else looks green.

@stevecheckoway
Copy link
Collaborator

I need to investigate the gentoo test. I'm not entirely sure it's even a useful test to have. There was an issue filed about Nokogumbo not working on Gentoo so I constructed the test and it was indeed working (if memory serves).

@rubys
Copy link
Owner

rubys commented Dec 30, 2020

@stevecheckoway the gentoo action was passing last month (when I added it). See https://github.com/rubys/nokogumbo/actions/runs/388669684

@flavorjones
Copy link
Collaborator Author

Just wanted to bump this post-holidays. Nokogiri v1.11.0 is out which provides the cppflags information that this PR uses (when it's available -- not a hard requirement), and this change is backwards-compatible.

I believe the Gentoo CI issue is related to expired CA certs in the image.

@stevecheckoway
Copy link
Collaborator

I fixed the Gentoo test. I probably need to schedule regular pushes of the docker image.

Any idea what's up with Windows?

@flavorjones
Copy link
Collaborator Author

Ugh, I didn't see the Windows errors before, that looks like it has to do with the native nokogiri gems. I'll need to find a couple of hours to dig in on that, but I don't think it's related to this change. IWould you mind if I pinned Nokogiri to v1.10.10 in the test suite to confirm that?

@flavorjones
Copy link
Collaborator Author

flavorjones commented Jan 4, 2021 •
edited
Loading

Yeah, OK, pinning to v1.10.10 allows Windows to pass, which tells me that this change is OK to ship if you want to. I'll remove the nokogiri version commit.

Bug report created at sparklemotion/nokogiri#2167

Edit: passing suite can be seen at https://github.com/rubys/nokogumbo/actions/runs/461879340

@flavorjones
Copy link
Collaborator Author

I'd like to rebase this onto master once #166 and #167 are merged, to make sure CI is green.

@flavorjones
update extconf.rb to use Nokogiri's CPPFLAGS
5654530 
which are present starting in Nokogiri v1.11.0.rc4.

See sparklemotion/nokogiri#2145 for more information.

With this change, here's what compilation looks like when Nokogiri is
built with libxml2:

> /home/flavorjones/.rvm/rubies/ruby-2.7.2/bin/ruby -I. ../../../../ext/nokogumbo/extconf.rb
> checking for whether -I/home/flavorjones/.rvm/gems/ruby-2.7.2/gems/nokogiri-1.11.0.rc3/ext/nokogiri is accepted as CFLAGS... yes
> checking for whether -I/home/flavorjones/.rvm/gems/ruby-2.7.2/gems/nokogiri-1.11.0.rc3/ext/nokogiri/include is accepted as CFLAGS... yes
> checking for whether -I/home/flavorjones/.rvm/gems/ruby-2.7.2/gems/nokogiri-1.11.0.rc3/ext/nokogiri/include/libxml2 is accepted as CFLAGS... yes
> checking for libxml/tree.h... yes
> checking for nokogiri.h... yes
> creating Makefile

and here's what compilation looks like when Nokogiri is _not_ built with
libxml2:

> checking for whether -I/home/flavorjones/.rvm/gems/ruby-2.7.2/gems/nokogiri-1.11.0.rc3/ext/nokogiri is accepted as CFLAGS... yes
> checking for libxml/tree.h... no
> checking for nokogiri.h... no
> checking for xmlNewDoc() in -lxml2... yes
> checking for nokogiri.h in /home/flavorjones/.rvm/gems/ruby-2.7.2/gems/nokogiri-1.11.0.rc3/ext/nokogiri... yes
> creating Makefile

In a future update, once we've pinned the Nokogiri dependency to "~>
1.11", we should be able to remove the stanza that looks at
`libxml2_path`.
@flavorjones flavorjones mentioned this pull request Feb 27, 2021
Closed
flavorjones added a commit to sparklemotion/nokogiri that referenced this pull request Mar 7, 2021
@flavorjones
fix(windows): ensure libxml2 symbols are exported from nokogiri.so
7ace791 
Related to #2167
Related to rubys/nokogumbo#163
flavorjones added a commit to sparklemotion/nokogiri that referenced this pull request Mar 7, 2021
@flavorjones
fix(windows): ensure nokogiri symbols are exported from nokogiri.so
bb7cab7 
Related to #2167
Related to rubys/nokogumbo#163
@flavorjones
fix: use Nokogiri's LDFLAGS to link against precompiled libs
a41ab09 
This is necessary on Windows where unresolved symbols aren't
allowed. We also limit compatibility with Nokogiri's precompiled libraries
to Nokogiri >= 1.11.2 on Windows for this reason.

Related to:
- sparklemotion/nokogiri#2145
- sparklemotion/nokogiri#2167
- sparklemotion/nokogiri#2202
@flavorjones
Copy link
Collaborator Author

Most recent commit branches in Windows based on whether Nokogiri is >= v1.11.2 or not. CI is green for < v1.11.2, I'd like it to go green after I release that version of Nokogiri, and then I'll merge this.

@flavorjones
Copy link
Collaborator Author

OK, link to green windows build against nokogiri v1.11.1 is https://github.com/rubys/nokogumbo/pull/163/checks?check_run_id=2056840407

I'm now going to kick it off to see if we can get a green build against v1.11.2.

@flavorjones
Copy link
Collaborator Author

It worked, it's happening: https://github.com/rubys/nokogumbo/pull/163/checks?check_run_id=2087760749

@flavorjones flavorjones merged commit 7a6c04d into rubys:master Mar 12, 2021
@flavorjones flavorjones deleted the flavorjones-add-support-for-precompiled-native-nokogiri-gems branch March 12, 2021 13:07
@flavorjones flavorjones mentioned this pull request Mar 17, 2021
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@stevecheckoway stevecheckoway stevecheckoway left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@flavorjones @stevecheckoway @rubys