Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API change: allow environment variables to be passed into MiniPortile#execute #99

Closed
flavorjones opened this issue Apr 28, 2021 · 0 comments · Fixed by #100
Closed

API change: allow environment variables to be passed into MiniPortile#execute #99

flavorjones opened this issue Apr 28, 2021 · 0 comments · Fixed by #100

Comments

@flavorjones
Copy link
Owner

MiniPortile#execute is essentially a wrapper around Process.spawn, but doesn't support passing in an environment variable hash.

Today the most common use case is to run autoconf configure scripts, which can take environment variables as arguments (using the VAR=VALUE syntax), and so we jam env vars into the configure_options.

But for specialized uses, sometimes the compile step needs to pass in environment variables (e.g., Nokogiri building libgumbo needs to set AR and RANLIB for cross-compiling).

I'm proposing extending execute to accept an option key :env which will be a hash of environment variables that's passed to Process.spawn as the first argument.

The benefits to doing this:

  • users could override any step to do specialized things without having to worry about cross-platform escaping
  • spaghetti code like that found in Nokogiri's extconf could be well-factored into the relevant step

Apologies for not having more time to explain this properly. Here's a patch that does what I'm talking about:

diff --git a/lib/mini_portile2/mini_portile.rb b/lib/mini_portile2/mini_portile.rb
index 109de5b..110fc2b 100644
--- a/lib/mini_portile2/mini_portile.rb
+++ b/lib/mini_portile2/mini_portile.rb
@@ -380,16 +380,21 @@ def extract_file(file, target)
     execute('extract', [tar_exe, "#{tar_compression_switch(filename)}xf", file, "-C", target], {:cd => Dir.pwd, :initial_message => false})
   end
 
+  # command could be an array of args, or one string containing a command passed to the shell. See
+  # Process.spawn for more information.
   def execute(action, command, options={})
     log_out    = log_file(action)
 
+    env = options[:env] || {}
     Dir.chdir (options.fetch(:cd){ work_path }) do
       if options.fetch(:initial_message){ true }
+        output "DEBUG: compile command is #{command.inspect}" if options[:debug]
         message "Running '#{action}' for #{@name} #{@version}... "
       end
 
       if Process.respond_to?(:spawn) && ! RbConfig.respond_to?(:java)
-        args = [command].flatten + [{[:out, :err]=>[log_out, "a"]}]
+        options = {[:out, :err]=>[log_out, "a"]}
+        args = [env, command, options].flatten
         pid = spawn(*args)
         Process.wait(pid)
       else
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant