search

rubys / nokogumbo

A Nokogiri interface to the Gumbo HTML5 parser.
Apache License 2.0
186 stars 114 forks source link

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

Closed flavorjones closed 3 years ago

flavorjones commented 3 years ago

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

See https://github.com/sparklemotion/nokogiri/pull/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 commented 3 years ago

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

flavorjones commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

flavorjones commented 3 years ago

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 commented 3 years ago

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

Any idea what's up with Windows?

flavorjones commented 3 years ago

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 commented 3 years ago

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 https://github.com/sparklemotion/nokogiri/issues/2167

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

flavorjones commented 3 years ago

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

flavorjones commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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