Merge pull request #2143 from sparklemotion/flavorjones-fine-tune-pac…

…kaging

fine tune packaging

---

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

There are a few open issues and pull requests about the packaging and installation of Nokogiri which this PR is trying to address:

- [Native gems: eliminate the "extra" dll · Issue #2076 · sparklemotion/nokogiri](#2076)
- [Native gems: ensure that mini_portile2 is not a dependency · Issue #2078 · sparklemotion/nokogiri](#2078)
- [Nokogumbo should be able to compile against Nokogiri by finding headers in extensions directory by stevecheckoway · Pull Request #1788 · sparklemotion/nokogiri](#1788)

More specifically, the code that's been doing the gem packaging is ad-hoc, brittle, and not very discoverable, and has little-to-no test coverage. This PR embarks on cleaning up some of that code, and erecting tests around the files that are packaged and installed, so that I have the confidence to refactor and update this code going forward.

I want to give a shoutout to @stevecheckoway whose original PR at #1788 inspired this approach, though unfortunately I didn't use most of that PR because the precompiled native gems, added since that PR, complicated things.


**Have you included adequate test coverage?**

Ayuh. Notably two new scripts have been added and will be run in CI:

- `scripts/test-gem-file-contents` which makes assertions on the contents of the tarball and the packaged gemspec
- `scripts/test-gem-installation` which asserts on the filesystem left behind by the gem installation process


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

This affects only installation-time, the functional behavior of Nokogiri is not changed.

.gitignore

@@ -23,9 +23,10 @@ coverage
 ext/java/Canna
 ext/java/nokogiri/**/*.class
 ext/nokogiri/*.dll
+ext/nokogiri/include
 gems
-lib/nokogiri/**/nokogiri.so
 lib/nokogiri/**/nokogiri.bundle
+lib/nokogiri/**/nokogiri.so
 lib/nokogiri/nokogiri.jar
 pkg
 ports

Rakefile

@@ -43,6 +43,8 @@ HOE = Hoe.spec "nokogiri" do |hoe|
     "lib/nokogiri/[0-9].[0-9]",
     "coverage",
     "concourse/images/*.generated",
+    "ext/nokogiri/include",
+    "tmp",
   ]
   hoe.clean_globs += Dir.glob("ports/*").reject { |d| d =~ %r{/archives$} }
 

concourse/tasks/gem-test/gem-build-java.sh

@@ -20,6 +20,10 @@ bundle exec rake set-version-to-timestamp
 
 bundle exec rake java gem
 
+if [ -e ./scripts/test-gem-file-contents ] ; then
+  ./scripts/test-gem-file-contents pkg/nokogiri*java.gem
+fi
+
 mkdir -p ${OUTPUT_DIR}
 cp -v pkg/nokogiri*java.gem ${OUTPUT_DIR}
 sha256sum ${OUTPUT_DIR}/*

concourse/tasks/gem-test/gem-build.sh

@@ -32,6 +32,10 @@ else
   bundle exec rake clean compile
 
   bundle exec rake gem
+
+  if [ -e ./scripts/test-gem-file-contents ] ; then
+    ./scripts/test-gem-file-contents pkg/nokogiri*.gem
+  fi
 fi
 
 mkdir -p ${OUTPUT_DIR}

concourse/tasks/gem-test/gem-install-and-test.sh

@@ -26,6 +26,10 @@ pushd nokogiri
   bundle install --local || bundle install
   bundle info nokogiri
 
+  if [ -e ./scripts/test-gem-installation ] ; then
+    ./scripts/test-gem-installation
+  fi
+
   bundle exec rake test:cmd > run-test
   rm -rf lib ext # ensure we can't use the local files
   bundle exec bash run-test

ext/nokogiri/extconf.rb

@@ -445,6 +445,14 @@ def process_recipe(name, version, static_p, cross_p)
   end
 end
 
+def copy_packaged_libraries_headers(to_path:, from_recipes:)
+  FileUtils.rm_rf(to_path, secure: true)
+  FileUtils.mkdir(to_path)
+  from_recipes.each do |recipe|
+    FileUtils.cp_r(Dir[File.join(recipe.path, 'include/*')], to_path)
+  end
+end
+
 def do_help
   print NOKOGIRI_HELP_MESSAGE
   exit! 0
@@ -762,6 +770,20 @@ def configure
 
 have_func('vasprintf')
 
+unless using_system_libraries?
+  if cross_build_p
+    # When precompiling native gems, copy packaged libraries' headers to ext/nokogiri/include
+    # These are packaged up by the cross-compiling callback in the ExtensionTask
+    copy_packaged_libraries_headers(to_path: File.join(PACKAGE_ROOT_DIR, "ext/nokogiri/include"),
+                                    from_recipes: [libxml2_recipe, libxslt_recipe])
+  else
+    # When compiling during installation, install packaged libraries' header files into ext/nokogiri/include
+    copy_packaged_libraries_headers(to_path: "include",
+                                    from_recipes: [libxml2_recipe, libxslt_recipe])
+    $INSTALLFILES << ["include/**/*.h", "$(rubylibdir)"]
+  end
+end
+
 create_makefile('nokogiri/nokogiri')
 
 if enable_config('clean', true)

lib/nokogiri/version/info.rb

@@ -90,6 +90,9 @@ def to_hash
               libxml["source"] = "packaged"
               libxml["precompiled"] = libxml2_precompiled?
               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"))
             else
               libxml["source"] = "system"
             end

rakelib/cross-ruby.rake

@@ -279,7 +279,8 @@ namespace "gem" do
 
   desc "build a jruby gem"
   task "jruby" do
-    RakeCompilerDock.sh "gem install bundler --no-document && bundle && rake java gem", rubyvm: "jruby"
+    RakeCompilerDock.sh("gem install bundler --no-document && bundle && rake java gem",
+                        rubyvm: "jruby", platform: "jruby")
   end
 
   desc "build native gems for windows"
@@ -296,11 +297,14 @@ if java?
   require "rake/javaextensiontask"
   Rake::JavaExtensionTask.new("nokogiri", HOE.spec) do |ext|
     jruby_home = RbConfig::CONFIG['prefix']
+    jars = ["#{jruby_home}/lib/jruby.jar"] + FileList['lib/*.jar']
+
+    ext.gem_spec.files.reject! { |path| File.fnmatch?("ext/nokogiri/*.h", path) }
+
     ext.ext_dir = 'ext/java'
     ext.lib_dir = 'lib/nokogiri'
     ext.source_version = '1.6'
     ext.target_version = '1.6'
-    jars = ["#{jruby_home}/lib/jruby.jar"] + FileList['lib/*.jar']
     ext.classpath = jars.map { |x| File.expand_path x }.join ':'
     ext.debug = true if ENV['JAVA_DEBUG']
   end
@@ -316,21 +320,19 @@ else
   dependencies = YAML.load_file("dependencies.yml")
 
   task gem_build_path do
-    %w[libxml2 libxslt].each do |lib|
+    ["libxml2", "libxslt"].each do |lib|
       version = dependencies[lib]["version"]
       archive = File.join("ports", "archives", "#{lib}-#{version}.tar.gz")
       add_file_to_gem archive
+
       patchesdir = File.join("patches", lib)
       patches = `#{['git', 'ls-files', patchesdir].shelljoin}`.split("\n").grep(/\.patch\z/)
-      patches.each { |patch|
-        add_file_to_gem patch
-      }
-      (untracked = Dir[File.join(patchesdir, '*.patch')] - patches).empty? or
-        at_exit {
-          untracked.each { |patch|
-            puts "** WARNING: untracked patch file not added to gem: #{patch}"
-          }
-        }
+      patches.each { |patch| add_file_to_gem patch }
+
+      untracked = Dir[File.join(patchesdir, '*.patch')] - patches
+      at_exit do
+        untracked.each { |patch| puts "** WARNING: untracked patch file not added to gem: #{patch}" }
+      end
     end
   end
 
@@ -349,6 +351,21 @@ else
 
       spec.files.reject! { |path| File.fnmatch?('ports/*', path) }
       spec.dependencies.reject! { |dep| dep.name=='mini_portile2' }
+
+      # when pre-compiling a native gem, package all the C headers sitting in ext/nokogiri/include
+      # which were copied there in the $INSTALLFILES section of extconf.rb.
+      # (see scripts/test-gem-file-contents and scripts/test-gem-installation for tests)
+      headers_dir = "ext/nokogiri/include"
+
+      ["libxml2", "libxslt"].each do |lib|
+        unless File.directory?(File.join(headers_dir, lib))
+          raise "#{lib} headers are not present in #{headers_dir}"
+        end
+      end
+
+      Dir.glob(File.join(headers_dir, "**", "*.h")).each do |header|
+        spec.files << header
+      end
     end
   end
 end

scripts/build-gems

@@ -24,3 +24,7 @@ bundle exec rake clean
 bundle exec rake gem:windows
 bundle exec rake gem:linux
 cp -v pkg/nokogiri-*{x86,x64}*.gem gems
+
+for f in gems/*.gem ; do
+  ./scripts/test-gem-file-contents $f
+done