feat: integrate better with MakeMakefile

If MakeMakefile is loaded, assume we're in an `extconf.rb` and:

- prepend a `-L lib_path` and `-l lib_name` to $LDFLAGS
- prepend `-I include_path` to $CPPFLAGS

This should mean we don't need to use the pkgconf trick in extconf.rb
anymore, which completely avoids the behavior of Fedora's pkgconf
described in #118.

I'm a little worried that this might break packages in some subtle
way, so it can be turned off by passing `mkmf: false` to `MiniPortile#activate`

also:

- fix up the windows paths in the the LDFLAGS env var and the tests
- use assert_includes for better failure messages
- extract public class methods MiniPortile.native_path and .posix_path

Author: flavorjones

examples/Rakefile

@@ -1,9 +1,9 @@
 require 'rbconfig'
-require 'mkmf'
 
 $: << File.expand_path(File.join(File.dirname(__FILE__), "../lib"))
 require "mini_portile2"
 
+$LDFLAGS = "-mimic-being-in-extconf" # mimic being in an extconf (mkmf.rb sets this global variable)
 recipes = []
 recipe_hooks = {}
 
@@ -131,12 +131,7 @@ yaml.files = [{
               }]
 recipes.push(yaml)
 recipe_hooks["yaml"] = lambda do |recipe|
-  expected = "-L" + File.join(recipe.path, "lib")
-  !$LDFLAGS.split.include?(expected) or raise "assertion failed on setup"
-
-  conf = pkg_config(File.join(recipe.path, "lib", "pkgconfig", "yaml-0.1.pc"))
-  puts "pkg_config: #{conf.inspect}"
-
+  expected = "-L" + MiniPortile.native_path(File.join(recipe.path, "lib"))
   $LDFLAGS.split.include?(expected) or raise(<<~MSG)
     assertion failed: LDFLAGS not updated correctly:
     #{$LDFLAGS}
@@ -173,6 +168,7 @@ namespace :ports do
     recipes.each do |recipe|
       puts "Artifacts of '#{recipe.name}' in '#{recipe.path}'"
     end
+    puts "LIBRARY_PATH: #{ENV['LIBRARY_PATH'].inspect}"
     puts "LDFLAGS: #{ENV['LDFLAGS'].inspect}, #{$LDFLAGS.inspect}"
   end
 end

lib/mini_portile2/mini_portile.rb

@@ -76,6 +76,24 @@ def self.target_cpu
     RbConfig::CONFIG['target_cpu']
   end
 
+  def self.native_path(path)
+    path = File.expand_path(path)
+    if File::ALT_SEPARATOR
+      path.tr(File::SEPARATOR, File::ALT_SEPARATOR)
+    else
+      path
+    end
+  end
+
+  def self.posix_path(path)
+    path = File.expand_path(path)
+    if File::ALT_SEPARATOR
+      "/" + path.tr(File::ALT_SEPARATOR, File::SEPARATOR).tr(":", File::SEPARATOR)
+    else
+      path
+    end
+  end
+
   def initialize(name, version, **kwargs)
     @name = name
     @version = version
@@ -218,34 +236,45 @@ def cook
     return true
   end
 
-  def activate
+  def activate(mkmf: defined?(MakeMakefile))
+    output "Activating #{@name} #{@version} (from #{port_path})..."
+
+    bin_path = File.join(port_path, 'bin')
+    include_path = File.join(port_path, 'include')
     lib_path = File.join(port_path, "lib")
-    vars = {
-      'PATH'          => File.join(port_path, 'bin'),
-      'CPATH'         => File.join(port_path, 'include'),
-      'LIBRARY_PATH'  => lib_path
-    }.reject { |env, path| !File.directory?(path) }
 
-    output "Activating #{@name} #{@version} (from #{port_path})..."
-    vars.each do |var, path|
-      full_path = native_path(path)
+    if Dir.exist?(bin_path)
+      ENV["PATH"] = ENV.fetch("PATH", "").split(File::PATH_SEPARATOR)
+                      .unshift(native_path(bin_path)).join(File::PATH_SEPARATOR)
+    end
 
-      # save current variable value
-      old_value = ENV[var] || ''
+    if Dir.exist?(include_path)
+      ENV["CPATH"] = ENV.fetch("CPATH", "").split(File::PATH_SEPARATOR)
+                       .unshift(native_path(include_path)).join(File::PATH_SEPARATOR)
 
-      unless old_value.include?(full_path)
-        ENV[var] = "#{full_path}#{File::PATH_SEPARATOR}#{old_value}"
+      if mkmf
+        $CPPFLAGS = ($CPPFLAGS || "").split
+                      .unshift("-I#{native_path(include_path)}")
+                      .join(" ")
       end
     end
 
-    # rely on LDFLAGS when cross-compiling
-    if File.exist?(lib_path) && (@host != @original_host)
-      full_path = File.expand_path(lib_path)
+    if Dir.exist?(lib_path)
+      ENV["LIBRARY_PATH"] = ENV.fetch("LIBRARY_PATH", "").split(File::PATH_SEPARATOR)
+                              .unshift(native_path(lib_path)).join(File::PATH_SEPARATOR)
 
-      old_value = ENV.fetch("LDFLAGS", "")
+      ldflag ="-L#{native_path(lib_path)}"
+
+      if (@host != @original_host)
+        # rely on LDFLAGS when cross-compiling
+        ENV["LDFLAGS"] = ENV.fetch("LDFLAGS", "").split.unshift(ldflag).join(" ")
+      end
 
-      unless old_value.include?(full_path)
-        ENV["LDFLAGS"] = "-L#{full_path} #{old_value}".strip
+      if mkmf
+        $LDFLAGS = ($LDFLAGS || "").split
+                     .unshift("-l#{name}")
+                     .unshift(ldflag)
+                     .join(" ")
       end
     end
   end
@@ -265,21 +294,11 @@ def make_cmd
   private
 
   def native_path(path)
-    path = File.expand_path(path)
-    if File::ALT_SEPARATOR
-      path.tr(File::SEPARATOR, File::ALT_SEPARATOR)
-    else
-      path
-    end
+    MiniPortile.native_path(path)
   end
 
   def posix_path(path)
-    path = File.expand_path(path)
-    if File::ALT_SEPARATOR
-      "/" + path.tr(File::ALT_SEPARATOR, File::SEPARATOR).tr(":", File::SEPARATOR)
-    else
-      path
-    end
+    MiniPortile.posix_path(path)
   end
 
   def tmp_path

test/test_activate.rb

@@ -6,9 +6,11 @@ class TestActivate < TestCase
   def setup
     super
 
-    @save_env = %w[PATH CPATH LIBRARY_PATH].inject({}) do |env, var|
+    @save_env = %w[PATH CPATH LIBRARY_PATH LDFLAGS].inject({}) do |env, var|
       env.update(var => ENV[var])
     end
+    $LDFLAGS = nil
+    $CPPFLAGS = nil
 
     FileUtils.rm_rf(["tmp", "ports"]) # remove any previous test files
 
@@ -20,6 +22,8 @@ def setup
   def teardown
     FileUtils.rm_rf(["tmp", "ports"]) # remove any previous test files
 
+    $LDFLAGS = nil
+    $CPPFLAGS = nil
     @save_env.each do |var, val|
       ENV[var] = val
     end
@@ -28,79 +32,131 @@ def teardown
   end
 
   def test_PATH_env_var_when_bin_does_not_exist
+    ENV["PATH"] = "foo"
     refute(Dir.exist?(bin_path))
-    refute(path_elements('PATH').include?(bin_path))
+    refute_includes(path_elements('PATH'), bin_path)
 
     recipe.activate
 
-    refute(path_elements('PATH').include?(bin_path))
+    refute_includes(path_elements('PATH'), bin_path)
   end
 
   def test_PATH_env_var_when_bin_exists
+    ENV["PATH"] = "foo"
     FileUtils.mkdir_p(bin_path)
-    refute(path_elements('PATH').include?(bin_path))
+    refute_includes(path_elements('PATH'), bin_path)
 
     recipe.activate
 
-    assert(path_elements('PATH').include?(bin_path))
+    assert_includes(path_elements('PATH'), bin_path)
+    assert_equal(path_elements('PATH').first, bin_path)
   end
 
   def test_CPATH_env_var_when_include_does_not_exist
+    ENV["CPATH"] = "foo"
     refute(Dir.exist?(include_path))
-    refute(path_elements('CPATH').include?(include_path))
+    refute_includes(path_elements('CPATH'), include_path)
 
     recipe.activate
 
-    refute(path_elements('CPATH').include?(include_path))
+    refute_includes(path_elements('CPATH'), include_path)
   end
 
   def test_CPATH_env_var_when_include_exists
+    ENV["CPATH"] = "foo"
     FileUtils.mkdir_p(include_path)
-    refute(path_elements('CPATH').include?(include_path))
+    refute_includes(path_elements('CPATH'), include_path)
 
     recipe.activate
 
-    assert(path_elements('CPATH').include?(include_path))
+    assert_includes(path_elements('CPATH'), include_path)
+    assert_equal(path_elements('CPATH').first, include_path)
   end
 
   def test_LIBRARY_PATH_env_var_when_lib_does_not_exist
+    ENV["LIBRARY_PATH"] = "foo"
     refute(Dir.exist?(lib_path))
-    refute(path_elements('LIBRARY_PATH').include?(lib_path))
+    refute_includes(path_elements('LIBRARY_PATH'), lib_path)
 
     recipe.activate
 
-    refute(path_elements('LIBRARY_PATH').include?(lib_path))
+    refute_includes(path_elements('LIBRARY_PATH'), lib_path)
   end
 
   def test_LIBRARY_PATH_env_var_when_lib_exists
+    ENV["LIBRARY_PATH"] = "foo"
     FileUtils.mkdir_p(lib_path)
-    refute(path_elements('LIBRARY_PATH').include?(lib_path))
+    refute_includes(path_elements('LIBRARY_PATH'), lib_path)
 
     recipe.activate
 
-    assert(path_elements('LIBRARY_PATH').include?(lib_path))
+    assert_includes(path_elements('LIBRARY_PATH'), lib_path)
+    assert_equal(path_elements('LIBRARY_PATH').first, lib_path)
   end
 
   def test_LDFLAGS_env_var_when_not_cross_compiling
+    ENV["LDFLAGS"] = "-lfoo"
     FileUtils.mkdir_p(lib_path)
     assert_equal(recipe.host, recipe.original_host) # assert on setup)
 
-    refute(flag_elements('LDFLAGS').include?("-L#{lib_path}"))
+    refute_includes(flag_elements('LDFLAGS'), "-L#{lib_path}")
 
     recipe.activate
 
-    refute(flag_elements('LDFLAGS').include?("-L#{lib_path}"))
+    refute_includes(flag_elements('LDFLAGS'), "-L#{lib_path}")
   end
 
   def test_LDFLAGS_env_var_when_cross_compiling
+    ENV["LDFLAGS"] = "-lfoo"
     recipe.host = recipe.original_host + "-x" # make them not-equal
     FileUtils.mkdir_p(lib_path)
 
-    refute(flag_elements('LDFLAGS').include?("-L#{lib_path}"))
+    refute_includes(flag_elements('LDFLAGS'), "-L#{lib_path}")
 
     recipe.activate
 
-    assert(flag_elements('LDFLAGS').include?("-L#{lib_path}"))
+    assert_includes(flag_elements('LDFLAGS'), "-L#{lib_path}")
+    assert_equal(flag_elements('LDFLAGS').first, "-L#{lib_path}")
+  end
+
+  def test_LDFLAGS_global_not_set_when_mkmf_is_false
+    $LDFLAGS = "-lm"
+    FileUtils.mkdir_p(lib_path)
+
+    recipe.activate(mkmf: false)
+
+    refute_includes($LDFLAGS.split, "-l#{recipe.name}")
+    refute_includes($LDFLAGS.split, "-L#{lib_path}")
+  end
+
+  def test_LDFLAGS_global_set_in_mkmf
+    $LDFLAGS = '-lm'
+    FileUtils.mkdir_p(lib_path)
+
+    recipe.activate(mkmf: true)
+
+    assert_includes($LDFLAGS.split, "-l#{recipe.name}")
+    assert_includes($LDFLAGS.split, "-L#{lib_path}")
+    assert_equal($LDFLAGS.split.first, "-L#{lib_path}")
+  end
+
+  def test_CPPFLAGS_global_not_set_when_mkmf_is_false
+    $CPPFLAGS = "-Ifoo"
+    FileUtils.mkdir_p(include_path)
+
+    recipe.activate(mkmf: false)
+
+    refute_includes($CPPFLAGS.split, "-I#{include_path}")
+  end
+
+  def test_CPPFLAGS_global_set_in_mkmf
+    $CPPFLAGS = '-Ifoo'
+    FileUtils.mkdir_p(include_path)
+
+    recipe.activate(mkmf: true)
+
+    assert_includes($CPPFLAGS.split, "-I#{include_path}")
+    assert_equal($CPPFLAGS.split.first, "-I#{include_path}")
   end
 
   private
@@ -114,14 +170,14 @@ def flag_elements(varname)
   end
 
   def bin_path
-    File.join(recipe.path, "bin")
+    MiniPortile.native_path(File.join(recipe.path, "bin"))
   end
 
   def include_path
-    File.join(recipe.path, "include")
+    MiniPortile.native_path(File.join(recipe.path, "include"))
   end
 
   def lib_path
-    File.join(recipe.path, "lib")
+    MiniPortile.native_path(File.join(recipe.path, "lib"))
   end
 end