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

electron_29-bin: mark as insecure because it's EOL, electron-source.electron_29: remove as it's EOL #335850

Merged
merged 2 commits into from
Aug 27, 2024

Conversation

emilylange
Copy link
Member

@emilylange emilylange commented Aug 19, 2024

Description of changes

electron_29 will be EOL tomorrow (2024-08-20), as per https://www.electronjs.org/docs/latest/tutorial/electron-timelines.

Packages that depend on electron_29 and as such will require the user to opt-into the knownVulnerabilities.

Related: #333907

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.

@hatch01
Copy link
Contributor

hatch01 commented Aug 19, 2024

Not sure I fully understood what I should do.
Should I create a PR for Antares with the value knownVulnerability set to the same as you for electron29 ?
Or will nix prevent user automatically because Antares depend in electron29.
I can also just quickly create a PR to switch to electron30 which seems supported.

@NotAShelf
Copy link
Member

I'll take a look at bumping webcord-vencord's electron version - if upstream supports it.

@emilylange
Copy link
Member Author

Not sure I fully understood what I should do.
Should I create a PR for Antares with the value knownVulnerability set to the same as you for electron29 ?
Or will nix prevent user automatically because Antares depend in electron29.

If this PR gets merged as is, the following would happen:

# nix-build -A antares
error:
       … while evaluating 'strict' to select 'drvPath' on it
         at /builtin/derivation.nix:1:552:
       … while calling the 'derivationStrict' builtin
         at /builtin/derivation.nix:1:208:
       (stack trace truncated; use '--show-trace' to show the full trace)

       error: Package ‘electron-29.4.5’ in ./pkgs/development/tools/electron/binary/generic.nix:36 is marked as insecure, refusing to evaluate.


       Known issues:
        - Electron version 29.4.5 is EOL

       You can install it anyway by allowing this package, using the
       following methods:

       a) To temporarily allow all insecure packages, you can use an environment
          variable for a single invocation of the nix tools:

            $ export NIXPKGS_ALLOW_INSECURE=1

          Note: When using `nix shell`, `nix build`, `nix develop`, etc with a flake,
                then pass `--impure` in order to allow use of environment variables.

       b) for `nixos-rebuild` you can add ‘electron-29.4.5’ to
          `nixpkgs.config.permittedInsecurePackages` in the configuration.nix,
          like so:

            {
              nixpkgs.config.permittedInsecurePackages = [
                "electron-29.4.5"
              ];
            }

       c) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
          ‘electron-29.4.5’ to `permittedInsecurePackages` in
          ~/.config/nixpkgs/config.nix, like so:

            {
              permittedInsecurePackages = [
                "electron-29.4.5"
              ];
            }

I can also just quickly create a PR to switch to electron30 which seems supported.

That would be great and exactly what I wanted to get across without actually spelling it out.
Sorry for that.

Sometimes it's as simple as keeping the current version and bumping electron, other times it may require upstream to release a newer version.
It's up to the package maintainers to figure out and decide.

@hatch01
Copy link
Contributor

hatch01 commented Aug 19, 2024

#335924
I'm am working on it but facing some issue with the npm deps.

@SuperSandro2000
Copy link
Member

Lets wait a day or two until we have reviewed and merged the linked PRs. We are in no rush here.

@SuperSandro2000
Copy link
Member

We are a bit stuck with #330137, so some help is appreciated.

@emilylange emilylange marked this pull request as ready for review August 23, 2024 15:02
@amarshall
Copy link
Member

Have a working update for bitwarden-desktop in #337164.

@emilazy
Copy link
Member

emilazy commented Aug 25, 2024

This should be backported to 24.05 after #336018, right?

@emilylange
Copy link
Member Author

Yes, should get backported.

@ofborg eval

@emilazy emilazy added the backport release-24.05 Backport PR automatically label Aug 25, 2024
@hatch01
Copy link
Contributor

hatch01 commented Aug 25, 2024

@emilylange my pr for antares (#335924) is ready for review !

@emilazy
Copy link
Member

emilazy commented Aug 26, 2024

Backports in progress:

Seems unlikely we’ll get more PRs beyond the antares one in a timely fashion at this point?

@NotAShelf
Copy link
Member

NotAShelf commented Aug 26, 2024

I can pick one or two programs to update if maintainers don't respond.

@emilazy
Copy link
Member

emilazy commented Aug 26, 2024

That’d be nice :) But if there’s no active maintainers for a package we’ll probably run into the same situation with them again next time an Electron version goes EOL, so if you don’t want to adopt any of them and no maintainer appears to sort them out it might be better in the long run to let nature take its course.

(Hypocritical of me to say given the amount of time I’ve spent patching abandoned packages for newer FFmpegs lately, though…)

@teutat3s teutat3s added 12.approvals: 1 This PR was reviewed and approved by one reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package labels Aug 26, 2024
@NotAShelf
Copy link
Member

Seeing no maintainers responded, I'll pick up kuro and maybe teams-for-linux.

Kuro seems to work with electron_30 just fine (despite upstream still providing electron 22, may be a problem in the future). teams-for-linux needs testing.

This should not be considered a blocker though, as I might need a day to create the PRs.

@SuperSandro2000 SuperSandro2000 merged commit 2f12730 into NixOS:master Aug 27, 2024
32 of 33 checks passed
Copy link
Contributor

Backport failed for release-24.05, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-24.05
git worktree add -d .worktree/backport-335850-to-release-24.05 origin/release-24.05
cd .worktree/backport-335850-to-release-24.05
git switch --create backport-335850-to-release-24.05
git cherry-pick -x 39f4f0877bcbd5c7ad569e203e1b8d6feaa683d6 fd911150a24ccb9ec4594bba2f9c4062bc23d990

@emilylange emilylange deleted the electron_29-EOL branch August 27, 2024 19:30
@emilazy
Copy link
Member

emilazy commented Aug 27, 2024

I’ll handle the backport.

@emilazy emilazy mentioned this pull request Aug 27, 2024
13 tasks
@khaneliman
Copy link
Contributor

Seeing no maintainers responded, I'll pick up kuro and maybe teams-for-linux.

Kuro seems to work with electron_30 just fine (despite upstream still providing electron 22, may be a problem in the future). teams-for-linux needs testing.

This should not be considered a blocker though, as I might need a day to create the PRs.

I got an open PR out for teams-for-linux raf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 12.approvals: 1 This PR was reviewed and approved by one reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package backport release-24.05 Backport PR automatically
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants