Merge pull request #133 from flavorjones/flavorjones-more-precise-pkg…

…-config

feat: more precise implementation of mkmf_config

examples/Rakefile

@@ -132,10 +132,10 @@ recipes.unshift(yaml)
 recipe_hooks["yaml"] = lambda do |recipe|
   recipe.mkmf_config(pkg: "yaml-0.1")
 
-  expected = "-L" + File.join(recipe.path, "lib")
-  $LDFLAGS.split.include?(expected) or raise(<<~MSG)
-    assertion failed: LDFLAGS not updated correctly:
-    #{$LDFLAGS}
+  expected = File.join(recipe.path, "lib")
+  $LIBPATH.include?(expected) or raise(<<~MSG)
+    assertion failed: $LIBPATH not updated correctly:
+    #{$LIBPATH}
     should have included '#{expected}'
   MSG
 
@@ -174,10 +174,14 @@ namespace :ports do
     recipes.each do |recipe|
       puts "Artifacts of '#{recipe.name}' in '#{recipe.path}'"
     end
+    puts "---"
     puts "LIBRARY_PATH: #{ENV['LIBRARY_PATH'].inspect}"
     puts "LDFLAGS: #{ENV['LDFLAGS'].inspect}"
-    puts "$LDFLAGS: #{$LDFLAGS.inspect}"
+    puts "---"
+    puts "$INCFLAGS: #{$INCFLAGS.inspect}"
     puts "$CFLAGS: #{$CFLAGS.inspect}"
+    puts "$LIBPATH: #{$LIBPATH.inspect}"
+    puts "$libs: #{$libs.inspect}"
   end
 end
 

lib/mini_portile2/mini_portile.rb

@@ -286,8 +286,10 @@ def mkmf_config(pkg: nil, dir: nil)
       # append to PKG_CONFIG_PATH as we go, so later pkg-config files can depend on earlier ones
       ENV["PKG_CONFIG_PATH"] = [ENV["PKG_CONFIG_PATH"], dir].compact.join(File::PATH_SEPARATOR)
 
-      cflags = minimal_pkg_config(pcfile, "cflags")
-      ldflags = minimal_pkg_config(pcfile, "libs", "static")
+      incflags = minimal_pkg_config(pcfile, "cflags-only-I")
+      cflags = minimal_pkg_config(pcfile, "cflags-only-other")
+      ldflags = minimal_pkg_config(pcfile, "libs-only-L", "static")
+      libflags = minimal_pkg_config(pcfile, "libs-only-l", "static")
     else
       output "Configuring MakeMakefile for #{@name} #{@version} (from #{path})\n"
 
@@ -296,13 +298,26 @@ def mkmf_config(pkg: nil, dir: nil)
 
       lib_name = name.sub(/\Alib/, "") # TODO: use delete_prefix when we no longer support ruby 2.4
 
-      cflags = "-I#{include_path}" if Dir.exist?(include_path)
-      ldflags = "-L#{lib_path} -l#{lib_name}" if Dir.exist?(lib_path)
+      incflags = "-I#{include_path}" if Dir.exist?(include_path)
+      ldflags = "-L#{lib_path}" if Dir.exist?(lib_path)
+      libflags = "-l#{lib_name}" if Dir.exist?(lib_path)
     end
 
-    $CFLAGS << " " << cflags if cflags
-    $CXXFLAGS << " " << cflags if cflags
-    $LDFLAGS << " " << ldflags if ldflags
+    if ldflags
+      libpaths = ldflags.split.map { |f| f.sub(/\A-L/, "") }
+    end
+
+    # prefer this package by prepending directories to the search path
+    #
+    # use $LIBPATH instead of $LDFLAGS to ensure we get the `-Wl,-rpath` linker flag for re-finding
+    # shared libraries
+    $INCFLAGS = [incflags, $INCFLAGS].join(" ").strip if incflags
+    $LIBPATH = libpaths | $LIBPATH if libpaths
+
+    # prefer this package's flags by appending them to the command line
+    $CFLAGS = [$CFLAGS, cflags].join(" ").strip if cflags
+    $CXXFLAGS = [$CXXFLAGS, cflags].join(" ").strip if cflags
+    $libs = [$libs, libflags].join(" ").strip if libflags
   end
 
   def path

test/assets/pkgconf/libxml2/libxml-2.0.pc

@@ -10,4 +10,4 @@ Description: libXML library version2.
 Requires:
 Libs: -L${libdir} -lxml2
 Libs.private: -L/foo/zlib/1.3/lib -lz     -lm  
-Cflags: -I${includedir}/libxml2 
+Cflags: -I${includedir}/libxml2 -ggdb3

test/assets/pkgconf/libxslt/libxslt.pc

@@ -8,6 +8,6 @@ Name: libxslt
 Version: 1.1.38
 Description: XSLT library version 2.
 Requires: libxml-2.0
-Cflags: -I${includedir}
+Cflags: -I${includedir} -Wno-deprecated-enum-enum-conversion
 Libs: -L${libdir} -lxslt
 Libs.private: -lm

test/test_mkmf_config.rb

@@ -1,5 +1,7 @@
 require File.expand_path('../helper', __FILE__)
 
+require "mkmf" # initialize $LDFLAGS et al here, instead of in the middle of a test
+
 class TestMkmfConfig < TestCase
   attr_reader :recipe, :include_path, :lib_path
 
@@ -12,8 +14,11 @@ def setup
     @save_env = %w[PATH CPATH LIBRARY_PATH LDFLAGS PKG_CONFIG_PATH].inject({}) do |env, var|
       env.update(var => ENV[var])
     end
-    $LDFLAGS = ""
-    $CFLAGS = ""
+    $INCFLAGS = "-I/xxx"
+    $LIBPATH = ["xxx"]
+    $CFLAGS = "-xxx"
+    $CXXFLAGS = "-xxx"
+    $libs = "-lxxx"
 
     FileUtils.rm_rf(["tmp", "ports"]) # remove any previous test files
 
@@ -27,43 +32,50 @@ def setup
   def teardown
     FileUtils.rm_rf(["tmp", "ports"]) # remove any previous test files
 
-    $LDFLAGS = ""
+    $INCFLAGS = ""
+    $LIBPATH = []
     $CFLAGS = ""
+    $CXXFLAGS = ""
+    $libs = ""
     @save_env.each do |var, val|
       ENV[var] = val
     end
 
     super
   end
 
-  def test_mkmf_config_recipe_LDFLAGS_global_lib_dir_does_not_exist
+  def test_mkmf_config_recipe_LIBPATH_global_lib_dir_does_not_exist
     recipe.mkmf_config
 
-    refute_includes($LDFLAGS.split, "-L#{lib_path}")
-    refute_includes($LDFLAGS.split, "-lfoo")
+    refute_includes($LIBPATH, lib_path)
+    refute_includes($libs.split, "-lfoo")
   end
 
-  def test_mkmf_config_recipe_LDFLAGS_global
+  def test_mkmf_config_recipe_LIBPATH_global
     FileUtils.mkdir_p(lib_path)
 
     recipe.mkmf_config
 
-    assert_includes($LDFLAGS.split, "-L#{lib_path}")
-    assert_includes($LDFLAGS.split, "-lfoo") # note the recipe name is "libfoo"
+    assert_includes($LIBPATH, lib_path)
+    assert_operator($LIBPATH.index(lib_path), :<, $LIBPATH.index("xxx")) # prepend
+
+    assert_includes($libs.split, "-lfoo") # note the recipe name is "libfoo"
+    assert_match(%r{-lxxx.*-lfoo}, $libs) # append
   end
 
-  def test_mkmf_config_recipe_CFLAGS_global_include_dir_does_not_exist
+  def test_mkmf_config_recipe_INCFLAGS_global_include_dir_does_not_exist
     recipe.mkmf_config
 
-    refute_includes($CFLAGS.split, "-I#{include_path}")
+    refute_includes($INCFLAGS.split, "-I#{include_path}")
   end
 
-  def test_mkmf_config_recipe_CFLAGS_global
+  def test_mkmf_config_recipe_INCFLAGS_global
     FileUtils.mkdir_p(include_path)
 
     recipe.mkmf_config
 
