Skip to content

zed-editor: fix darwin#329653

Merged
adamcstephens merged 2 commits intoNixOS:staging-nextfrom
niklaskorz:zed-darwin
Oct 21, 2024
Merged

zed-editor: fix darwin#329653
adamcstephens merged 2 commits intoNixOS:staging-nextfrom
niklaskorz:zed-darwin

Conversation

@niklaskorz
Copy link
Member

@niklaskorz niklaskorz commented Jul 24, 2024

Description of changes

Fixes #320084.

Once zed-industries/zed#13343 lands, building Zed on macOS becomes much easier as we do not have to deal with building a separate Swift library that Zed depends on anymore.

As zed-industries/zed#13343 seems to need much longer to implement than I originally anticipated, I updated this PR to disable livekit until this change lands, so we can already go ahead and release darwin support without it, using the same internal fallbacks the Linux version is using.

As this PR is making use of the new pattern for using macOS SDKs, it's targetting staging-next, so merging this PR will have to wait until staging-next has been merged into master (and this PR has been retargetted at master).

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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.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.

@github-actions github-actions bot added 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: haskell General-purpose, statically typed, purely functional programming language 6.topic: qt/kde Object-oriented framework for GUI creation 6.topic: kernel The Linux kernel 8.has: documentation This PR adds or changes documentation 8.has: changelog This PR adds or changes release notes 8.has: module (update) This PR changes an existing module in `nixos/` 6.topic: emacs Text editor 6.topic: printing Drivers, CUPS & Co. 6.topic: rust General-purpose programming language emphasizing performance, type safety, and concurrency. 6.topic: golang Go is a high-level general purpose programming language that is statically typed and compiled. 6.topic: ruby A dynamic, open source programming language with a focus on simplicity and productivity. 6.topic: vim Advanced text editor 6.topic: stdenv Standard environment 6.topic: nodejs Node.js is a free, open-source, cross-platform JavaScript runtime environment 6.topic: TeX Issues regarding texlive and TeX in general 6.topic: lua Lua is a powerful, efficient, lightweight, embeddable scripting language. 6.topic: systemd Software suite that provides an array of system components for Linux operating systems. 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related 6.topic: dotnet Language: .NET labels Jul 24, 2024
@niklaskorz niklaskorz changed the base branch from master to staging-next July 24, 2024 13:38
@github-actions github-actions bot removed 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: haskell General-purpose, statically typed, purely functional programming language 6.topic: qt/kde Object-oriented framework for GUI creation 6.topic: kernel The Linux kernel 8.has: documentation This PR adds or changes documentation 8.has: changelog This PR adds or changes release notes 8.has: module (update) This PR changes an existing module in `nixos/` labels Jul 24, 2024
@ofborg ofborg bot requested a review from GaetanLepage October 18, 2024 15:55
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Oct 18, 2024
@niklaskorz niklaskorz force-pushed the zed-darwin branch 2 times, most recently from 3d7a223 to b988f96 Compare October 18, 2024 16:40
@niklaskorz niklaskorz marked this pull request as ready for review October 19, 2024 21:03
repo = "zed";
rev = "refs/tags/v${version}";
hash = "sha256-xtSdlzj1AxhJN4aXLJ+Oy51LX4QduLwcuCfK42kthvE=";
fetchSubmodules = true;
Copy link
Member Author

@niklaskorz niklaskorz Oct 19, 2024

Choose a reason for hiding this comment

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

This seems to have become obsolete for a while now for the Zed repository, so I disabled fetching submodules, but correct me if I'm wrong.

@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 329653


x86_64-linux

✅ 1 package built:
  • zed-editor

aarch64-linux

✅ 1 package built:
  • zed-editor

x86_64-darwin

❌ 1 package failed to build:
  • zed-editor

aarch64-darwin

❌ 1 package failed to build:
  • zed-editor

@niklaskorz
Copy link
Member Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 329653

x86_64-linux

✅ 1 package built:

aarch64-linux

✅ 1 package built:

x86_64-darwin

❌ 1 package failed to build:

aarch64-darwin

❌ 1 package failed to build:

Could you provide any logs for the failing builds? As it builds fine on aarch64-darwin here. I'll try x86_64-darwin in a moment.

@GaetanLepage
Copy link
Contributor

Could you provide any logs for the failing builds? As it builds fine on aarch64-darwin here. I'll try x86_64-darwin in a moment.

Actually I naively ran the tool and didn't notice that the PR was targetting staging.
I think it failed on darwin because cargo wasn't building.

@niklaskorz
Copy link
Member Author

niklaskorz commented Oct 20, 2024

Could you provide any logs for the failing builds? As it builds fine on aarch64-darwin here. I'll try x86_64-darwin in a moment.

Actually I naively ran the tool and didn't notice that the PR was targetting staging. I think it failed on darwin because cargo wasn't building.

I see! Yeah, it takes a good while because Rust and Cargo 1.82 are not cached yet on staging-next, but both should build fine :)

I'm still compiling x86_64-darwin:

┏━ Dependency Graph:
┃    ┌─ ⏸ xim-parser-0.2.1-x86_64-darwin waiting for 1 ⏵
┃    │           ┌─ ⏸ cargo-build-hook.sh-x86_64-darwin waiting for 1 ⏵
┃    │           │           ┌─ ⏵ llvm-18.1.8-x86_64-darwin (buildPhase) ⏱ 15m39s
┃    │           │        ┌─ ⏸ rustc-1.82.0-x86_64-darwin
┃    │           │     ┌─ ⏸ rustc-wrapper-1.82.0-x86_64-darwin
┃    │           │  ┌─ ⏸ cargo-1.82.0-x86_64-darwin
┃    │           ├─ ⏸ cargo-check-hook.sh-x86_64-darwin
┃    │        ┌─ ⏸ cargo-auditable-0.6.2-x86_64-darwin
┃    │     ┌─ ⏸ auditable-cargo-bootstrap-1.81.0-x86_64-darwin
┃    │  ┌─ ⏸ cargo-1.82.0-x86_64-darwin
┃    ├─ ⏸ xim-ctext-0.3.0-x86_64-darwin
┃ ┌─ ⏸ cargo-vendor-dir-x86_64-darwin
┃ ⏸ zed-editor-0.157.5-x86_64-darwin

Edit: Took quite a while to compile through Rosetta, but finished now.

┃ ✔ zed-editor-0.157.5-x86_64-darwin ⏱ 1h3m58s
┣━━━ Builds            │ Downloads          │ Host
┃        │ ✔ 330 │     │     │        │     │ localhost
┃        │       │     │     │ ↓ 1308 │     │ https://cache.nixos.org
┗━ ∑ ⏵ 0 │ ✔ 330 │ ⏸ 0 │ ↓ 0 │ ↓ 1308 │ ⏸ 0 │ Finished at 13:55:58 after 3h24m47s

$ file result/Applications/Zed.app/Contents/MacOS/zed
result/Applications/Zed.app/Contents/MacOS/zed: Mach-O 64-bit executable x86_64
grafik grafik

@GaetanLepage
Copy link
Contributor

Good job on this @niklaskorz !
Do you want to merge this against staging-next or wait that it lands on master first and then merge it on master ?

@@ -191,6 +195,9 @@ rustPlatform.buildRustPackage rec {
ZED_UPDATE_EXPLANATION = "zed has been installed using nix. Auto-updates have thus been disabled.";
Copy link
Contributor

@jansol jansol Oct 20, 2024

Choose a reason for hiding this comment

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

Suggested change
ZED_UPDATE_EXPLANATION = "zed has been installed using nix. Auto-updates have thus been disabled.";
ZED_UPDATE_EXPLANATION = "Zed has been installed using Nix. Auto-updates have thus been disabled.";

Copy link
Member Author

Choose a reason for hiding this comment

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

Unrelated to this PR, but I agree that looks nicer, will add it as a separate commit :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, hence not an actual review :D It just caught my eye on the above screenshots

@niklaskorz
Copy link
Member Author

niklaskorz commented Oct 20, 2024

Good job on this @niklaskorz ! Do you want to merge this against staging-next or wait that it lands on master first and then merge it on master ?

I am fine with both. On the "Nix on macOS" Matrix room I asked what the usual procedure was and @khaneliman answered:

If there's a dependency on something in staging/staging-next I don't think it's necessarily wrong to target them if you don't want to wait until it propagates down. I think there's a chance stuff gets ripped out or reverted in those branches, though so it might make sense to wait until it gets to master. Unless, you're more involved in the staging flow.

But I don't think there is any chance at this point that the new Apple SDK and darwin minimum version hook get removed from staging-next, as they were a long time coming before they were merged into staging. And these are the only two things from staging-next this PR relies on. Also @emilazy is already aware of the NIX_CFLAGS_COMPILE workaround in this PR for the currently disabled -isysroot, so she knows where to find and remove it once #349555 progresses.

@GaetanLepage
Copy link
Contributor

Good job on this @niklaskorz ! Do you want to merge this against staging-next or wait that it lands on master first and then merge it on master ?

I am fine with both. On the "Nix on macOS" Matrix room I asked what the usual procedure was and @khaneliman answered:

If there's a dependency on something in staging/staging-next I don't think it's necessarily wrong to target them if you don't want to wait until it propagates down. I think there's a chance stuff gets ripped out or reverted in those branches, though so it might make sense to wait until it gets to master. Unless, you're more involved in the staging flow.

But I don't think there is any chance at this point that the new Apple SDK and darwin minimum version hook get removed from staging-next, as they were a long time coming before they were merged into staging. And these are the only two things from staging-next this PR relies on. Also @emilazy is already aware of the NIX_CFLAGS_COMPILE workaround in this PR for the currently disabled -isysroot, so she knows where to find and remove it once #349555 progresses.

Ok, let's go for it then :)

@ofborg ofborg bot requested a review from GaetanLepage October 20, 2024 22:30
@adamcstephens adamcstephens merged commit 67d8538 into NixOS:staging-next Oct 21, 2024
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/title-the-darwin-sdks-have-been-updated/55295/1

@niklaskorz niklaskorz deleted the zed-darwin branch December 20, 2024 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build failure: zed-editor

9 participants