Skip to content

nix: Return to building with crane#21292

Merged
mrnugget merged 1 commit intozed-industries:mainfrom
WHForks:nix-crane
Nov 29, 2024
Merged

nix: Return to building with crane#21292
mrnugget merged 1 commit intozed-industries:mainfrom
WHForks:nix-crane

Conversation

@WeetHet
Copy link
Contributor

@WeetHet WeetHet commented Nov 28, 2024

This removes .envrc, putting it into gitignore as well as building with crane, as it does not require an up to date hash for a FOD.

Release Notes:

  • N/A

cc @mrnugget @jaredramirez

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Nov 28, 2024
@WeetHet
Copy link
Contributor Author

WeetHet commented Nov 28, 2024

Someone on linux should check if this builds for them as well as someone on macOS, just to double check

@WeetHet WeetHet changed the title nix: return to building with crane nix: Return to building with crane Nov 28, 2024
@jaredramirez
Copy link
Contributor

I'm out of town for a bit, so I can't test this builds for a couple weeks!

@mrnugget
Copy link
Contributor

Thanks! Works for me again locally. Since the Nix build is not officially used, I think it's fine to merge this.

@mrnugget mrnugget merged commit 94faf9d into zed-industries:main Nov 29, 2024
@WeetHet WeetHet deleted the nix-crane branch November 29, 2024 11:23
JaLnYn pushed a commit to JaLnYn/zed that referenced this pull request Nov 29, 2024
This removes .envrc, putting it into gitignore as well as building with
crane, as it does not require an up to date hash for a FOD.

Release Notes:

- N/A

cc @mrnugget @jaredramirez
@jaredramirez
Copy link
Contributor

I got around to trying this and I'm hitting this error when building the flake:

error: Cannot find Git revision '799f10133d93ba2a88642cd480d01ec4da53408c' in ref 'refs/heads/main' of repository 'https://github.com/zed-industries/rust-sdks'! Please make sure that the rev exists on the ref you've specified or add allRefs = true; to fetchGit.

I searched the repo for this hash and I see it in Cargo.toml:

livekit = { git = "https://github.com/zed-industries/rust-sdks", rev="799f10133d93ba2a88642cd480d01ec4da53408c", features = ["dispatcher", "services-dispatcher", "rustls-tls-native-roots"], default-features = false }

If I go to the rust-sdks repo and specifically find that commit (here), github show the warning:

This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Someone else in the crane repo has flagged this here, but according to that issue, it seems like this only happens when a repo is corrupt. I can open a new issue, but is it possible to update this dependency to use a different commit? Seems like it would be a good idea in general, not just for fixing the Nix build

@dibenzepin
Copy link

@jaredramirez I'm running into that too, it appears regular cargo can "find" the commit, but crane can't. It appears that commit is part of a closed PR, and after finding the origin branch zed-patches with the identical commit (1f6af333fc8872683c30b39354cec312a1026a0a), changing that in Cargo.toml and running nix build works (well, I think it's working, it's not complaining anymore).

(also, I'd like to ask how exactly is building for nix supposed to work,,,i naïvely just cd'd in and ran nix develop then cargo build, but then it complains of a mold dependency (because their config.toml specifies that linker). Adding pkgs.mold to shell.nix works, but then the build fails on webrtc-sys looking for libbz2.so.1...and adding pkgs.haskellpackages.bz2 doesn't. But then just running nix build on the other hand, works...or appears to work?)

@minhtrancccp
Copy link

minhtrancccp commented Dec 15, 2024

@jaredramirez I'm running into that too, it appears regular cargo can "find" the commit, but crane can't. It appears that commit is part of a closed PR, and after finding the origin branch zed-patches with the identical commit (1f6af333fc8872683c30b39354cec312a1026a0a), changing that in Cargo.toml and running nix build works (well, I think it's working, it's not complaining anymore).

(also, I'd like to ask how exactly is building for nix supposed to work,,,i naïvely just cd'd in and ran nix develop then cargo build, but then it complains of a mold dependency (because their config.toml specifies that linker). Adding pkgs.mold to shell.nix works, but then the build fails on webrtc-sys looking for libbz2.so.1...and adding pkgs.haskellpackages.bz2 doesn't. But then just running nix build on the other hand, works...or appears to work?)

pinging per pr #21550: @mgsloan @SomeoneToIgnore @mikayla-maki

@dibenzepin
Copy link

maybe pinging everyone is a bit much lol 😭

i did manage to get it building in the end (diff), but not with nix build (i was trying to figure out #18982 so it didn't really matter how i did it):

  • i changed packages in shell.nix to include pkgs.mold,
  • buildInputs to have pkgs.bzip2, pkgs.stdenv.cc.cc.lib, and pkgs.vulkan-loader,
  • lib.optionals isLinux to have pkgs.xorg.libxcb and pkgs.wayland

then the usual nix develop and cargo run

(not exactly sure this is how it should be, i think the cc.lib should be Linux-only for instance, but i didn't bother testing on a mac...)

i'm sure someone could probably adapt the package.nix used by nixpkgs...i tried but couldn't get it to work, hence me ham-fisting the shell.nix to make a devshell (they do make it in an fhs-environment and everything though, maybe the main zed team doesn't want that)

i do think #17458 is a good idea though, so something like this doesn't happen again, and maybe nix instructions could be added to script/linux or the Linux.md file at least (i could help with that probs)

mgsloan added a commit that referenced this pull request Dec 29, 2024
Also renamed the repo to `livekit-rustk-sdks` on GitHub for clarity.
The old name will still work for old references.

See [discussion
here](#21292 (comment)). The
only changes in the target repo are immaterial:

```
$ git diff 799f10133d93ba2a88642cd480d01ec4da53408c 060964da10574cd9bf06463a53bf6e0769c5c45e
diff --git a/livekit-protocol/src/livekit.rs b/livekit-protocol/src/livekit.rs
index 43f5496..19181c2 100644
--- a/livekit-protocol/src/livekit.rs
+++ b/livekit-protocol/src/livekit.rs
@@ -3837,7 +3837,7 @@ pub struct SendDataRequest {
     #[prost(string, optional, tag="5")]
     pub topic: ::core::option::Option<::prost::alloc::string::String>,
 }
-
+///
 #[allow(clippy::derive_partial_eq_without_eq)]
 #[derive(Clone, PartialEq, ::prost::Message)]
 pub struct SendDataResponse {
diff --git a/livekit-runtime/Cargo.toml b/livekit-runtime/Cargo.toml
index 4d83cdf..7051f30 100644
--- a/livekit-runtime/Cargo.toml
+++ b/livekit-runtime/Cargo.toml
@@ -7,6 +7,7 @@ edition = "2021"
 repository = "https://github.com/livekit/rust-sdks"

 [features]
+tokio = ["dep:tokio", "dep:tokio-stream"]
 async = ["dep:async-std", "dep:futures", "dep:async-io"]
 dispatcher = ["dep:futures", "dep:async-io", "dep:async-std", "dep:async-task"]
```
@mgsloan
Copy link
Contributor

mgsloan commented Dec 29, 2024

Hey y'all sorry for the delay in looking into the commit SHA thing. Not sure how that happened, but I've updated the SHA in cargo.toml to now point at our zed-patches branch in #22478

github-merge-queue bot pushed a commit that referenced this pull request Dec 29, 2024
Also renamed the repo to `livekit-rustk-sdks` on GitHub for clarity. The
old name will still work for old references.

See [discussion
here](#21292 (comment)).
The only changes in the target repo are immaterial:

```
$ git diff 799f10133d93ba2a88642cd480d01ec4da53408c 060964da10574cd9bf06463a53bf6e0769c5c45e
diff --git a/livekit-protocol/src/livekit.rs b/livekit-protocol/src/livekit.rs
index 43f5496..19181c2 100644
--- a/livekit-protocol/src/livekit.rs
+++ b/livekit-protocol/src/livekit.rs
@@ -3837,7 +3837,7 @@ pub struct SendDataRequest {
     #[prost(string, optional, tag="5")]
     pub topic: ::core::option::Option<::prost::alloc::string::String>,
 }
-
+///
 #[allow(clippy::derive_partial_eq_without_eq)]
 #[derive(Clone, PartialEq, ::prost::Message)]
 pub struct SendDataResponse {
diff --git a/livekit-runtime/Cargo.toml b/livekit-runtime/Cargo.toml
index 4d83cdf..7051f30 100644
--- a/livekit-runtime/Cargo.toml
+++ b/livekit-runtime/Cargo.toml
@@ -7,6 +7,7 @@ edition = "2021"
 repository = "https://github.com/livekit/rust-sdks"

 [features]
+tokio = ["dep:tokio", "dep:tokio-stream"]
 async = ["dep:async-std", "dep:futures", "dep:async-io"]
 dispatcher = ["dep:futures", "dep:async-io", "dep:async-std", "dep:async-task"]
```

Release Notes:

- N/A
helgemahrt pushed a commit to helgemahrt/zed that referenced this pull request Jan 1, 2025
…#22478)

Also renamed the repo to `livekit-rustk-sdks` on GitHub for clarity. The
old name will still work for old references.

See [discussion
here](zed-industries#21292 (comment)).
The only changes in the target repo are immaterial:

```
$ git diff 799f10133d93ba2a88642cd480d01ec4da53408c 060964da10574cd9bf06463a53bf6e0769c5c45e
diff --git a/livekit-protocol/src/livekit.rs b/livekit-protocol/src/livekit.rs
index 43f5496..19181c2 100644
--- a/livekit-protocol/src/livekit.rs
+++ b/livekit-protocol/src/livekit.rs
@@ -3837,7 +3837,7 @@ pub struct SendDataRequest {
     #[prost(string, optional, tag="5")]
     pub topic: ::core::option::Option<::prost::alloc::string::String>,
 }
-
+///
 #[allow(clippy::derive_partial_eq_without_eq)]
 #[derive(Clone, PartialEq, ::prost::Message)]
 pub struct SendDataResponse {
diff --git a/livekit-runtime/Cargo.toml b/livekit-runtime/Cargo.toml
index 4d83cdf..7051f30 100644
--- a/livekit-runtime/Cargo.toml
+++ b/livekit-runtime/Cargo.toml
@@ -7,6 +7,7 @@ edition = "2021"
 repository = "https://github.com/livekit/rust-sdks"

 [features]
+tokio = ["dep:tokio", "dep:tokio-stream"]
 async = ["dep:async-std", "dep:futures", "dep:async-io"]
 dispatcher = ["dep:futures", "dep:async-io", "dep:async-std", "dep:async-task"]
```

Release Notes:

- N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The user has signed the Contributor License Agreement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants