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

[24.05] Various Electron backports #337776

Merged
merged 11 commits into from
Aug 29, 2024
Merged

Conversation

emilazy
Copy link
Member

@emilazy emilazy commented Aug 27, 2024

Description of changes

Manual backport of #319252, #319415, #336036, and #335850.

Pending on:

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.

@emilazy
Copy link
Member Author

emilazy commented Aug 27, 2024

This actually also marks 27 and 28 as EOL, which are used by the following packages that don’t use them on masters:

  • blockbench
  • lx-music-desktop
  • ride
  • feishin
  • obsidian
  • webtorrent_desktop

So there’s some more backporting work to do here. I’ll go PR‐hunting in a bit, but I’d appreciate some help.

@emilazy emilazy changed the title [24.05] electron_29-bin: mark as insecure because it's EOL [24.05] electron_{27,28,29}-bin: mark as insecure because they’re EOL Aug 27, 2024
@emilazy
Copy link
Member Author

emilazy commented Aug 27, 2024

(Also I do need to actually either backport the removals or mark the source packages as vulnerable too, of course… Drafting this for now.)

@emilazy emilazy marked this pull request as draft August 27, 2024 19:47
alyssais and others added 2 commits August 27, 2024 20:56
Follow-up to 7892638.

We still have the EOLed `electron-{27,28}-bin` builds, that can be used
instead.

`electron-source.electron_{27,28}` do not build anymore due to LLVM
incompatibilities.

This is beneficial to users of software that still depend on those EOLed
electron versions as well.

Instead of wasting potentially hours of compute trying to build known
broken versions from source, the working binary variants will be used.

Furthermore, this allows us to cleanup the underlying chromium and
electron-source derivations from now unused version conditions such
as version specific patches and build flags.

(cherry picked from commit 268ce0e)
@emilazy emilazy changed the title [24.05] electron_{27,28,29}-bin: mark as insecure because they’re EOL [24.05] electron_{27,28,29}-bin: mark as insecure because they’re EOL; electron-source.electron_{27,28,29}: remove Aug 27, 2024
@emilazy emilazy marked this pull request as ready for review August 27, 2024 20:02
@emilazy emilazy changed the title [24.05] electron_{27,28,29}-bin: mark as insecure because they’re EOL; electron-source.electron_{27,28,29}: remove [24.05] Various Electron backports Aug 27, 2024
@emilazy
Copy link
Member Author

emilazy commented Aug 27, 2024

Okay, I think I have all the relevant Electron commits in this now, but it could definitely use double‐checking…

I’ll work on getting the PRs for the software this would break backported.

@emilazy
Copy link
Member Author

emilazy commented Aug 27, 2024

Final diff of the Electron directory compared to master now looks benign:

diff --git a/pkgs/development/tools/electron/binary/default.nix b/pkgs/development/tools/electron/binary/default.nix
index 07637d13e9..b884428cf8 100644
--- a/pkgs/development/tools/electron/binary/default.nix
+++ b/pkgs/development/tools/electron/binary/default.nix
@@ -1,11 +1,8 @@
-let
-  infoJson = builtins.fromJSON (builtins.readFile ./info.json);
-in
-
 { lib, callPackage }:
 
 let
   mkElectron = callPackage ./generic.nix { };
+  infoJson = builtins.fromJSON (builtins.readFile ./info.json);
 in
 lib.mapAttrs' (majorVersion: info:
   lib.nameValuePair
diff --git a/pkgs/development/tools/electron/common.nix b/pkgs/development/tools/electron/common.nix
index 1e86382771..311757ae1b 100644
--- a/pkgs/development/tools/electron/common.nix
+++ b/pkgs/development/tools/electron/common.nix
@@ -16,7 +16,7 @@
 , pipewire
 , libsecret
 , libpulseaudio
-, speechd-minimal
+, speechd
 , info
 }:
 
@@ -49,7 +49,7 @@
 
   src = null;
 
-  patches = base.patches ++ lib.optional (lib.versionOlder info.version "30")
+  patches = base.patches ++ lib.optional (lib.versionAtLeast info.version "29" && lib.versionOlder info.version "30")
     (substituteAll {
       # disable a component that requires CIPD blobs
       name = "disable-screen-ai.patch";
@@ -171,8 +171,10 @@
     use_qt = false;
     v8_builtins_profiling_log_file = "";
     enable_dangling_raw_ptr_checks = false;
+  } // lib.optionalAttrs (lib.versionAtLeast info.version "28") {
     dawn_use_built_dxc = false;
     v8_enable_private_mapping_fork_optimization = true;
+  } // lib.optionalAttrs (lib.versionAtLeast info.version "29") {
     v8_expose_public_symbols = true;
   } // lib.optionalAttrs (lib.versionOlder info.version "31") {
     use_perfetto_client_library = false;
@@ -204,7 +206,7 @@
         stdenv.cc.cc.lib
         libsecret
         libpulseaudio
-        speechd-minimal
+        speechd
       ];
     in
   base.postFixup + ''

@emilazy emilazy mentioned this pull request Aug 27, 2024
13 tasks
@emilazy
Copy link
Member Author

emilazy commented Aug 27, 2024

Okay, I think I tracked down all the relevant PRs to backport. Still some manual ones to tackle and they all need review.

@emilazy
Copy link
Member Author

emilazy commented Aug 28, 2024

Final diff of the Electron directory compared to master now looks benign:

Or not! Guess I have to backport that change too. I’ll do it tomorrow.

emilylange and others added 9 commits August 28, 2024 15:14
The minimum versions for both chromium and electron-source are higher
than the version bounds of those conditionals.
As such, they can be safely removed.

This is a no-op.

(cherry picked from commit fb9cdbd)
@emilazy
Copy link
Member Author

emilazy commented Aug 28, 2024

This should actually work now. Apologies for my sloppiness; this is my first time working with the Electron packages.

@emilazy
Copy link
Member Author

emilazy commented Aug 29, 2024

This should be ready now.

@teutat3s
Copy link
Member

I'll build this branch on my tower and report back with test results.

@teutat3s
Copy link
Member

Result of nixpkgs-review pr 337776 run on x86_64-linux 1

10 packages marked as broken and skipped:
  • electron_27
  • electron_28
  • electron_29
  • electron_29-bin
  • kuro
  • logseq
  • micropad
  • passky-desktop
  • pocket-casts
  • whalebird
12 packages built:
  • antares
  • deltachat-desktop
  • electron-chromedriver_29
  • electron-chromedriver_30
  • electron_30
  • electron_30-bin
  • feishin
  • lx-music-desktop
  • morgen
  • teams-for-linux
  • webcord
  • webcord-vencord

@emilazy
Copy link
Member Author

emilazy commented Aug 29, 2024

Thanks for checking!

is it expected that electron-chromedriver_29 isn’t marked as vulnerable? Not really relevant to this PR, though, since it shouldn’t be any different on master.

@emilazy emilazy merged commit 893e9c6 into NixOS:release-24.05 Aug 29, 2024
33 of 34 checks passed
@emilazy emilazy deleted the push-snvwpukywltm branch August 29, 2024 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants