rubys / nokogumbo

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

Prevent failed execution when using Nokogiri > 1.11.0 #148

Closed tupaschoal closed 3 years ago

tupaschoal commented 4 years ago

Nokogiri, from 1.11.0 onwards:

Nokogiri::VersionInfo will no longer contain the following keys (previously these were set only when vendored libraries were being used)

libxml/libxml2_path libxml/libxslt_path

As there was already a fallback for downloading, I let it use that, not sure if we should come up with another inventive way of finding nokogiri's path for xml

flavorjones commented 4 years ago

Please note that this change isn't set it stone - if this is necessary info for nokogumbo to build, I'm happy to keep it. Just need y'all to weigh in.

Please see https://github.com/sparklemotion/nokogiri/pull/1788#issuecomment-621226365 for proof (!) that I was aware some breakage was likely to happen wrt libxml_path.

stevecheckoway commented 4 years ago

@flavorjones It's been a little while since I thought about this. For performance reasons, we want to build the Nokogumbo extension using libxml2 headers. When Nokogiri builds libxml2, we'd like to simply use the same headers when possible. When Nokogiri uses the system libxml2, we'd like to do that as well. Nokogumbo has a fallback path that downloads the libxml2 source and configures it in order to get the headers.

Nokogumbo also supports a mode where it doesn't link against libxml2 at all and everything is just slower.

@tupaschoal This looks good regardless of what method we ultimately use for selecting which headers to use.

flavorjones commented 4 years ago

OK. I'm hoping to get time this month to ship another nokogiri v1.11.0 release candidate, which should contain the headers in the extension directory (via @stevecheckoway's https://github.com/sparklemotion/nokogiri/pull/1788). Hopefully we'll be able to close this feedback loop then.

tupaschoal commented 3 years ago

I'll assume this is obsolete by now

flavorjones commented 3 years ago

@tupaschoal Yes, thanks for closing it. Nokogiri v1.11.0 and later should be working fine for Nokogumbo, but please let us know if you have problems with it.