Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zed-editor: 0.165.4 -> 0.166.1 #356757

Merged
merged 4 commits into from
Dec 20, 2024
Merged

Conversation

niklaskorz
Copy link
Contributor

@niklaskorz niklaskorz commented Nov 17, 2024

As zed-industries/zed#21550 has finally been merged, we can prepare for one of the next releases making use of the Livekit Rust SDK for macOS and Linux, instead of just the Livekit Swift SDK for macOS.

This PR provides the necessary work to build the Livekit Rust SDK for Zed.

The from-source built of WebRTC itself, which is required by Livekit, has been implemented by @WeetHet, with some feedback and fixes from my side (the commit log reflects this, where I specified @WeetHet as the main author of the WebRTC commit, with me as co-author). Thank you!

Note: Screensharing and audio recording is currently disabled on Linux. This is an upstream limitation that also exists in the official Zed distributions. You can receive audio and screensharings shared by macOS users though.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@ofborg ofborg bot requested a review from GaetanLepage November 18, 2024 03:24
@github-actions github-actions bot added the 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` label Nov 18, 2024
@ofborg ofborg bot added 8.has: package (new) This PR adds a new package and removed 10.rebuild-darwin: 1 10.rebuild-linux: 1 labels Nov 18, 2024
@niklaskorz niklaskorz force-pushed the zed-editor-livekit branch 3 times, most recently from b5e32d7 to 12c50f9 Compare December 8, 2024 12:38
@niklaskorz niklaskorz mentioned this pull request Dec 11, 2024
13 tasks
@niklaskorz niklaskorz changed the title zed-editor: enable livekit zed-editor: 0.165.x -> 0.166.0 Dec 11, 2024
@bbigras
Copy link
Contributor

bbigras commented Dec 18, 2024

v0.166.1 is out.

@niklaskorz niklaskorz changed the title zed-editor: 0.165.x -> 0.166.0 zed-editor: 0.165.4 -> 0.166.1 Dec 18, 2024
@niklaskorz niklaskorz mentioned this pull request Dec 18, 2024
13 tasks
@niklaskorz niklaskorz force-pushed the zed-editor-livekit branch 2 times, most recently from fd26b71 to af96c0e Compare December 18, 2024 23:54
Copy link
Contributor

@jcdickinson jcdickinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Smoke tested on NixOS, boots to UI just fine.

@niklaskorz niklaskorz force-pushed the zed-editor-livekit branch 3 times, most recently from 9c59284 to be4ffda Compare December 19, 2024 22:16
@niklaskorz
Copy link
Contributor Author

macOS is pretty much ready, checking Linux again tomorrow

@jcdickinson
Copy link
Contributor

jcdickinson commented Dec 20, 2024

Did a smoke test with some light editing, but no collaboration. Seems to be working on NixOS unstable.

Thanks for figuring out LiveKit BTW.

@WeetHet
Copy link
Contributor

WeetHet commented Dec 20, 2024

Did a smoke test with some light editing, but no collaboration. Seems to be working on NixOS unstable.

Thanks for figuring out LiveKit BTW.

I really would love to also have dynamically linked boringssl in the livekit build cause I really don't love statically linking SSL. Probably not in this PR though

@GaetanLepage
Copy link
Contributor

Fails on aarch64-linux: https://paste.glepage.com/raw/sloth-bird-sheep

@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 356757


x86_64-linux

✅ 2 packages built:
  • livekit-libwebrtc
  • zed-editor

aarch64-linux

❌ 2 packages failed to build:
  • livekit-libwebrtc
  • zed-editor

x86_64-darwin

✅ 2 packages built:
  • livekit-libwebrtc
  • zed-editor

aarch64-darwin

✅ 2 packages built:
  • livekit-libwebrtc
  • zed-editor

@niklaskorz
Copy link
Contributor Author

niklaskorz commented Dec 20, 2024

aarch64-linux

❌ 2 packages failed to build:

* livekit-libwebrtc

* zed-editor

Oh interesting, I'll try to figure out what's going wrong there.

Edit: aarch64-linux-gnu-gcc: not found sounds like WebRTC is expecting some kind of cross compiler when targeting aarch64 instead of just using gcc

Edit 2: Fortunately that appears to be a problem that was solved in the Chromium package already, where they have this patch:

lib.optionalString (stdenv.hostPlatform == stdenv.buildPlatform && stdenv.hostPlatform.isAarch64)
''
substituteInPlace build/toolchain/linux/BUILD.gn \
--replace 'toolprefix = "aarch64-linux-gnu-"' 'toolprefix = ""'
''

@niklaskorz niklaskorz force-pushed the zed-editor-livekit branch 2 times, most recently from bfb07f1 to c5d3900 Compare December 20, 2024 12:41
@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 356757


x86_64-linux

✅ 2 packages built:
  • livekit-libwebrtc
  • zed-editor

aarch64-linux

✅ 2 packages built:
  • livekit-libwebrtc
  • zed-editor

x86_64-darwin

✅ 2 packages built:
  • livekit-libwebrtc
  • zed-editor

aarch64-darwin

✅ 2 packages built:
  • livekit-libwebrtc
  • zed-editor

@GaetanLepage
Copy link
Contributor

image

Looks good !

@niklaskorz niklaskorz marked this pull request as ready for review December 20, 2024 15:06
@niklaskorz
Copy link
Contributor Author

niklaskorz commented Dec 20, 2024

Overall I think this is ready. There are some more improvements that could be done to the updateScript of livekit-webrtc (currently it works directly on the branch name while it should fetch the latest commit rev of the m114 branch instead and pass that to gclient2nix), and I'm also not entirely sure about the version = "m114"; (should it be version = "114-unstable-2024-06-05"; instead? WebRTC does not have tagged versions).

I also again want to emphasize the amount of work @WeetHet contributed, without who this would not have been possible. :)

@GaetanLepage
Copy link
Contributor

Overall I think this is ready. There are some more improvements that could be done to the updateScript of livekit-webrtc (currently it works directly on the branch name while it should fetch the latest commit rev of the m114 branch instead and pass that to gclient2nix), and I'm also not entirely sure about the version = "m114"; (should it be version = "114-unstable-2024-06-05"; instead? WebRTC does not have tagged versions).

I also again want to emphasize the amount of work @WeetHet contributed, without who this would not have been possible. :)

Well done guys !

Copy link
Contributor

@GaetanLepage GaetanLepage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 356757


x86_64-linux

✅ 2 packages built:
  • livekit-libwebrtc
  • zed-editor

aarch64-linux

✅ 2 packages built:
  • livekit-libwebrtc
  • zed-editor

x86_64-darwin

✅ 2 packages built:
  • livekit-libwebrtc
  • zed-editor

aarch64-darwin

✅ 2 packages built:
  • livekit-libwebrtc
  • zed-editor

@GaetanLepage GaetanLepage merged commit 019d66f into NixOS:master Dec 20, 2024
43 of 44 checks passed
@niklaskorz niklaskorz deleted the zed-editor-livekit branch January 22, 2025 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants