introduce the "static" parameter to mkmf_config

which will: - conditionally pass "--static" to pkg-config - rewrite ldflags and libflags to link against the static archive - remember what static archives it's already seen, so any pkg-config packages that depend on it will also get its flags rewritten
flavorjones · Sep 17, 2023 · 0afc888 · 0afc888
1 parent 6b61ce6
commit 0afc888
Show file tree

Hide file tree

Showing 3 changed files with 125 additions and 33 deletions.
diff --git a/examples/Rakefile b/examples/Rakefile
@@ -128,18 +128,19 @@ yaml.files = [{
                 url: "https://github.com/yaml/libyaml/releases/download/0.2.5/yaml-0.2.5.tar.gz",
                 sha256: "c642ae9b75fee120b2d96c712538bd2cf283228d2337df2cf2988e3c02678ef4",
               }]
+yaml.configure_options << "CFLAGS=-fPIC"
 recipes.unshift(yaml)
 recipe_hooks["yaml"] = lambda do |recipe|
-  recipe.mkmf_config(pkg: "yaml-0.1")
+  recipe.mkmf_config(pkg: "yaml-0.1", static: "yaml")
 
-  expected = File.join(recipe.path, "lib")
-  $LIBPATH.include?(expected) or raise(<<~MSG)
-    assertion failed: $LIBPATH not updated correctly:
-    #{$LIBPATH}
+  expected = File.join(recipe.path, "lib", "libyaml.a")
+  $libs.include?(expected) or raise(<<~MSG)
+    assertion failed: $libs not updated correctly:
+    #{$libs}
     should have included '#{expected}'
   MSG
 
-  unless have_library("yaml", "yaml_get_version", "yaml.h")
+  unless have_func("yaml_get_version", "yaml.h")
     raise("could not find libyaml development environment")
   end
 end

diff --git a/lib/mini_portile2/mini_portile.rb b/lib/mini_portile2/mini_portile.rb
@@ -27,6 +27,8 @@ def edit_path(path)
   end
 end
 
+$MINI_PORTILE_STATIC_LIBS = {}
+
 class MiniPortile
   DEFAULT_TIMEOUT = 10
 
@@ -267,7 +269,15 @@ def activate
     end
   end
 
-  def mkmf_config(pkg: nil, dir: nil)
+  # pkg: the pkg-config file name (without the .pc extension)
+  # dir: inject the directory path for the pkg-config file (probably only useful for tests)
+  # static: the name of the static library archive (without the "lib" prefix or the file extension), or nil for dynamic linking
+  #
+  # we might be able to be terribly clever and infer the name of the static archive file, but
+  # unfortunately projects have so much freedom in what they can report (for name, for libs, etc.)
+  # that it feels unreliable to try to do so, so I'm preferring to just have the developer make it
+  # explicit.
+  def mkmf_config(pkg: nil, dir: nil, static: nil)
     require "mkmf"
 
     if pkg
@@ -287,8 +297,13 @@ def mkmf_config(pkg: nil, dir: nil)
 
       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")
+      if static
+        ldflags = minimal_pkg_config(pcfile, "libs-only-L", "static")
+        libflags = minimal_pkg_config(pcfile, "libs-only-l", "static")
+      else
+        ldflags = minimal_pkg_config(pcfile, "libs-only-L")
+        libflags = minimal_pkg_config(pcfile, "libs-only-l")
+      end
     else
       output "Configuring MakeMakefile for #{@name} #{@version} (from #{path})\n"
 
@@ -300,6 +315,37 @@ def mkmf_config(pkg: nil, dir: nil)
       libflags = Dir.exist?(lib_path) ? "-l#{lib_name}" : ""
     end
 
