language/python: order args for virtualenv_install_with_resources

Add `without`, `start_with`, and `end_with` to allow basic control over
the order that resources are installed so that the situations where we
have to split up `virtualenv_install_with_resources` is reduced.

Co-authored-by: Kevin <apainintheneck@gmail.com>
Co-authored-by: Mike McQuaid <mike@mikemcquaid.com>
Signed-off-by: Michael Cho <michael@michaelcho.dev>

Library/Homebrew/language/python.rb

@@ -224,10 +224,13 @@ module Language
           system_site_packages: T::Boolean,
           without_pip:          T::Boolean,
           link_manpages:        T::Boolean,
+          without:              T.nilable(T.any(String, T::Array[String])),
+          start_with:           T.nilable(T.any(String, T::Array[String])),
+          end_with:             T.nilable(T.any(String, T::Array[String])),
         ).returns(Virtualenv)
       }
       def virtualenv_install_with_resources(using: nil, system_site_packages: true, without_pip: true,
-                                            link_manpages: false)
+                                            link_manpages: false, without: nil, start_with: nil, end_with: nil)
         python = using
         if python.nil?
           wanted = python_names.select { |py| needs_python?(py) }
@@ -237,9 +240,22 @@ module Language
           python = T.must(wanted.first)
           python = "python3" if python == "python"
         end
+
+        venv_resources = if without.nil? && start_with.nil? && end_with.nil?
+          resources
+        else
+          remaining_resources = resources.to_h { |resource| [resource.name, resource] }
+
+          slice_resources!(remaining_resources, Array(without))
+          start_with_resources = slice_resources!(remaining_resources, Array(start_with))
+          end_with_resources = slice_resources!(remaining_resources, Array(end_with))
+
+          start_with_resources + remaining_resources.values + end_with_resources
+        end
+
         venv = virtualenv_create(libexec, python.delete("@"), system_site_packages:,
                                                               without_pip:)
-        venv.pip_install resources
+        venv.pip_install venv_resources
         venv.pip_install_and_link(T.must(buildpath), link_manpages:)
         venv
       end
@@ -249,6 +265,22 @@ module Language
         %w[python python3 pypy pypy3] + Formula.names.select { |name| name.start_with? "python@" }
       end
 
+      private
+
+      sig {
+        params(
+          resources_hash: T::Hash[String, Resource],
+          resource_names: T::Array[String],
+        ).returns(T::Array[Resource])
+      }
+      def slice_resources!(resources_hash, resource_names)
+        resource_names.map do |resource_name|
+          resources_hash.delete(resource_name) do
+            raise ArgumentError, "Resource \"#{resource_name}\" is not defined in formula or is already used"
+          end
+        end
+      end
+
       # Convenience wrapper for creating and installing packages into Python
       # virtualenvs.
       class Virtualenv

Library/Homebrew/test/language/python/virtualenv_spec.rb

@@ -3,151 +3,299 @@
 require "language/python"
 require "resource"
 
-RSpec.describe Language::Python::Virtualenv::Virtualenv, :needs_python do
+RSpec.describe Language::Python::Virtualenv, :needs_python do
-  subject(:virtualenv) { described_class.new(formula, dir, "python") }
+  describe "#virtualenv_install_with_resources" do
+    let(:venv) { instance_double(Language::Python::Virtualenv::Virtualenv) }
+    let(:f) do
+      formula "foo" do
+        # Couldn't find a way to get described_class to work inside formula do
+        # rubocop:disable RSpec/DescribedClass
+        include Language::Python::Virtualenv
+        # rubocop:enable RSpec/DescribedClass
 
-  let(:dir) { mktmpdir }
+        url "https://brew.sh/foo-1.0.tgz"
 
-  let(:resource) { instance_double(Resource, "resource", stage: true) }
+        resource "resource-a" do
-  let(:formula_bin) { dir/"formula_bin" }
+          url "https://brew.sh/resource1.tar.gz"
-  let(:formula_man) { dir/"formula_man" }
+          sha256 "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
-  let(:formula) { instance_double(Formula, "formula", resource:, bin: formula_bin, man: formula_man) }
+        end
 
-  describe "#create" do
+        resource "resource-b" do
-    it "creates a venv" do
+          url "https://brew.sh/resource2.tar.gz"
-      expect(formula).to receive(:system).with("python", "-m", "venv", "--system-site-packages", "--without-pip", dir)
+          sha256 "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
-      virtualenv.create
+        end
+
+        resource "resource-c" do
+          url "https://brew.sh/resource3.tar.gz"
+          sha256 "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"
+        end
+
+        resource "resource-d" do
+          url "https://brew.sh/resource4.tar.gz"
+          sha256 "dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"
+        end
+      end
+    end
+    let(:r_a) { f.resource("resource-a") }
+    let(:r_b) { f.resource("resource-b") }
+    let(:r_c) { f.resource("resource-c") }
+    let(:r_d) { f.resource("resource-d") }
+    let(:buildpath) { Pathname(TEST_TMPDIR) }
+
+    before { f.instance_variable_set(:@buildpath, buildpath) }
+
+    it "works with `using: \"python\"` and installs resources in order" do
+      expect(f).to receive(:virtualenv_create).with(
+        f.libexec, "python", { system_site_packages: true, without_pip: true }
+      ).and_return(venv)
+      expect(venv).to receive(:pip_install).with([r_a, r_b, r_c, r_d])
+      expect(venv).to receive(:pip_install_and_link).with(buildpath, { link_manpages: false })
+      f.virtualenv_install_with_resources(using: "python")
     end
 
-    it "creates a venv with pip" do
+    it "works with `using: \"python@3.12\"` and installs resources in order" do
-      expect(formula).to receive(:system).with("python", "-m", "venv", "--system-site-packages", dir)
+      expect(f).to receive(:virtualenv_create).with(
-      virtualenv.create(without_pip: false)
+        f.libexec, "python3.12", { system_site_packages: true, without_pip: true }
+      ).and_return(venv)
+      expect(venv).to receive(:pip_install).with([r_a, r_b, r_c, r_d])
+      expect(venv).to receive(:pip_install_and_link).with(buildpath, { link_manpages: false })
+      f.virtualenv_install_with_resources(using: "python@3.12")
+    end
+
+    it "skips a `without` resource string and installs remaining resources in order" do
+      expect(f).to receive(:virtualenv_create).and_return(venv)
+      expect(venv).to receive(:pip_install).with([r_a, r_b, r_d])
+      expect(venv).to receive(:pip_install_and_link).with(buildpath, { link_manpages: false })
+      f.virtualenv_install_with_resources(using: "python", without: r_c.name)
+    end
+
+    it "skips all resources in `without` array and installs remaining resources in order" do
+      expect(f).to receive(:virtualenv_create).and_return(venv)
+      expect(venv).to receive(:pip_install).with([r_b, r_c])
+      expect(venv).to receive(:pip_install_and_link).with(buildpath, { link_manpages: false })
+      f.virtualenv_install_with_resources(using: "python", without: [r_d.name, r_a.name])
+    end
+
+    it "errors if `without` resource string does not exist in formula" do
+      expect do
+        f.virtualenv_install_with_resources(using: "python", without: "unknown")
+      end.to raise_error(ArgumentError)
+    end
+
+    it "errors if `without` resource array refers to a resource that does not exist in formula" do
+      expect do
+        f.virtualenv_install_with_resources(using: "python", without: [r_a.name, "unknown"])
+      end.to raise_error(ArgumentError)
+    end
+
+    it "installs a `start_with` resource string and then remaining resources in order" do
+      expect(f).to receive(:virtualenv_create).and_return(venv)
+      expect(venv).to receive(:pip_install).with([r_c, r_a, r_b, r_d])
+      expect(venv).to receive(:pip_install_and_link).with(buildpath, { link_manpages: false })
+      f.virtualenv_install_with_resources(using: "python", start_with: r_c.name)
+    end
+
+    it "installs all resources in `start_with` array and then remaining resources in order" do
+      expect(f).to receive(:virtualenv_create).and_return(venv)
+      expect(venv).to receive(:pip_install).with([r_d, r_b, r_a, r_c])
+      expect(venv).to receive(:pip_install_and_link).with(buildpath, { link_manpages: false })
+      f.virtualenv_install_with_resources(using: "python", start_with: [r_d.name, r_b.name])
+    end
+
+    it "errors if `start_with` resource string does not exist in formula" do
+      expect do
+        f.virtualenv_install_with_resources(using: "python", start_with: "unknown")
+      end.to raise_error(ArgumentError)
+    end
+
+    it "errors if `start_with` resource array refers to a resource that does not exist in formula" do
+      expect do
+        f.virtualenv_install_with_resources(using: "python", start_with: [r_a.name, "unknown"])
+      end.to raise_error(ArgumentError)
+    end
+
+    it "installs an `end_with` resource string as last resource" do
+      expect(f).to receive(:virtualenv_create).and_return(venv)
+      expect(venv).to receive(:pip_install).with([r_a, r_c, r_d, r_b])
+      expect(venv).to receive(:pip_install_and_link).with(buildpath, { link_manpages: false })
+      f.virtualenv_install_with_resources(using: "python", end_with: r_b.name)
+    end
+
+    it "installs all resources in `end_with` array after other resources are installed" do
+      expect(f).to receive(:virtualenv_create).and_return(venv)
+      expect(venv).to receive(:pip_install).with([r_a, r_d, r_c, r_b])
+      expect(venv).to receive(:pip_install_and_link).with(buildpath, { link_manpages: false })
+      f.virtualenv_install_with_resources(using: "python", end_with: [r_c.name, r_b.name])
+    end
+
+    it "errors if `end_with` resource string does not exist in formula" do
+      expect do
+        f.virtualenv_install_with_resources(using: "python", end_with: "unknown")
+      end.to raise_error(ArgumentError)
+    end
+
+    it "errors if `end_with` resource array refers to a resource that does not exist in formula" do
+      expect do
+        f.virtualenv_install_with_resources(using: "python", end_with: [r_a.name, "unknown"])
+      end.to raise_error(ArgumentError)
+    end
+
+    it "installs resources in correct order when combining `without`, `start_with`, and `end_with" do
+      expect(f).to receive(:virtualenv_create).and_return(venv)
+      expect(venv).to receive(:pip_install).with([r_d, r_c, r_b])
+      expect(venv).to receive(:pip_install_and_link).with(buildpath, { link_manpages: false })
+      f.virtualenv_install_with_resources(using: "python", without: r_a.name,
+                                          start_with: r_d.name, end_with: r_b.name)
     end
   end
 
