Merge pull request #24249 from Homebrew/env-build-state

Keep ENV build metadata off the shareable ENV

Library/Homebrew/extend/ENV/shared.rb

@@ -19,6 +19,9 @@ module SharedEnvExtension
 
   requires_ancestor { Sorbet::Private::Static::ENVClass }
 
+  autoload :State, "extend/ENV/state"
+  private_constant :State
+
   CC_FLAG_VARS = %w[CFLAGS CXXFLAGS OBJCFLAGS OBJCXXFLAGS].freeze
   private_constant :CC_FLAG_VARS
 
@@ -39,7 +42,7 @@ module SharedEnvExtension
   private_constant :SANITIZED_VARS
 
   sig { returns(T.nilable(String)) }
-  attr_reader :bottle_arch
+  def bottle_arch = build_environment_state.bottle_arch
 
   sig {
     params(
@@ -53,20 +56,21 @@ module SharedEnvExtension
   }
   def setup_build_environment(formula: nil, cc: nil, build_bottle: false, bottle_arch: nil, testing_formula: false,
                               debug_symbols: false)
-    @formula = T.let(formula, T.nilable(Formula))
-    @cc = T.let(cc, T.nilable(String))
-    @build_bottle = T.let(build_bottle, T.nilable(T::Boolean))
-    @bottle_arch = T.let(bottle_arch, T.nilable(String))
-    @debug_symbols = T.let(debug_symbols, T.nilable(T::Boolean))
-    @testing_formula = T.let(testing_formula, T.nilable(T::Boolean))
+    state = build_environment_state(writable: true)
+    state.formula = formula
+    state.cc = cc
+    state.build_bottle = build_bottle
+    state.bottle_arch = bottle_arch
+    state.debug_symbols = debug_symbols
+    state.testing_formula = testing_formula
     reset
   end
 
   sig { returns(T::Boolean) }
-  def build_bottle? = @build_bottle == true
+  def build_bottle? = build_environment_state.build_bottle == true
 
   sig { returns(T::Boolean) }
-  def debug_symbols? = @debug_symbols == true
+  def debug_symbols? = build_environment_state.debug_symbols == true
 
   sig { void }
   def reset
@@ -222,8 +226,11 @@ module SharedEnvExtension
   # end</pre>
   sig { returns(T.any(Symbol, String)) }
   def compiler
-    @compiler ||= T.let(nil, T.nilable(T.any(Symbol, String)))
-    @compiler ||= if (cc = @cc)
+    compiler = build_environment_state.compiler
+    return compiler if compiler
+
+    state = build_environment_state(writable: true)
+    state.compiler = if (cc = state.cc)
       warn_about_non_apple_gcc(cc) if cc.match?(GNU_GCC_REGEXP)
 
       fetch_compiler(cc, "--cc")
@@ -232,14 +239,14 @@ module SharedEnvExtension
 
       compiler = fetch_compiler(cc, "HOMEBREW_CC")
 
-      if @formula
+      if (formula = state.formula)
         compilers = [compiler] + CompilerSelector.compilers
-        compiler = CompilerSelector.select_for(@formula, compilers, testing_formula: @testing_formula == true)
+        compiler = CompilerSelector.select_for(formula, compilers, testing_formula: state.testing_formula)
       end
 
       compiler
-    elsif @formula
-      CompilerSelector.select_for(@formula, testing_formula: @testing_formula == true)
+    elsif (formula = state.formula)
+      CompilerSelector.select_for(formula, testing_formula: state.testing_formula)
     else
       DevelopmentTools.default_compiler
     end
@@ -258,7 +265,8 @@ module SharedEnvExtension
 
   COMPILERS.each do |compiler|
     define_method(compiler) do
-      @compiler = T.let(compiler, T.nilable(T.any(Symbol, String)))
+      T.bind(self, SharedEnvExtension)
+      build_environment_state(writable: true).compiler = compiler
 
       send(:cc=, send(:determine_cc))
       send(:cxx=, send(:determine_cxx))
@@ -270,9 +278,9 @@ module SharedEnvExtension
     # Ignore repeated calls to this function as it will misleadingly warn about
     # building with an alternative Fortran compiler without optimization flags,
     # despite it often being the Homebrew-provided one set up in the first call.
-    return if @fortran_setup_done
+    return if build_environment_state.fortran_setup_done
 
-    @fortran_setup_done = T.let(true, T.nilable(TrueClass))
+    build_environment_state(writable: true).fortran_setup_done = true
 
     flags = []
 
@@ -298,8 +306,8 @@ module SharedEnvExtension
 
   sig { returns(Symbol) }
   def effective_arch
-    if @build_bottle && @bottle_arch
-      @bottle_arch.to_sym
+    if build_bottle? && (arch = bottle_arch)
+      arch.to_sym
     else
       Hardware.oldest_cpu
     end
@@ -352,6 +360,17 @@ module SharedEnvExtension
 
   private
 
+  sig { params(writable: T::Boolean).returns(State) }
+  def build_environment_state(writable: false)
+    return State.process if equal?(ENV)
+
+    @build_environment_state ||= T.let(State.new, T.nilable(State))
+    # Hash#dup copies instance variables but drops singleton methods, so detach
+    # before writes to preserve the state of copies that have not been extended yet.
+    @build_environment_state = @build_environment_state.dup if writable
+    @build_environment_state
+  end
+
   sig { params(_flags: T::Array[String], _map: T::Hash[Symbol, String]).void }
   def set_cpu_flags(_flags, _map = {}); end
 

Library/Homebrew/extend/ENV/state.rb

@@ -0,0 +1,26 @@
+# typed: strict
+# frozen_string_literal: true
+
+require "formula"
+
+module SharedEnvExtension
+  # Build metadata cannot be stored on Ruby's shareable ENV object.
+  class State < T::Struct
+    prop :formula, T.nilable(Formula)
+    prop :cc, T.nilable(String)
+    prop :build_bottle, T.nilable(T::Boolean)
+    prop :bottle_arch, T.nilable(String)
+    prop :debug_symbols, T.nilable(T::Boolean)
+    prop :testing_formula, T::Boolean, default: false
+    prop :compiler, T.nilable(T.any(Symbol, String))
+    prop :fortran_setup_done, T::Boolean, default: false
+    prop :keg_only_deps, T::Array[Formula], default: []
+    prop :deps, T::Array[Formula], default: []
+    prop :run_time_deps, T::Array[Formula], default: []
+
+    sig { returns(State) }
+    def self.process
+      @process ||= T.let(new, T.nilable(State))
+    end
+  end
+end

Library/Homebrew/extend/ENV/super.rb

@@ -20,16 +20,34 @@ module Superenv
   include Utils::Output::Mixin
 
   sig { returns(T::Array[Formula]) }
-  attr_accessor :keg_only_deps
+  def keg_only_deps = build_environment_state.keg_only_deps
+
+  sig { params(deps: T::Array[Formula]).void }
+  def keg_only_deps=(deps)
+    build_environment_state(writable: true).keg_only_deps = deps
+  end
 
   sig { returns(T::Array[Formula]) }
-  attr_accessor :deps
+  def deps = build_environment_state.deps
+
+  sig { params(deps: T::Array[Formula]).void }
+  def deps=(deps)
+    build_environment_state(writable: true).deps = deps
+  end
 
   sig { returns(T::Array[Formula]) }
-  attr_accessor :run_time_deps
+  def run_time_deps = build_environment_state.run_time_deps
+
+  sig { params(deps: T::Array[Formula]).void }
+  def run_time_deps=(deps)
+    build_environment_state(writable: true).run_time_deps = deps
+  end
 
   sig { params(base: Superenv).void }
   def self.extended(base)
+    # Runtime signatures on the dependency setters resolve Formula on first call.
+    require "formula"
+
     base.keg_only_deps = []
     base.deps = []
     base.run_time_deps = []
@@ -53,15 +71,6 @@ module Superenv
     !bin.nil?
   end
 
-  sig { void }
-  def initialize
-    @keg_only_deps = T.let([], T::Array[Formula])
-    @deps = T.let([], T::Array[Formula])
-    @run_time_deps = T.let([], T::Array[Formula])
-
-    @formula = T.let(nil, T.nilable(Formula))
-  end
-
   sig { void }
   def reset
     super
@@ -111,7 +120,7 @@ module Superenv
     self["HOMEBREW_INCLUDE_PATHS"] = determine_include_paths&.to_s
     self["HOMEBREW_LIBRARY_PATHS"] = determine_library_paths&.to_s
     self["HOMEBREW_DEPENDENCIES"] = determine_dependencies
-    self["HOMEBREW_FORMULA_PREFIX"] = @formula.prefix.to_s unless @formula.nil?
+    self["HOMEBREW_FORMULA_PREFIX"] = formula.prefix.to_s if formula
     # Prevent the OpenSSL rust crate from building a vendored OpenSSL.
     # https://github.com/sfackler/rust-openssl/blob/994e5ff8c63557ab2aa85c85cc6956b0b0216ca7/openssl/src/lib.rs#L65
     self["OPENSSL_NO_VENDOR"] = "1"

Library/Homebrew/test/ENV_spec.rb

@@ -6,9 +6,46 @@ require "extend/ENV"
 RSpec.describe "ENV" do
   subject(:env) { {}.extend(EnvActivation).extend(described_class) }
 
+  it "configures the process environment", :integration_test do
+    expect do
+      brew "ruby", "-e", <<~RUBY
+        require "extend/ENV"
+        ENV.activate_extensions!
+        ENV.setup_build_environment(build_bottle: true, bottle_arch: Hardware.oldest_cpu.to_s, debug_symbols: true)
+        puts ENV.build_bottle?, ENV.bottle_arch, ENV.debug_symbols?
+      RUBY
+    end.to output("true\n#{Hardware.oldest_cpu}\ntrue\n").to_stdout.and not_to_output.to_stderr.and be_a_success
+  end
+
+  it "loads build state without preloading formula" do
+    ENV["HOMEBREW_SORBET_RUNTIME"] = "1"
+
+    expect(Utils.popen_read(*HOMEBREW_RUBY_EXEC_ARGS,
+                            "-I", $LOAD_PATH.join(File::PATH_SEPARATOR), "-rglobal",
+                            "-e", 'require "extend/ENV/super"; {}.extend(Superenv); puts "loaded"', err: :out))
+      .to eq("loaded\n")
+  end
+
   shared_examples EnvActivation do
     include Context
 
+    it "keeps compiler choices separate for copied environments" do
+      subject.clang
+      copy = subject.dup.extend(EnvActivation).extend(described_class)
+      copy.public_send(:"gcc-9")
+
+      expect([subject.compiler, copy.compiler]).to eq([:clang, "gcc-9"])
+    end
+
+    it "preserves the compiler when a copied environment is activated later" do
+      subject.clang
+      copy = subject.dup
+      subject.public_send(:"gcc-9")
+      copy.extend(EnvActivation).extend(described_class)
+
+      expect([subject.compiler, copy.compiler]).to eq(["gcc-9", :clang])
+    end
+
     it "supports switching compilers" do
       subject.clang
       expect(subject["LD"]).to be_nil