Skip to content

Commit

Permalink
Merge pull request #2145 from sparklemotion/flavorjones-version-info-…
Browse files Browse the repository at this point in the history
…cppflags

feat: adds `nokogiri/cppflags` to VERSION_INFO

---

the presence of CPPFLAGS suitable for compilation against nokogiri and
the packaged libraries will allow us to deprecate
`libxml/libxml2_path` in v1.12.x

also:
- change VERSION_INFO["nokogiri"] from a string to a hash
- add VERSION_INFO["nokogiri"]["version"] to store the version string

**What problem is this PR intended to solve?**

I'd like to make sure we have a mechanism in place in v1.11.x that will replace `VERSION_INFO["libxml"]["libxml2_path"] so that we can deprecate it. With this value, another gem's extconf is as simple as:

```ruby
append_cflags(Nokogiri::VERSION_INFO["nokogiri"]["cppflags"])
have_libxml2 = have_header('libxml/tree.h')
have_ng = have_header('nokogiri.h')
```


**Have you included adequate test coverage?**

Yes, test coverage added to `test/test_version.rb` and `scripts/test-gem-installation`


**Does this change affect the behavior of either the C or the Java implementations?**

No.
  • Loading branch information
flavorjones authored Dec 24, 2020
2 parents 19e3010 + ba04a58 commit f82b28f
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 8 deletions.
17 changes: 15 additions & 2 deletions lib/nokogiri/version/info.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# frozen_string_literal: true
require "singleton"
require "shellwords"

module Nokogiri
class VersionInfo # :nodoc:
Expand Down Expand Up @@ -72,9 +73,21 @@ def warnings
end

def to_hash
header_directory = File.expand_path(File.join(File.dirname(__FILE__), "../../../ext/nokogiri"))
{}.tap do |vi|
vi["warnings"] = []
vi["nokogiri"] = Nokogiri::VERSION
vi["nokogiri"] = {}.tap do |nokogiri|
nokogiri["version"] = Nokogiri::VERSION

unless jruby?
cppflags = ["-I#{header_directory.shellescape}"]
if libxml2_using_packaged?
cppflags << "-I#{File.join(header_directory, "include").shellescape}"
cppflags << "-I#{File.join(header_directory, "include/libxml2").shellescape}"
end
nokogiri["cppflags"] = cppflags
end
end
vi["ruby"] = {}.tap do |ruby|
ruby["version"] = ::RUBY_VERSION
ruby["platform"] = ::RUBY_PLATFORM
Expand All @@ -92,7 +105,7 @@ def to_hash
libxml["patches"] = Nokogiri::LIBXML2_PATCHES

# this is for nokogumbo and shouldn't be forever
libxml["libxml2_path"] = File.expand_path(File.join(File.dirname(__FILE__), "../../../ext/nokogiri"))
libxml["libxml2_path"] = header_directory
else
libxml["source"] = "system"
end
Expand Down
32 changes: 26 additions & 6 deletions scripts/test-gem-installation
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ describe gemspec.full_name do
let(:nokogiri_header_files) { ["nokogiri.h", "xml_document.h", "xml_node.h"] }
let(:packaged_library_header_files) { ["libxml2/libxml/tree.h", "libxslt/xslt.h", "libexslt/exslt.h"] }

let(:headers_dirs) { Nokogiri::VERSION_INFO["nokogiri"]["cppflags"].map { |f| f.gsub(/^-I/, "") } }

it "loads the same version as the spec we've loaded" do
assert_equal(Nokogiri::VERSION, gemspec.version.to_s)
end
Expand All @@ -59,6 +61,12 @@ describe gemspec.full_name do
nokogiri_header_files.each do |header|
assert(File.file?(File.join(nokogiri_ext_dir, header)),
"expected #{header} to be installed in #{nokogiri_ext_dir}")

found = false
headers_dirs.each do |header_dir|
found = true if File.file?(File.join(header_dir, "nokogiri.h"))
end
assert(found, "expected to find nokogiri.h in one of: #{headers_dirs.inspect}")
end
end

Expand All @@ -76,16 +84,28 @@ describe gemspec.full_name do

describe "library" do
describe "packaged" do
it "declares where headers are installed" do
describe "for nokogumbo" do
# this is for nokogumbo and shouldn't be forever
assert_equal(nokogiri_ext_dir, Nokogiri::VERSION_INFO["libxml"]["libxml2_path"],
"expected Nokogiri::VERSION_INFO to point to #{nokogiri_ext_dir}")
it "declares where headers are installed" do
assert_equal(nokogiri_ext_dir, Nokogiri::VERSION_INFO["libxml"]["libxml2_path"],
"expected Nokogiri::VERSION_INFO to point to #{nokogiri_ext_dir}")
end

it "installs packaged libraries' headers" do
packaged_library_header_files.each do |header|
assert(File.file?(File.join(nokogiri_include_dir, header)),
"expected #{header} to be installed in #{nokogiri_include_dir}")
end
end
end

it "installs packaged libraries' headers" do
it "points to packaged libraries' headers" do
packaged_library_header_files.each do |header|
assert(File.file?(File.join(nokogiri_include_dir, header)),
"expected #{header} to be installed in #{nokogiri_include_dir}")
found = false
headers_dirs.each do |header_dir|
found = true if File.file?(File.join(header_dir, header))
end
assert(found, "expected to find #{header} in one of: #{headers_dirs.inspect}")
end
end
end if Nokogiri::VersionInfo.instance.libxml2_using_packaged?
Expand Down
9 changes: 9 additions & 0 deletions test/test_version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@ module TestVersionInfoTests
def test_version_info_basics
assert_match(VERSION_MATCH, Nokogiri::VERSION)

assert_equal(Nokogiri::VERSION, Nokogiri::VERSION_INFO["nokogiri"]["version"])

if jruby?
refute(Nokogiri::VERSION_INFO["nokogiri"].has_key?("cppflags"), "did not expect cppflags")
else
# cppflags are more fully tested in scripts/test-gem-installation
assert_kind_of(Array, Nokogiri::VERSION_INFO["nokogiri"]["cppflags"], "expected cppflags to be an array")
end

assert_equal(::RUBY_VERSION, Nokogiri::VERSION_INFO["ruby"]["version"])
assert_equal(::RUBY_PLATFORM, Nokogiri::VERSION_INFO["ruby"]["platform"])
assert_equal(::Gem::Platform.local.to_s, Nokogiri::VERSION_INFO["ruby"]["gem_platform"])
Expand Down

0 comments on commit f82b28f

Please sign in to comment.