ssh remoting: Check nightly version correctly by comparing commit SHA (#19884)

Bennet Bo Fenner and Thorsten created

This ensures that we detect if a new nightly version of the remote
server is available.
Previously we would always mark a version as matching if they had the
same semantic version.
However, for nightly versions we also need to check if they have the
same commit SHA.

Co-Authored-by: Thorsten <thorsten@zed.dev>

Release Notes:

- N/A

---------

Co-authored-by: Thorsten <thorsten@zed.dev>

Change summary

crates/auto_update/src/auto_update.rs         | 10 +-
crates/recent_projects/src/ssh_connections.rs | 25 +++++-
crates/remote/src/ssh_session.rs              | 75 ++++++++++++++------
crates/remote_server/build.rs                 | 21 +++++
crates/remote_server/src/main.rs              |  7 +
script/bundle-mac                             | 10 +-
6 files changed, 107 insertions(+), 41 deletions(-)

Detailed changes

crates/auto_update/src/auto_update.rs 🔗

@@ -84,9 +84,9 @@ pub struct AutoUpdater {
 }
 
 #[derive(Deserialize)]
-struct JsonRelease {
-    version: String,
-    url: String,
+pub struct JsonRelease {
+    pub version: String,
+    pub url: String,
 }
 
 struct MacOsUnmounter {
@@ -482,7 +482,7 @@ impl AutoUpdater {
         release_channel: ReleaseChannel,
         version: Option<SemanticVersion>,
         cx: &mut AsyncAppContext,
-    ) -> Result<(String, String)> {
+    ) -> Result<(JsonRelease, String)> {
         let this = cx.update(|cx| {
             cx.default_global::<GlobalAutoUpdate>()
                 .0
@@ -504,7 +504,7 @@ impl AutoUpdater {
         let update_request_body = build_remote_server_update_request_body(cx)?;
         let body = serde_json::to_string(&update_request_body)?;
 
-        Ok((release.url, body))
+        Ok((release, body))
     }
 
     async fn get_release(

crates/recent_projects/src/ssh_connections.rs 🔗

@@ -14,7 +14,7 @@ use gpui::{AppContext, Model};
 use language::CursorShape;
 use markdown::{Markdown, MarkdownStyle};
 use release_channel::{AppVersion, ReleaseChannel};
-use remote::ssh_session::ServerBinary;
+use remote::ssh_session::{ServerBinary, ServerVersion};
 use remote::{SshConnectionOptions, SshPlatform, SshRemoteClient};
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
@@ -446,7 +446,7 @@ impl remote::SshClientDelegate for SshClientDelegate {
         platform: SshPlatform,
         upload_binary_over_ssh: bool,
         cx: &mut AsyncAppContext,
-    ) -> oneshot::Receiver<Result<(ServerBinary, SemanticVersion)>> {
+    ) -> oneshot::Receiver<Result<(ServerBinary, ServerVersion)>> {
         let (tx, rx) = oneshot::channel();
         let this = self.clone();
         cx.spawn(|mut cx| async move {
@@ -491,7 +491,7 @@ impl SshClientDelegate {
         platform: SshPlatform,
         upload_binary_via_ssh: bool,
         cx: &mut AsyncAppContext,
-    ) -> Result<(ServerBinary, SemanticVersion)> {
+    ) -> Result<(ServerBinary, ServerVersion)> {
         let (version, release_channel) = cx.update(|cx| {
             let version = AppVersion::global(cx);
             let channel = ReleaseChannel::global(cx);
@@ -505,7 +505,10 @@ impl SshClientDelegate {
             let result = self.build_local(cx, platform, version).await?;
             // Fall through to a remote binary if we're not able to compile a local binary
             if let Some((path, version)) = result {
-                return Ok((ServerBinary::LocalBinary(path), version));
+                return Ok((
+                    ServerBinary::LocalBinary(path),
+                    ServerVersion::Semantic(version),
+                ));
             }
         }
 
@@ -540,9 +543,12 @@ impl SshClientDelegate {
                 )
             })?;
 
-            Ok((ServerBinary::LocalBinary(binary_path), version))
+            Ok((
+                ServerBinary::LocalBinary(binary_path),
+                ServerVersion::Semantic(version),
+            ))
         } else {
-            let (request_url, request_body) = AutoUpdater::get_remote_server_release_url(
+            let (release, request_body) = AutoUpdater::get_remote_server_release_url(
                     platform.os,
                     platform.arch,
                     release_channel,
@@ -560,9 +566,14 @@ impl SshClientDelegate {
                     )
                 })?;
 
+            let version = release
+                .version
+                .parse::<SemanticVersion>()
+                .map(ServerVersion::Semantic)
+                .unwrap_or_else(|_| ServerVersion::Commit(release.version));
             Ok((
                 ServerBinary::ReleaseUrl {
-                    url: request_url,
+                    url: release.url,
                     body: request_body,
                 },
                 version,

crates/remote/src/ssh_session.rs 🔗

@@ -227,6 +227,20 @@ pub enum ServerBinary {
     ReleaseUrl { url: String, body: String },
 }
 
+pub enum ServerVersion {
+    Semantic(SemanticVersion),
+    Commit(String),
+}
+
+impl std::fmt::Display for ServerVersion {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::Semantic(version) => write!(f, "{}", version),
+            Self::Commit(commit) => write!(f, "{}", commit),
+        }
+    }
+}
+
 pub trait SshClientDelegate: Send + Sync {
     fn ask_password(
         &self,
@@ -243,7 +257,7 @@ pub trait SshClientDelegate: Send + Sync {
         platform: SshPlatform,
         upload_binary_over_ssh: bool,
         cx: &mut AsyncAppContext,
-    ) -> oneshot::Receiver<Result<(ServerBinary, SemanticVersion)>>;
+    ) -> oneshot::Receiver<Result<(ServerBinary, ServerVersion)>>;
     fn set_status(&self, status: Option<&str>, cx: &mut AsyncAppContext);
 }
 
@@ -1714,34 +1728,47 @@ impl SshRemoteConnection {
         }
 
         let upload_binary_over_ssh = self.socket.connection_options.upload_binary_over_ssh;
-        let (binary, version) = delegate
+        let (binary, new_server_version) = delegate
             .get_server_binary(platform, upload_binary_over_ssh, cx)
             .await??;
 
-        let mut remote_version = None;
         if cfg!(not(debug_assertions)) {
-            if let Ok(installed_version) =
+            let installed_version = if let Ok(version_output) =
                 run_cmd(self.socket.ssh_command(dst_path).arg("version")).await
             {
-                if let Ok(version) = installed_version.trim().parse::<SemanticVersion>() {
-                    remote_version = Some(version);
+                if let Ok(version) = version_output.trim().parse::<SemanticVersion>() {
+                    Some(ServerVersion::Semantic(version))
                 } else {
-                    log::warn!("failed to parse version of remote server: {installed_version:?}",);
+                    Some(ServerVersion::Commit(version_output.trim().to_string()))
                 }
-            }
+            } else {
+                None
+            };
 
-            if let Some(remote_version) = remote_version {
-                if remote_version == version {
-                    log::info!("remote development server present and matching client version");
-                    return Ok(());
-                } else if remote_version > version {
-                    let error = anyhow!("The version of the remote server ({}) is newer than the Zed version ({}). Please update Zed.", remote_version, version);
-                    return Err(error);
-                } else {
-                    log::info!(
-                        "remote development server has older version: {}. updating...",
-                        remote_version
-                    );
+            if let Some(installed_version) = installed_version {
+                use ServerVersion::*;
+                match (installed_version, new_server_version) {
+                    (Semantic(installed), Semantic(new)) if installed == new => {
+                        log::info!("remote development server present and matching client version");
+                        return Ok(());
+                    }
+                    (Semantic(installed), Semantic(new)) if installed > new => {
+                        let error = anyhow!("The version of the remote server ({}) is newer than the Zed version ({}). Please update Zed.", installed, new);
+                        return Err(error);
+                    }
+                    (Commit(installed), Commit(new)) if installed == new => {
+                        log::info!(
+                            "remote development server present and matching client version {}",
+                            installed
+                        );
+                        return Ok(());
+                    }
+                    (installed, _) => {
+                        log::info!(
+                            "remote development server has version: {}. updating...",
+                            installed
+                        );
+                    }
                 }
             }
         }
@@ -2224,12 +2251,12 @@ mod fake {
         },
         select_biased, FutureExt, SinkExt, StreamExt,
     };
-    use gpui::{AsyncAppContext, SemanticVersion, Task};
+    use gpui::{AsyncAppContext, Task};
     use rpc::proto::Envelope;
 
     use super::{
-        ChannelClient, RemoteConnection, ServerBinary, SshClientDelegate, SshConnectionOptions,
-        SshPlatform,
+        ChannelClient, RemoteConnection, ServerBinary, ServerVersion, SshClientDelegate,
+        SshConnectionOptions, SshPlatform,
     };
 
     pub(super) struct FakeRemoteConnection {
@@ -2349,7 +2376,7 @@ mod fake {
             _: SshPlatform,
             _: bool,
             _: &mut AsyncAppContext,
-        ) -> oneshot::Receiver<Result<(ServerBinary, SemanticVersion)>> {
+        ) -> oneshot::Receiver<Result<(ServerBinary, ServerVersion)>> {
             unreachable!()
         }
 

crates/remote_server/build.rs 🔗

@@ -1,3 +1,5 @@
+use std::process::Command;
+
 const ZED_MANIFEST: &str = include_str!("../zed/Cargo.toml");
 
 fn main() {
@@ -7,4 +9,23 @@ fn main() {
         "cargo:rustc-env=ZED_PKG_VERSION={}",
         zed_cargo_toml.package.unwrap().version.unwrap()
     );
+
+    // If we're building this for nightly, we want to set the ZED_COMMIT_SHA
+    if let Some(release_channel) = std::env::var("ZED_RELEASE_CHANNEL").ok() {
+        if release_channel.as_str() == "nightly" {
+            // Populate git sha environment variable if git is available
+            println!("cargo:rerun-if-changed=../../.git/logs/HEAD");
+            if let Some(output) = Command::new("git")
+                .args(["rev-parse", "HEAD"])
+                .output()
+                .ok()
+                .filter(|output| output.status.success())
+            {
+                let git_sha = String::from_utf8_lossy(&output.stdout);
+                let git_sha = git_sha.trim();
+
+                println!("cargo:rustc-env=ZED_COMMIT_SHA={git_sha}");
+            }
+        }
+    }
 }

crates/remote_server/src/main.rs 🔗

@@ -72,7 +72,12 @@ fn main() {
             }
         },
         Some(Commands::Version) => {
-            println!("{}", env!("ZED_PKG_VERSION"));
+            if let Some(build_sha) = option_env!("ZED_COMMIT_SHA") {
+                println!("{}", build_sha);
+            } else {
+                println!("{}", env!("ZED_PKG_VERSION"));
+            }
+
             std::process::exit(0);
         }
         None => {

script/bundle-mac 🔗

@@ -63,6 +63,12 @@ if [[ $# -gt 0 ]]; then
     fi
 fi
 
+# Get release channel
+pushd crates/zed
+channel=$(<RELEASE_CHANNEL)
+export ZED_RELEASE_CHANNEL="${channel}"
+popd
+
 export ZED_BUNDLE=true
 export MACOSX_DEPLOYMENT_TARGET=10.15.7
 
@@ -90,10 +96,6 @@ else
 fi
 
 echo "Creating application bundle"
-pushd crates/zed
-channel=$(<RELEASE_CHANNEL)
-popd
-
 pushd crates/zed
 cp Cargo.toml Cargo.toml.backup
 sed \