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*: mark older versions as EOL #319252

Merged
merged 1 commit into from
Jun 12, 2024
Merged

Conversation

alyssais
Copy link
Member

@alyssais alyssais commented Jun 12, 2024

Description of changes

https://www.electronjs.org/docs/latest/tutorial/electron-timelines#timeline

Some discussion in #318857 (comment).

Heads up to maintainers of applications that should be updated to avoid using these versions:

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.

Copy link
Member

@vcunat vcunat left a comment

Choose a reason for hiding this comment

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

Looks OK to me, though I don't really know electron stuff in particular.

@SuperSandro2000
Copy link
Member

We need to wait for the next bitwarden-desktop release to contain bitwarden/clients@a4dd1f4 or just bump to electron 28.

@vcunat
Copy link
Member

vcunat commented Jun 12, 2024

28 is also EOL now. EDIT: and more urgently, it's about to get broken: https://hydra.nixos.org/build/262684505

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Jun 12, 2024
Copy link
Member

@yayayayaka yayayayaka left a comment

Choose a reason for hiding this comment

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

Thanks!

@yayayayaka yayayayaka added 12.approvals: 3+ This PR was reviewed and approved by three or more reputable people 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package labels Jun 12, 2024
@onny
Copy link
Contributor

onny commented Jun 12, 2024

Added a upstream bug report for Feishin about updating Electron version jeffvli/feishin#643

@SuperSandro2000
Copy link
Member

28 is also EOL now

That's an arbitrary decision we can also delay by a week or two with no hard drawbacks. It only becomes a problem if we delay it for a long time.

Having to compile electron is a hard drawback and most people cannot even do that or it becomes an unreasonable hard and unjustifiable burden just to have that EOL version up to date.

Also that build failure seems to be just a gcc warning being treated as error. We can ignore that.

@SuperSandro2000
Copy link
Member

Noise, they literally tagged a release 30 minutes ago https://github.com/bitwarden/clients/releases/tag/desktop-v2024.6.0

@@ -234,6 +234,7 @@ in (chromium.override { upstream-info = info.chromium; }).mkDerivation (base: {
meta = with lib; {
description = "Cross platform desktop application shell";
homepage = "https://github.com/electron/electron";
knownVulnerabilities = optional (versionOlder info.version "29") "Electron version ${version} is EOL";
Copy link
Member

Choose a reason for hiding this comment

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

Don't we usually remove the EOL electron-source versions and mark the binary electron_*-bin version EOL?
Not both?

See #295672 (c45702d and 1f9c426).

@vcunat
Copy link
Member

vcunat commented Jun 12, 2024

@SuperSandro2000 as I mentioned above, electron 27 and 28 also won't build now anymore (due to LLVM incompatibilities).

@vcunat
Copy link
Member

vcunat commented Jun 12, 2024

They now fail to build on nixpkgs master, so I merged already to avoid Hydra expending lots of resources on futile builds.

@vcunat vcunat merged commit d7cfd8c into NixOS:master Jun 12, 2024
31 checks passed
@emilylange
Copy link
Member

Don't we usually remove the EOL electron-source versions and mark the binary electron_*-bin version EOL? Not both?

See #295672 (c45702d and 1f9c426).

I opened #319415 for what I meant, including version conditional cleanups in both the underlying chromium derivation and electron-source.

jlbribeiro added a commit to jlbribeiro/nixpkgs that referenced this pull request Jun 13, 2024
Addresses NixOS#319252: v27 has been EOL since 2024/04/16.
@jlbribeiro jlbribeiro mentioned this pull request Jun 13, 2024
13 tasks
@yu-re-ka
Copy link
Contributor

For reference, the intention when adding source-built electron was to keep around only non-EOL versions as source-built, so once they go EOL, it should be switched over to the binary version (which should also be marked as EOL) as done in #319415

@alyssais alyssais deleted the electron-eol branch July 17, 2024 20:00
@jlbribeiro jlbribeiro mentioned this pull request Aug 2, 2024
13 tasks
@emilazy emilazy mentioned this pull request Aug 27, 2024
13 tasks
emilazy pushed a commit to emilazy/nixpkgs that referenced this pull request Aug 28, 2024
Addresses NixOS#319252: v27 has been EOL since 2024/04/16.

(cherry picked from commit 5e4fd0a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux 12.approvals: 3+ This PR was reviewed and approved by three or more reputable people 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants