Skip to content

Comments

joplin-desktop: use electron_36#442657

Merged
pyrox0 merged 1 commit intoNixOS:masterfrom
fugidev:joplin-desktop-electron_36
Sep 17, 2025
Merged

joplin-desktop: use electron_36#442657
pyrox0 merged 1 commit intoNixOS:masterfrom
fugidev:joplin-desktop-electron_36

Conversation

@fugidev
Copy link
Member

@fugidev fugidev commented Sep 13, 2025

Electron 35 is EOL and being deprecated in nixpkgs.

Upstream currently still uses v35 in the latest stable release, next one should use v37.

A bug when using Electron 37 was reported: #440567
Using Electron 36, the bug does not occur according to my tests, so I think it's best to update to this version for now.

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 9.needs: reviewer This PR currently has no reviewers requested and needs attention. labels Sep 13, 2025
Copy link
Contributor

@ThanePatrol ThanePatrol left a comment

Choose a reason for hiding this comment

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

Haven't seen build results but it looks like you built it so lgtm :)

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 1 This PR was reviewed and approved by one person. and removed 9.needs: reviewer This PR currently has no reviewers requested and needs attention. labels Sep 14, 2025
@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Sep 15, 2025
@pyrox0 pyrox0 added this pull request to the merge queue Sep 17, 2025
@pyrox0 pyrox0 added 1.severity: security Issues which raise a security issue, or PRs that fix one backport release-25.05 labels Sep 17, 2025
Merged via the queue into NixOS:master with commit 69839cc Sep 17, 2025
40 of 42 checks passed
@nixpkgs-ci
Copy link
Contributor

nixpkgs-ci bot commented Sep 17, 2025

Backport failed for release-25.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-25.05
git worktree add -d .worktree/backport-442657-to-release-25.05 origin/release-25.05
cd .worktree/backport-442657-to-release-25.05
git switch --create backport-442657-to-release-25.05
git cherry-pick -x 5f619c0c3ccbc17946cc117599d26c2e1b829588

@fugidev
Copy link
Member Author

fugidev commented Sep 17, 2025

Can't be backported because all of the recent commits haven't been either. On the release-25.05 branch, this package is still being built from the AppImage – version 3.3.12, which uses Electron 35.2.1. What is the best course of action here?

a) backport everything to release-25.05 (possibly breaking changes, although no major breakage has been reported from unstable users)
b) mark the package on release-25.05 as insecure (should be no big deal for end users as it's a from-binary packaging)
c) don't do anything?

I think b would be best, what do you think @pyrox0? I can take care of it then.

@fugidev fugidev deleted the joplin-desktop-electron_36 branch September 17, 2025 17:41
@pyrox0
Copy link
Member

pyrox0 commented Sep 17, 2025

I'd prefer A, but if B makes your life easier then that's okay with me as well. Would it be possible to skip bumping the package and just bump the electron bin version it uses? Then we wouldn't have to mark insecure but wouldn't risk breaking changes for joplin users either.

@fugidev
Copy link
Member Author

fugidev commented Sep 17, 2025

Hmm, I think just using a newer Electron won't work, because the app is compiled against a specific version. I could however try backporting the source build without bumping the package version. If that turns out not to be trivial, I would rather mark the package as insecure tho. Is it okay to ping you for a review on a follow up PR? :)

@pyrox0
Copy link
Member

pyrox0 commented Sep 17, 2025

pinging me would be A-OK!

@fugidev fugidev added the 8.has: port to stable This PR already has a backport to the stable release. label Sep 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.severity: security Issues which raise a security issue, or PRs that fix one 8.has: port to stable This PR already has a backport to the stable release. 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. 12.approvals: 2 This PR was reviewed and approved by two persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants