python: Install basedpyright if the basedpyright-langserver binary is missing (#38426)

Cole Miller and Peter Tripp created 1 month ago

Potential fix for #38377 

Release Notes:

- N/A

---------

Co-authored-by: Peter Tripp <[e-mail]>

Change summary

crates/language/src/language.rs |  5 --
crates/languages/src/python.rs  | 48 +++++++++++++++++++++++-----------
2 files changed, 33 insertions(+), 20 deletions(-)

Detailed changes

crates/language/src/language.rs

@@ -55,7 +55,7 @@ use std::{
     str,
     sync::{
         Arc, LazyLock,
-        atomic::{AtomicU64, AtomicUsize, Ordering::SeqCst},
+        atomic::{AtomicUsize, Ordering::SeqCst},
     },
 };
 use syntax_map::{QueryCursorHandle, SyntaxSnapshot};
@@ -168,7 +168,6 @@ pub struct CachedLspAdapter {
     pub disk_based_diagnostics_progress_token: Option<String>,
     language_ids: HashMap<LanguageName, String>,
     pub adapter: Arc<dyn LspAdapter>,
-    pub reinstall_attempt_count: AtomicU64,
     cached_binary: ServerBinaryCache,
 }
 
@@ -185,7 +184,6 @@ impl Debug for CachedLspAdapter {
                 &self.disk_based_diagnostics_progress_token,
             )
             .field("language_ids", &self.language_ids)
-            .field("reinstall_attempt_count", &self.reinstall_attempt_count)
             .finish_non_exhaustive()
     }
 }
@@ -204,7 +202,6 @@ impl CachedLspAdapter {
             language_ids,
             adapter,
             cached_binary: Default::default(),
-            reinstall_attempt_count: AtomicU64::new(0),
         })
     }
 

crates/languages/src/python.rs

@@ -1559,7 +1559,7 @@ impl LspInstaller for PyLspAdapter {
             util::command::new_smol_command(pip_path.as_path())
                 .arg("install")
                 .arg("python-lsp-server")
-                .arg("-U")
+                .arg("--upgrade")
                 .output()
                 .await?
                 .status
@@ -1570,7 +1570,7 @@ impl LspInstaller for PyLspAdapter {
             util::command::new_smol_command(pip_path.as_path())
                 .arg("install")
                 .arg("python-lsp-server[all]")
-                .arg("-U")
+                .arg("--upgrade")
                 .output()
                 .await?
                 .status
@@ -1581,7 +1581,7 @@ impl LspInstaller for PyLspAdapter {
             util::command::new_smol_command(pip_path)
                 .arg("install")
                 .arg("pylsp-mypy")
-                .arg("-U")
+                .arg("--upgrade")
                 .output()
                 .await?
                 .status
@@ -1589,6 +1589,10 @@ impl LspInstaller for PyLspAdapter {
             "pylsp-mypy installation failed"
         );
         let pylsp = venv.join(BINARY_DIR).join("pylsp");
+        ensure!(
+            delegate.which(pylsp.as_os_str()).await.is_some(),
+            "pylsp installation was incomplete"
+        );
         Ok(LanguageServerBinary {
             path: pylsp,
             env: None,
@@ -1603,6 +1607,7 @@ impl LspInstaller for PyLspAdapter {
     ) -> Option<LanguageServerBinary> {
         let venv = self.base_venv(delegate).await.ok()?;
         let pylsp = venv.join(BINARY_DIR).join("pylsp");
+        delegate.which(pylsp.as_os_str()).await?;
         Some(LanguageServerBinary {
             path: pylsp,
             env: None,
@@ -1641,9 +1646,11 @@ impl BasedPyrightLspAdapter {
                 .arg("venv")
                 .arg("basedpyright-venv")
                 .current_dir(work_dir)
-                .spawn()?
+                .spawn()
+                .context("spawning child")?
                 .output()
-                .await?;
+                .await
+                .context("getting child output")?;
         }
 
         Ok(path.into())
@@ -1885,11 +1892,14 @@ impl LspInstaller for BasedPyrightLspAdapter {
             let path = Path::new(toolchain?.path.as_ref())
                 .parent()?
                 .join(Self::BINARY_NAME);
-            path.exists().then(|| LanguageServerBinary {
-                path,
-                arguments: vec!["--stdio".into()],
-                env: None,
-            })
+            delegate
+                .which(path.as_os_str())
+                .await
+                .map(|_| LanguageServerBinary {
+                    path,
+                    arguments: vec!["--stdio".into()],
+                    env: None,
+                })
         }
     }
 
@@ -1905,16 +1915,21 @@ impl LspInstaller for BasedPyrightLspAdapter {
             util::command::new_smol_command(pip_path.as_path())
                 .arg("install")
                 .arg("basedpyright")
-                .arg("-U")
+                .arg("--upgrade")
                 .output()
-                .await?
+                .await
+                .context("getting pip install output")?
                 .status
                 .success(),
             "basedpyright installation failed"
         );
-        let pylsp = venv.join(BINARY_DIR).join(Self::BINARY_NAME);
+        let path = venv.join(BINARY_DIR).join(Self::BINARY_NAME);
+        ensure!(
+            delegate.which(path.as_os_str()).await.is_some(),
+            "basedpyright installation was incomplete"
+        );
         Ok(LanguageServerBinary {
-            path: pylsp,
+            path,
             env: None,
             arguments: vec!["--stdio".into()],
         })
@@ -1926,9 +1941,10 @@ impl LspInstaller for BasedPyrightLspAdapter {
         delegate: &dyn LspAdapterDelegate,
     ) -> Option<LanguageServerBinary> {
         let venv = self.base_venv(delegate).await.ok()?;
-        let pylsp = venv.join(BINARY_DIR).join(Self::BINARY_NAME);
+        let path = venv.join(BINARY_DIR).join(Self::BINARY_NAME);
+        delegate.which(path.as_os_str()).await?;
         Some(LanguageServerBinary {
-            path: pylsp,
+            path,
             env: None,
             arguments: vec!["--stdio".into()],
         })