Merge pull request #134 from flavorjones/flavorjones-improve-mkmf-con…

…fig-20230917

improve mkmf config

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

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
 
@@ -237,11 +239,10 @@ def cook
   end
 
   def activate
-    lib_path = File.join(port_path, "lib")
     vars = {
       'PATH'          => File.join(port_path, 'bin'),
-      'CPATH'         => File.join(port_path, 'include'),
-      'LIBRARY_PATH'  => lib_path
+      'CPATH'         => include_path,
+      'LIBRARY_PATH'  => lib_path,
     }.reject { |env, path| !File.directory?(path) }
 
     output "Activating #{@name} #{@version} (from #{port_path})..."
@@ -268,11 +269,19 @@ 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
-      dir ||= File.join(path, "lib", "pkgconfig")
+      dir ||= File.join(lib_path, "pkgconfig")
       pcfile = File.join(dir, "#{pkg}.pc")
       unless File.exist?(pcfile)
         raise ArgumentError, "pkg-config file '#{pcfile}' does not exist"
@@ -288,42 +297,81 @@ 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"
 
-      include_path = File.join(path, "include")
-      lib_path = File.join(path, "lib")
-
       lib_name = name.sub(/\Alib/, "") # TODO: use delete_prefix when we no longer support ruby 2.4
 
-      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)
+      incflags = Dir.exist?(include_path) ? "-I#{include_path}" : ""
+      cflags = ""
+      ldflags = Dir.exist?(lib_path) ? "-L#{lib_path}" : ""
+      libflags = Dir.exist?(lib_path) ? "-l#{lib_name}" : ""
     end
 
-    if ldflags
-      libpaths = ldflags.split.map { |f| f.sub(/\A-L/, "") }
+    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 directories to the search path
+    # prefer this package by prepending to search paths and library flags
     #
-    # 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
+    # convert the ldflags into a list of directories and append to $LIBPATH (instead of just using
+    # $LDFLAGS) to ensure we get the `-Wl,-rpath` linker flag for re-finding shared libraries.
+    $INCFLAGS = [incflags, $INCFLAGS].join(" ").strip
+    libpaths = ldflags.shellsplit.map { |f| f.sub(/\A-L/, "") }
+    $LIBPATH = libpaths | $LIBPATH
+    $libs = [libflags, $libs].join(" ").strip
 
-    # 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
+    # prefer this package's compiler flags by appending them to the command line
+    $CFLAGS = [$CFLAGS, cflags].join(" ").strip
+    $CXXFLAGS = [$CXXFLAGS, cflags].join(" ").strip
   end
 
   def path
     File.expand_path(port_path)
   end
 
+  def include_path
+    File.join(path, "include")
+  end
+
+  def lib_path
+    File.join(path, "lib")
+  end
+
   def gcc_cmd
     (ENV["CC"] || @gcc_command || RbConfig::CONFIG["CC"] || "gcc").dup
   end