+    if static
+      libdir = lib_path
+      if pcfile
+        variables = minimal_pkg_config(pcfile, "print-variables").split("\n").map(&:strip)
+        if variables.include?("libdir")
+          libdir = minimal_pkg_config(pcfile, "variable=libdir")
+        end
+      end
+
+      #
+      # keep track of the libraries we're statically linking against, and fix up ldflags and
+      # libflags to make sure we link statically against the recipe's libaries.
+      #
+      # this avoids the unintentionally dynamically linking against system libraries, and makes sure
+      # that if multiple pkg-config files reference each other that we are able to intercept flags
+      # from dependent packages that reference the static archive.
+      #
+      $MINI_PORTILE_STATIC_LIBS[static] = libdir
+      static_ldflags = $MINI_PORTILE_STATIC_LIBS.values.map { |v| "-L#{v}" }
+      static_libflags = $MINI_PORTILE_STATIC_LIBS.keys.map { |v| "-l#{v}" }
+
+      # remove `-L#{libdir}` and `-lfoo`. we don't need them since we link against the static
+      # archive using the full path.
+      ldflags = ldflags.shellsplit.reject { |f| static_ldflags.include?(f) }.shelljoin
+      libflags = libflags.shellsplit.reject { |f| static_libflags.include?(f) }.shelljoin
+
+      # prepend the full path to the static archive to the linker flags
+      static_archive = File.join(libdir, "lib#{static}.#{$LIBEXT}")
+      libflags = [static_archive, libflags].join(" ").strip
+    end
+
     # prefer this package by prepending to search paths and library flags
     #
     # convert the ldflags into a list of directories and append to $LIBPATH (instead of just using

diff --git a/test/test_mkmf_config.rb b/test/test_mkmf_config.rb
@@ -8,6 +8,12 @@ class TestMkmfConfig < TestCase
   LIBXML_PCP = File.join(__dir__, "assets", "pkgconf", "libxml2")
   LIBXSLT_PCP = File.join(__dir__, "assets", "pkgconf", "libxslt")
 
+  def make_recipe(name, version)
+    MiniPortile.new(name, version).tap do |recipe|
+      recipe.logger = StringIO.new # hush output
+    end
+  end
+
   def setup
     super
 
@@ -19,12 +25,11 @@ def setup
     $CFLAGS = "-xxx"
     $CXXFLAGS = "-xxx"
     $libs = "-lxxx"
+    $MINI_PORTILE_STATIC_LIBS = {}
 
     FileUtils.rm_rf(["tmp", "ports"]) # remove any previous test files
 
-    @recipe = MiniPortile.new("libfoo", "1.0.0").tap do |recipe|
-      recipe.logger = StringIO.new
-    end
+    @recipe = make_recipe("libfoo", "1.0.0")
   end
 
   def teardown
@@ -35,6 +40,7 @@ def teardown
     $CFLAGS = ""
     $CXXFLAGS = ""
     $libs = ""
+    $MINI_PORTILE_STATIC_LIBS = {}
     @save_env.each do |var, val|
       ENV[var] = val
     end
@@ -49,7 +55,7 @@ def test_mkmf_config_recipe_LIBPATH_global_lib_dir_does_not_exist
     refute_includes($libs.shellsplit, "-lfoo")
   end
 
-  def test_mkmf_config_recipe_LIBPATH_global
+  def test_mkmf_config_recipe_LIBPATH_global_not_static
     FileUtils.mkdir_p(recipe.lib_path)
 
     recipe.mkmf_config
@@ -61,6 +67,19 @@ def test_mkmf_config_recipe_LIBPATH_global
     assert_match(%r{-lfoo.*-lxxx}, $libs) # prepend
   end
 
+  def test_mkmf_config_recipe_LIBPATH_global_static
+    FileUtils.mkdir_p(recipe.lib_path)
+    static_lib_path = File.join(recipe.lib_path, "libfoo.#{$LIBEXT}")
+
+    recipe.mkmf_config(static: "foo")
+
+    refute_includes($LIBPATH, recipe.lib_path)
+
+    refute_includes($libs.shellsplit, "-lfoo") # note the recipe name is "libfoo"
+    assert_includes($libs.shellsplit, static_lib_path)
+    assert_match(%r{#{static_lib_path}.*-lxxx}, $libs) # prepend
+  end
+
   def test_mkmf_config_recipe_INCFLAGS_global_include_dir_does_not_exist
     recipe.mkmf_config
 
@@ -82,17 +101,36 @@ def test_mkmf_config_pkgconf_does_not_exist
     end
   end
 
-  def test_mkmf_config_pkgconf_LIBPATH_global
+  def test_mkmf_config_pkgconf_LIBPATH_global_not_static
     # 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($LIBPATH, "/foo/libxml2/2.11.5/lib")
     assert_operator($LIBPATH.index("/foo/libxml2/2.11.5/lib"), :<, $LIBPATH.index("xxx")) # prepend
+    refute_includes($LIBPATH, "/foo/zlib/1.3/lib")
 
     assert_includes($libs.shellsplit, "-lxml2")
     assert_match(%r{-lxml2.*-lxxx}, $libs) # prepend
+    refute_includes($libs.shellsplit, "-lz")
+  end
+
+  def test_mkmf_config_pkgconf_LIBPATH_global_static
+    # can't get the pkgconf utility to install on windows with ruby 2.3 in CI
+    skip if MiniPortile.windows? && RUBY_VERSION < "2.4"
+
+    static_lib_path = "/foo/libxml2/2.11.5/lib/libxml2.#{$LIBEXT}"
+
+    recipe.mkmf_config(pkg: "libxml-2.0", dir: LIBXML_PCP, static: "xml2")
+
+    refute_includes($LIBPATH, "/foo/libxml2/2.11.5/lib")
+    refute_includes($libs.shellsplit, "-lxml2")
+    assert_includes($libs.shellsplit, static_lib_path)
+    assert_match(%r{#{static_lib_path}.*-lxxx}, $libs) # prepend
+
+    assert_includes($LIBPATH, "/foo/zlib/1.3/lib") # from --static
+    assert_includes($libs.shellsplit, "-lz") # from --static
   end
 
   def test_mkmf_config_pkgconf_CFLAGS_global
@@ -120,37 +158,44 @@ def test_mkmf_config_pkgconf_path_accumulation
       refute_includes(pcpaths, LIBXSLT_PCP)
     end
 
-    recipe.mkmf_config(pkg: "libxml-2.0", dir: LIBXML_PCP)
+    make_recipe("libxml2", "2.11.5").tap do |recipe|
+      recipe.mkmf_config(pkg: "libxml-2.0", dir: LIBXML_PCP, static: "xml2")
 
-    ENV["PKG_CONFIG_PATH"].split(File::PATH_SEPARATOR).tap do |pcpaths|
-      assert_includes(pcpaths, LIBXML_PCP)
-      refute_includes(pcpaths, LIBXSLT_PCP)
+      ENV["PKG_CONFIG_PATH"].split(File::PATH_SEPARATOR).tap do |pcpaths|
+        assert_includes(pcpaths, LIBXML_PCP)
+        refute_includes(pcpaths, LIBXSLT_PCP)
+      end
     end
 
-    recipe.mkmf_config(pkg: "libxslt", dir: LIBXSLT_PCP)
+    make_recipe("libxslt", "1.13.8").tap do |recipe|
+      recipe.mkmf_config(pkg: "libxslt", dir: LIBXSLT_PCP, static: "xslt")
 
-    ENV["PKG_CONFIG_PATH"].split(File::PATH_SEPARATOR).tap do |pcpaths|
-      assert_includes(pcpaths, LIBXML_PCP)
-      assert_includes(pcpaths, LIBXSLT_PCP)
-    end
+      ENV["PKG_CONFIG_PATH"].split(File::PATH_SEPARATOR).tap do |pcpaths|
+        assert_includes(pcpaths, LIBXML_PCP)
+        assert_includes(pcpaths, LIBXSLT_PCP)
+      end
 
-    recipe.mkmf_config(pkg: "libexslt", dir: LIBXSLT_PCP)
+      recipe.mkmf_config(pkg: "libexslt", dir: LIBXSLT_PCP, static: "exslt")
+    end
 
-    $INCFLAGS.split.tap do |incflags|
+    $INCFLAGS.shellsplit.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|
+    $CFLAGS.shellsplit.tap do |cflags|
       assert_includes(cflags, "-ggdb3")
       assert_includes(cflags, "-Wno-deprecated-enum-enum-conversion")
     end
-    $libs.split.tap do |libflags|
-      assert_includes(libflags, "-lxml2")
-      assert_includes(libflags, "-lxslt")
-      assert_includes(libflags, "-lexslt")
+    refute_includes($LIBPATH, "/foo/libxml2/2.11.5/lib")
+    refute_includes($LIBPATH, "/foo/libxslt/1.1.38/lib")
+    assert_includes($LIBPATH, "/foo/zlib/1.3/lib") # from `--static`
+    $libs.shellsplit.tap do |libflags|
+      refute_includes(libflags, "-lxml2")
+      assert_includes(libflags, "/foo/libxml2/2.11.5/lib/libxml2.#{$LIBEXT}")
+      refute_includes(libflags, "-lxslt")
+      assert_includes(libflags, "/foo/libxslt/1.1.38/lib/libxslt.#{$LIBEXT}")
+      refute_includes(libflags, "-lexslt")
+      assert_includes(libflags, "/foo/libxslt/1.1.38/lib/libexslt.#{$LIBEXT}")
       assert_includes(libflags, "-lz") # from `--static`
     end
   end