From f1c45d988e227f40260e03ad2cbea0e625273d7a Mon Sep 17 00:00:00 2001 From: Shish Date: Fri, 11 Oct 2024 18:28:34 +0100 Subject: [PATCH] collab: Remove dependency on X11 (#19079) collab: Remove dependency on X11 I'm not sure if this is the best solution (perhaps pulling `LanguageName` into a separate `language_types` crate would be better...?) - but it massively reduces build time / dependencies / size and means that the collab server no longer requires X11 libraries to be installed. tl;dr: `telemetry_events` requires the `language` crate, and the language crate requires a whole ton of extra stuff. Since telemetry_events only uses `language` for a single type definition (`LanguageName`, aka `String`), we can cut all of these out by using the base `String` type (This doesn't seem too terrible, given that all other telemetry fields are using basic datatypes like String as opposed to more strongly-typed variants). FYI the dependency tree for "why does collab need X11 libraries??" looks like this: ``` collab \- telemetry_events \- language |- gpui |- fuzzy | \- gpui |- git | \- gpui |- lsp | |- gpui | \- release_channel | \- gpui |- settings | |- fs | | \- gpui | \- gpui |- task | \- gpui \- theme \- gpui ``` Release Notes: - N/A --- Cargo.lock | 1 - Dockerfile-collab | 23 +------------------ crates/assistant/src/context.rs | 2 +- crates/assistant/src/inline_assistant.rs | 6 ++--- crates/telemetry_events/Cargo.toml | 1 - .../telemetry_events/src/telemetry_events.rs | 3 +-- 6 files changed, 6 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 752050c995..0a607155e0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11346,7 +11346,6 @@ dependencies = [ name = "telemetry_events" version = "0.1.0" dependencies = [ - "language", "semantic_version", "serde", ] diff --git a/Dockerfile-collab b/Dockerfile-collab index 7945eb426d..507ad3be19 100644 --- a/Dockerfile-collab +++ b/Dockerfile-collab @@ -13,30 +13,9 @@ ARG GITHUB_SHA ENV GITHUB_SHA=$GITHUB_SHA -# At some point in the past 3 weeks, additional dependencies on `xkbcommon` and -# `xkbcommon-x11` were introduced into collab. -# -# A `git bisect` points to this commit as being the culprit: `b8e6098f60e5dabe98fe8281f993858dacc04a55`. -# -# Now when we try to build collab for the Docker image, it fails with the following -# error: -# -# ``` -# 985.3 = note: /usr/bin/ld: cannot find -lxkbcommon: No such file or directory -# 985.3 /usr/bin/ld: cannot find -lxkbcommon-x11: No such file or directory -# 985.3 collect2: error: ld returned 1 exit status -# ``` -# -# The last successful deploys were at: -# - Staging: `4f408ec65a3867278322a189b4eb20f1ab51f508` -# - Production: `fc4c533d0a8c489e5636a4249d2b52a80039fbd7` -# # Also add `cmake`, since we need it to build `wasmtime`. -# -# Installing these as a temporary workaround, but I think ideally we'd want to figure -# out what caused them to be included in the first place. RUN apt-get update; \ - apt-get install -y --no-install-recommends libxkbcommon-dev libxkbcommon-x11-dev cmake + apt-get install -y --no-install-recommends cmake RUN --mount=type=cache,target=./script/node_modules \ --mount=type=cache,target=/usr/local/cargo/registry \ diff --git a/crates/assistant/src/context.rs b/crates/assistant/src/context.rs index 1a5a4d44f0..610652a371 100644 --- a/crates/assistant/src/context.rs +++ b/crates/assistant/src/context.rs @@ -2160,7 +2160,7 @@ impl Context { model_provider: model.provider_id().to_string(), response_latency, error_message, - language_name, + language_name: language_name.map(|name| name.to_proto()), }); } diff --git a/crates/assistant/src/inline_assistant.rs b/crates/assistant/src/inline_assistant.rs index b3427271b9..4ecf8dd37e 100644 --- a/crates/assistant/src/inline_assistant.rs +++ b/crates/assistant/src/inline_assistant.rs @@ -267,7 +267,7 @@ impl InlineAssistant { model_provider: model.provider_id().to_string(), response_latency: None, error_message: None, - language_name: buffer.language().map(|language| language.name()), + language_name: buffer.language().map(|language| language.name().to_proto()), }); } } @@ -788,7 +788,7 @@ impl InlineAssistant { model_provider: model.provider_id().to_string(), response_latency: None, error_message: None, - language_name, + language_name: language_name.map(|name| name.to_proto()), }); } } @@ -2954,7 +2954,7 @@ impl CodegenAlternative { model_provider: model_provider_id.to_string(), response_latency, error_message, - language_name, + language_name: language_name.map(|name| name.to_proto()), }); } diff --git a/crates/telemetry_events/Cargo.toml b/crates/telemetry_events/Cargo.toml index 34ccc20c3a..01145549b1 100644 --- a/crates/telemetry_events/Cargo.toml +++ b/crates/telemetry_events/Cargo.toml @@ -12,6 +12,5 @@ workspace = true path = "src/telemetry_events.rs" [dependencies] -language.workspace = true semantic_version.workspace = true serde.workspace = true diff --git a/crates/telemetry_events/src/telemetry_events.rs b/crates/telemetry_events/src/telemetry_events.rs index 14bcf985bf..47e66a46a7 100644 --- a/crates/telemetry_events/src/telemetry_events.rs +++ b/crates/telemetry_events/src/telemetry_events.rs @@ -1,6 +1,5 @@ //! See [Telemetry in Zed](https://zed.dev/docs/telemetry) for additional information. -use language::LanguageName; use semantic_version::SemanticVersion; use serde::{Deserialize, Serialize}; use std::{fmt::Display, sync::Arc, time::Duration}; @@ -150,7 +149,7 @@ pub struct AssistantEvent { pub model_provider: String, pub response_latency: Option, pub error_message: Option, - pub language_name: Option, + pub language_name: Option, } #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]