-  describe "#pip_install" do
+  describe Language::Python::Virtualenv::Virtualenv do
-    it "accepts a string" do
+    subject(:virtualenv) { described_class.new(formula, dir, "python") }
-      expect(formula).to receive(:std_pip_args).with(prefix:          false,
+
-                                                     build_isolation: true).and_return(["--std-pip-args"])
+    let(:dir) { mktmpdir }
-      expect(formula).to receive(:system)
+
-        .with("python", "-m", "pip", "--python=#{dir}/bin/python", "install", "--std-pip-args", "foo")
+    let(:resource) { instance_double(Resource, "resource", stage: true) }
-        .and_return(true)
+    let(:formula_bin) { dir/"formula_bin" }
-      virtualenv.pip_install "foo"
+    let(:formula_man) { dir/"formula_man" }
+    let(:formula) { instance_double(Formula, "formula", resource:, bin: formula_bin, man: formula_man) }
+
+    describe "#create" do
+      it "creates a venv" do
+        expect(formula).to receive(:system)
+          .with("python", "-m", "venv", "--system-site-packages", "--without-pip", dir)
+        virtualenv.create
+      end
+
+      it "creates a venv with pip" do
+        expect(formula).to receive(:system).with("python", "-m", "venv", "--system-site-packages", dir)
+        virtualenv.create(without_pip: false)
+      end
     end
 
-    it "accepts a multi-line strings" do
+    describe "#pip_install" do
-      expect(formula).to receive(:std_pip_args).with(prefix:          false,
+      it "accepts a string" do
-                                                     build_isolation: true).and_return(["--std-pip-args"])
+        expect(formula).to receive(:std_pip_args).with(prefix:          false,
-      expect(formula).to receive(:system)
+                                                       build_isolation: true).and_return(["--std-pip-args"])
-        .with("python", "-m", "pip", "--python=#{dir}/bin/python", "install", "--std-pip-args", "foo", "bar")
+        expect(formula).to receive(:system)
-        .and_return(true)
+          .with("python", "-m", "pip", "--python=#{dir}/bin/python", "install", "--std-pip-args", "foo")
+          .and_return(true)
+        virtualenv.pip_install "foo"
+      end
 
