From e71e3e8dd1dad8b4491de0444bbd6a91ae06b1d0 Mon Sep 17 00:00:00 2001
From: Charlie Marsh <charlie.r.marsh@gmail.com>
Date: Mon, 15 Jan 2024 13:56:18 -0500
Subject: [PATCH] Refresh `BuildDispatch` when running pip install with
 `--reinstall` (#933)

## Summary

This fixes an extremely subtle bug in `pip install --reinstall`, whereby
if you depend on `setuptools` at the top level, we end up uninstalling
it after resolving, which breaks some cached state. If we have
`--reinstall`, we need to reset that cached state between resolving and
installing.

## Test Plan

Running `pip install --reinstall` with:

```txt
setuptools
devpi @ https://files.pythonhosted.org/packages/e4/f3/e334eb4dc930ccb677363e1305a3730003d203efbb023329e4b610e4515b/devpi-2.2.0.tar.gz
```

Fails on `main`, but passes.
---
 crates/puffin-cli/src/commands/pip_install.rs | 29 ++++++++--
 crates/puffin-cli/tests/pip_install.rs        | 56 +++++++++++++++++++
 .../black_editable/pyproject.toml             |  1 -
 3 files changed, 81 insertions(+), 5 deletions(-)

diff --git a/crates/puffin-cli/src/commands/pip_install.rs b/crates/puffin-cli/src/commands/pip_install.rs
index 27a995182..2757eade2 100644
--- a/crates/puffin-cli/src/commands/pip_install.rs
+++ b/crates/puffin-cli/src/commands/pip_install.rs
@@ -149,7 +149,7 @@ pub(crate) async fn pip_install(
 
     let options = ResolutionOptions::new(resolution_mode, prerelease_mode, exclude_newer);
 
-    let build_dispatch = BuildDispatch::new(
+    let resolve_dispatch = BuildDispatch::new(
         &client,
         &cache,
         &interpreter,
@@ -176,7 +176,7 @@ pub(crate) async fn pip_install(
             &cache,
             tags,
             &client,
-            &build_dispatch,
+            &resolve_dispatch,
             printer,
         )
         .await?
@@ -196,7 +196,7 @@ pub(crate) async fn pip_install(
         markers,
         &client,
         &flat_index,
-        &build_dispatch,
+        &resolve_dispatch,
         options,
         printer,
     )
@@ -215,6 +215,27 @@ pub(crate) async fn pip_install(
         Err(err) => return Err(err.into()),
     };
 
+    // Re-initialize the in-flight map.
+    let in_flight = InFlight::default();
+
+    // If we're running with `--reinstall`, initialize a separate `BuildDispatch`, since we may
+    // end up removing some distributions from the environment.
+    let install_dispatch = if reinstall.is_none() {
+        resolve_dispatch
+    } else {
+        BuildDispatch::new(
+            &client,
+            &cache,
+            &interpreter,
+            &index_locations,
+            &flat_index,
+            &in_flight,
+            venv.python_executable(),
+            setup_py,
+            no_build,
+        )
+    };
+
     // Sync the environment.
     install(
         &resolution,
@@ -226,7 +247,7 @@ pub(crate) async fn pip_install(
         tags,
         &client,
         &in_flight,
-        &build_dispatch,
+        &install_dispatch,
         &cache,
         &venv,
         printer,
diff --git a/crates/puffin-cli/tests/pip_install.rs b/crates/puffin-cli/tests/pip_install.rs
index 90655d674..1ef1dac96 100644
--- a/crates/puffin-cli/tests/pip_install.rs
+++ b/crates/puffin-cli/tests/pip_install.rs
@@ -8,6 +8,7 @@ use anyhow::Result;
 use assert_cmd::assert::Assert;
 use assert_cmd::prelude::*;
 use assert_fs::prelude::*;
+use indoc::indoc;
 use insta_cmd::_macro_support::insta;
 use insta_cmd::{assert_cmd_snapshot, get_cargo_bin};
 
@@ -687,3 +688,58 @@ fn install_editable_and_registry() -> Result<()> {
 
     Ok(())
 }
+
+/// Install a source distribution that uses the `flit` build system, along with `flit`
+/// at the top-level, along with `--reinstall` to force a re-download after resolution, to ensure
+/// that the `flit` install and the source distribution build don't conflict.
+#[test]
+fn reinstall_build_system() -> Result<()> {
+    let temp_dir = assert_fs::TempDir::new()?;
+    let cache_dir = assert_fs::TempDir::new()?;
+    let venv = create_venv_py312(&temp_dir, &cache_dir);
+
+    // Install devpi.
+    let requirements_txt = temp_dir.child("requirements.txt");
+    requirements_txt.write_str(indoc! {r"
+        flit_core<4.0.0
+        flask @ https://files.pythonhosted.org/packages/d8/09/c1a7354d3925a3c6c8cfdebf4245bae67d633ffda1ba415add06ffc839c5/flask-3.0.0.tar.gz
+        "
+    })?;
+
+    insta::with_settings!({
+        filters => INSTA_FILTERS.to_vec()
+    }, {
+        assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
+            .arg("pip")
+            .arg("install")
+            .arg("--reinstall")
+            .arg("-r")
+            .arg("requirements.txt")
+            .arg("--strict")
+            .arg("--cache-dir")
+            .arg(cache_dir.path())
+            .arg("--exclude-newer")
+            .arg(EXCLUDE_NEWER)
+            .env("VIRTUAL_ENV", venv.as_os_str())
+            .current_dir(&temp_dir), @r###"
+        success: true
+        exit_code: 0
+        ----- stdout -----
+
+        ----- stderr -----
+        Resolved 8 packages in [TIME]
+        Downloaded 8 packages in [TIME]
+        Installed 8 packages in [TIME]
+         + blinker==1.7.0
+         + click==8.1.7
+         + flask==3.0.0 (from https://files.pythonhosted.org/packages/d8/09/c1a7354d3925a3c6c8cfdebf4245bae67d633ffda1ba415add06ffc839c5/flask-3.0.0.tar.gz)
+         + flit-core==3.9.0
+         + itsdangerous==2.1.2
+         + jinja2==3.1.2
+         + markupsafe==2.1.3
+         + werkzeug==3.0.1
+        "###);
+    });
+
+    Ok(())
+}
diff --git a/scripts/editable-installs/black_editable/pyproject.toml b/scripts/editable-installs/black_editable/pyproject.toml
index ca17596ae..9a0f6a2c8 100644
--- a/scripts/editable-installs/black_editable/pyproject.toml
+++ b/scripts/editable-installs/black_editable/pyproject.toml
@@ -13,6 +13,5 @@ license = {text = "MIT"}
 requires = ["pdm-backend"]
 build-backend = "pdm.backend"
 
-
 [tool.pdm]
 package-type = "library"