Skip to content

Commit

Permalink
collab: Remove dependency on X11 (#19079)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
shish authored Oct 11, 2024
1 parent 84b61c8 commit f1c45d9
Show file tree
Hide file tree
Showing 6 changed files with 6 additions and 30 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 1 addition & 22 deletions Dockerfile-collab
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
2 changes: 1 addition & 1 deletion crates/assistant/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
});
}

Expand Down
6 changes: 3 additions & 3 deletions crates/assistant/src/inline_assistant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
});
}
}
Expand Down Expand Up @@ -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()),
});
}
}
Expand Down Expand Up @@ -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()),
});
}

Expand Down
1 change: 0 additions & 1 deletion crates/telemetry_events/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,5 @@ workspace = true
path = "src/telemetry_events.rs"

[dependencies]
language.workspace = true
semantic_version.workspace = true
serde.workspace = true
3 changes: 1 addition & 2 deletions crates/telemetry_events/src/telemetry_events.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -150,7 +149,7 @@ pub struct AssistantEvent {
pub model_provider: String,
pub response_latency: Option<Duration>,
pub error_message: Option<String>,
pub language_name: Option<LanguageName>,
pub language_name: Option<String>,
}

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
Expand Down

0 comments on commit f1c45d9

Please sign in to comment.