-      virtualenv.pip_install <<~EOS
+      it "accepts a multi-line strings" do
-        foo
+        expect(formula).to receive(:std_pip_args).with(prefix:          false,
-        bar
+                                                       build_isolation: true).and_return(["--std-pip-args"])
-      EOS
+        expect(formula).to receive(:system)
+          .with("python", "-m", "pip", "--python=#{dir}/bin/python", "install", "--std-pip-args", "foo", "bar")
+          .and_return(true)
+
+        virtualenv.pip_install <<~EOS
+          foo
+          bar
+        EOS
+      end
+
+      it "accepts an array" do
+        expect(formula).to receive(:std_pip_args).with(prefix:          false,
+                                                       build_isolation: true).and_return(["--std-pip-args"])
+        expect(formula).to receive(:system)
+          .with("python", "-m", "pip", "--python=#{dir}/bin/python", "install", "--std-pip-args", "foo")
+          .and_return(true)
+
+        expect(formula).to receive(:std_pip_args).with(prefix:          false,
+                                                       build_isolation: true).and_return(["--std-pip-args"])
+        expect(formula).to receive(:system)
+          .with("python", "-m", "pip", "--python=#{dir}/bin/python", "install", "--std-pip-args", "bar")
+          .and_return(true)
+
+        virtualenv.pip_install ["foo", "bar"]
+      end
+
+      it "accepts a Resource" do
+        res = Resource.new("test")
+
+        expect(res).to receive(:stage).and_yield
+        expect(formula).to receive(:std_pip_args).with(prefix:          false,
+                                                       build_isolation: true).and_return(["--std-pip-args"])
+        expect(formula).to receive(:system)
+          .with("python", "-m", "pip", "--python=#{dir}/bin/python", "install", "--std-pip-args", Pathname.pwd)
+          .and_return(true)
+
+        virtualenv.pip_install res
+      end
+
+      it "works without build isolation" do
+        expect(formula).to receive(:std_pip_args).with(prefix:          false,
+                                                       build_isolation: false).and_return(["--std-pip-args"])
+        expect(formula).to receive(:system)
+          .with("python", "-m", "pip", "--python=#{dir}/bin/python", "install", "--std-pip-args", "foo")
+          .and_return(true)
+        virtualenv.pip_install("foo", build_isolation: false)
+      end
     end
 
-    it "accepts an array" do
+    describe "#pip_install_and_link" do
-      expect(formula).to receive(:std_pip_args).with(prefix:          false,
+      let(:src_bin) { dir/"bin" }
-                                                     build_isolation: true).and_return(["--std-pip-args"])
+      let(:src_man) { dir/"share/man" }
-      expect(formula).to receive(:system)
+      let(:dest_bin) { formula.bin }
-        .with("python", "-m", "pip", "--python=#{dir}/bin/python", "install", "--std-pip-args", "foo")
+      let(:dest_man) { formula.man }
-        .and_return(true)
 
-      expect(formula).to receive(:std_pip_args).with(prefix:          false,
+      it "can link scripts" do
-                                                     build_isolation: true).and_return(["--std-pip-args"])
+        src_bin.mkpath
-      expect(formula).to receive(:system)
-        .with("python", "-m", "pip", "--python=#{dir}/bin/python", "install", "--std-pip-args", "bar")
-        .and_return(true)
 
-      virtualenv.pip_install ["foo", "bar"]
+        expect(src_bin/"kilroy").not_to exist
-    end
+        expect(dest_bin/"kilroy").not_to exist
 
-    it "accepts a Resource" do
+        FileUtils.touch src_bin/"irrelevant"
-      res = Resource.new("test")
+        bin_before = Dir.glob(src_bin/"*")
+        FileUtils.touch src_bin/"kilroy"
+        bin_after = Dir.glob(src_bin/"*")
 
-      expect(res).to receive(:stage).and_yield
+        expect(virtualenv).to receive(:pip_install).with("foo", { build_isolation: true })
-      expect(formula).to receive(:std_pip_args).with(prefix:          false,
+        expect(Dir).to receive(:[]).with(src_bin/"*").twice.and_return(bin_before, bin_after)
-                                                     build_isolation: true).and_return(["--std-pip-args"])
-      expect(formula).to receive(:system)
-        .with("python", "-m", "pip", "--python=#{dir}/bin/python", "install", "--std-pip-args", Pathname.pwd)
-        .and_return(true)
 
-      virtualenv.pip_install res
+        virtualenv.pip_install_and_link "foo"
-    end
 
-    it "works without build isolation" do
+        expect(src_bin/"kilroy").to exist
-      expect(formula).to receive(:std_pip_args).with(prefix:          false,
+        expect(dest_bin/"kilroy").to exist
-                                                     build_isolation: false).and_return(["--std-pip-args"])
+        expect(dest_bin/"kilroy").to be_a_symlink
-      expect(formula).to receive(:system)
+        expect((src_bin/"kilroy").realpath).to eq((dest_bin/"kilroy").realpath)
-        .with("python", "-m", "pip", "--python=#{dir}/bin/python", "install", "--std-pip-args", "foo")
+        expect(dest_bin/"irrelevant").not_to exist
-        .and_return(true)
+      end
-      virtualenv.pip_install("foo", build_isolation: false)
-    end
-  end
 
-  describe "#pip_install_and_link" do
+      it "can link manpages" do
-    let(:src_bin) { dir/"bin" }
+        (src_man/"man1").mkpath
-    let(:src_man) { dir/"share/man" }
+        (src_man/"man3").mkpath
-    let(:dest_bin) { formula.bin }
-    let(:dest_man) { formula.man }
 