-    assert_includes($CFLAGS.split, "-I#{include_path}")
+    assert_includes($INCFLAGS.split, "-I#{include_path}")
+    assert_match(%r{-I#{include_path}.*-I/xxx}, $INCFLAGS) # prepend
   end
 
   def test_mkmf_config_pkgconf_does_not_exist
@@ -72,14 +84,17 @@ def test_mkmf_config_pkgconf_does_not_exist
     end
   end
 
-  def test_mkmf_config_pkgconf_LDFLAGS_global
+  def test_mkmf_config_pkgconf_LIBPATH_global
     # can't get the pkgconf utility to install on windows with ruby 2.3 in CI
     skip if MiniPortile.windows? && RUBY_VERSION < "2.4"
 
     recipe.mkmf_config(pkg: "libxml-2.0", dir: LIBXML_PCP)
 
-    assert_includes($LDFLAGS.split, "-L/foo/libxml2/2.11.5/lib")
-    assert_includes($LDFLAGS.split, "-lxml2")
+    assert_includes($LIBPATH, "/foo/libxml2/2.11.5/lib")
+    assert_operator($LIBPATH.index("/foo/libxml2/2.11.5/lib"), :<, $LIBPATH.index("xxx")) # prepend
+
+    assert_includes($libs.split, "-lxml2")
+    assert_match(%r{-lxxx.*-lxml2}, $libs) # append
   end
 
   def test_mkmf_config_pkgconf_CFLAGS_global
@@ -88,7 +103,14 @@ def test_mkmf_config_pkgconf_CFLAGS_global
 
     recipe.mkmf_config(pkg: "libxml-2.0", dir: LIBXML_PCP)
 
-    assert_includes($CFLAGS.split, "-I/foo/libxml2/2.11.5/include/libxml2")
+    assert_includes($INCFLAGS.split, "-I/foo/libxml2/2.11.5/include/libxml2")
+    assert_match(%r{-I/foo/libxml2/2.11.5/include/libxml2.*-I/xxx}, $INCFLAGS) # prepend
+
+    assert_includes($CFLAGS.split, "-ggdb3")
+    assert_match(%r{-xxx.*-ggdb3}, $CFLAGS) # prepend
+
+    assert_includes($CXXFLAGS.split, "-ggdb3")
+    assert_match(%r{-xxx.*-ggdb3}, $CXXFLAGS) # prepend
   end
 
   def test_mkmf_config_pkgconf_path_accumulation
@@ -116,18 +138,22 @@ def test_mkmf_config_pkgconf_path_accumulation
 
     recipe.mkmf_config(pkg: "libexslt", dir: LIBXSLT_PCP)
 
+    $INCFLAGS.split.tap do |incflags|
+      assert_includes(incflags, "-I/foo/libxml2/2.11.5/include/libxml2")
+      assert_includes(incflags, "-I/foo/libxslt/1.1.38/include")
+    end
+    assert_includes($LIBPATH, "/foo/libxml2/2.11.5/lib")
+    assert_includes($LIBPATH, "/foo/libxslt/1.1.38/lib")
+    assert_includes($LIBPATH, "/foo/zlib/1.3/lib") # from `--static`
     $CFLAGS.split.tap do |cflags|
-      assert_includes(cflags, "-I/foo/libxml2/2.11.5/include/libxml2")
-      assert_includes(cflags, "-I/foo/libxslt/1.1.38/include")
+      assert_includes(cflags, "-ggdb3")
+      assert_includes(cflags, "-Wno-deprecated-enum-enum-conversion")
     end
-    $LDFLAGS.split.tap do |ldflags|
-      assert_includes(ldflags, "-L/foo/libxml2/2.11.5/lib")
-      assert_includes(ldflags, "-lxml2")
-      assert_includes(ldflags, "-L/foo/libxslt/1.1.38/lib")
-      assert_includes(ldflags, "-lxslt")
-      assert_includes(ldflags, "-lexslt")
-      assert_includes(ldflags, "-L/foo/zlib/1.3/lib") # from `--static`
-      assert_includes(ldflags, "-lz") # from `--static`
+    $libs.split.tap do |libflags|
+      assert_includes(libflags, "-lxml2")
+      assert_includes(libflags, "-lxslt")
+      assert_includes(libflags, "-lexslt")
+      assert_includes(libflags, "-lz") # from `--static`
     end
   end
 end