-    it "can link scripts" do
+        expect(src_man/"man1/kilroy.1").not_to exist
-      src_bin.mkpath
+        expect(dest_man/"man1").not_to exist
+        expect(dest_man/"man3").not_to exist
+        expect(dest_man/"man5").not_to exist
 
-      expect(src_bin/"kilroy").not_to exist
+        FileUtils.touch src_man/"man1/irrelevant.1"
-      expect(dest_bin/"kilroy").not_to exist
+        FileUtils.touch src_man/"man3/irrelevant.3"
+        man_before = Dir.glob(src_man/"**/*")
+        (src_man/"man5").mkpath
+        FileUtils.touch src_man/"man1/kilroy.1"
+        FileUtils.touch src_man/"man5/kilroy.5"
+        man_after = Dir.glob(src_man/"**/*")
 
-      FileUtils.touch src_bin/"irrelevant"
+        expect(virtualenv).to receive(:pip_install).with("foo", { build_isolation: true })
-      bin_before = Dir.glob(src_bin/"*")
+        expect(Dir).to receive(:[]).with(src_bin/"*").and_return([])
-      FileUtils.touch src_bin/"kilroy"
+        expect(Dir).to receive(:[]).with(src_man/"man*/*").and_return(man_before)
-      bin_after = Dir.glob(src_bin/"*")
+        expect(Dir).to receive(:[]).with(src_bin/"*").and_return([])
+        expect(Dir).to receive(:[]).with(src_man/"man*/*").and_return(man_after)
 
-      expect(virtualenv).to receive(:pip_install).with("foo", { build_isolation: true })
+        virtualenv.pip_install_and_link("foo", link_manpages: true)
-      expect(Dir).to receive(:[]).with(src_bin/"*").twice.and_return(bin_before, bin_after)
 
-      virtualenv.pip_install_and_link "foo"
+        expect(src_man/"man1/kilroy.1").to exist
-
+        expect(dest_man/"man1/kilroy.1").to exist
-      expect(src_bin/"kilroy").to exist
+        expect(dest_man/"man5/kilroy.5").to exist
-      expect(dest_bin/"kilroy").to exist
+        expect(dest_man/"man1/kilroy.1").to be_a_symlink
-      expect(dest_bin/"kilroy").to be_a_symlink
+        expect((src_man/"man1/kilroy.1").realpath).to eq((dest_man/"man1/kilroy.1").realpath)
-      expect((src_bin/"kilroy").realpath).to eq((dest_bin/"kilroy").realpath)
+        expect(dest_man/"man1/irrelevant.1").not_to exist
-      expect(dest_bin/"irrelevant").not_to exist
+        expect(dest_man/"man3").not_to exist
-    end
+      end
-
-    it "can link manpages" do
-      (src_man/"man1").mkpath
-      (src_man/"man3").mkpath
-
-      expect(src_man/"man1/kilroy.1").not_to exist
-      expect(dest_man/"man1").not_to exist
-      expect(dest_man/"man3").not_to exist
-      expect(dest_man/"man5").not_to exist
-
-      FileUtils.touch src_man/"man1/irrelevant.1"
-      FileUtils.touch src_man/"man3/irrelevant.3"
-      man_before = Dir.glob(src_man/"**/*")
-      (src_man/"man5").mkpath
-      FileUtils.touch src_man/"man1/kilroy.1"
-      FileUtils.touch src_man/"man5/kilroy.5"
-      man_after = Dir.glob(src_man/"**/*")
-
-      expect(virtualenv).to receive(:pip_install).with("foo", { build_isolation: true })
-      expect(Dir).to receive(:[]).with(src_bin/"*").and_return([])
-      expect(Dir).to receive(:[]).with(src_man/"man*/*").and_return(man_before)
-      expect(Dir).to receive(:[]).with(src_bin/"*").and_return([])
-      expect(Dir).to receive(:[]).with(src_man/"man*/*").and_return(man_after)
-
-      virtualenv.pip_install_and_link("foo", link_manpages: true)
-
-      expect(src_man/"man1/kilroy.1").to exist
-      expect(dest_man/"man1/kilroy.1").to exist
-      expect(dest_man/"man5/kilroy.5").to exist
-      expect(dest_man/"man1/kilroy.1").to be_a_symlink
-      expect((src_man/"man1/kilroy.1").realpath).to eq((dest_man/"man1/kilroy.1").realpath)
-      expect(dest_man/"man1/irrelevant.1").not_to exist
-      expect(dest_man/"man3").not_to exist
     end
   end
 end