obsidian: use the bundled Electron in Obsidian binary instead of from nixpkgs

[hesprs]

Obsidian's Nix derivation fetches the compiled Obsidian tarball from Obsidian GitHub release. The fetched tarball already has Electron bundled.

Original derivation enforces Obsidian to use electron_39 from Nixpkgs instead of the one comes with it. Obsidian team seems to have patched Electron itself in the updates between v1.11.4 and v1.12.7. Using electron_39 in Nixpkgs invalidates the patch and causes certain bugs. A known one is https://forum.obsidian.md/t/keyboard-triggered-context-menu-does-not-show-spell-suggestions/114141

This commit removes electron_39 and _7zz dependency, making Obsidian depend on the Electron that is complied with it.

This commit also attempts to improve Electron's secret store auto-detection. The original detection fails on non-KDE or non-Gnome environments. A wrapper script is added to detect the currently running secret service and set the secret store backend flag instead of relying on matching user's desktop environment. This issue has already been improved in Electron 42: electron/electron#49054. But since Obsidian uses Obsidian 39, this patch is still needed.

Things done

• Built on platform:
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• Tested, as applicable:
  • NixOS tests in nixos/tests.
  • Package tests at passthru.tests.
  • Tests in lib/tests or pkgs/test for functions and "core" functionality.
• 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.

[w-lfchen]

what is the difference between this pr and #519461?

[hesprs]

That PR fails the commit message lint check and contains multiple commits. This PR has only one done cleanly.

[zaninime]

I understand this PR fixes a real problem, however I'm not sure how I feel about switching to a binary distribution of Electron (that is being binary-patched anyway, by the derivation), instead of relying on the one built by Hydra.

A known one is https://forum.obsidian.md/t/keyboard-triggered-context-menu-does-not-show-spell-suggestions/114141

I read through the discussion, but I haven't seen any mentions made by the maintainers about custom patches made on Electron. While Electron is MIT-licensed (hence Obsidian's team is not forced to release the source code with their modifications) I am sure they have interest in upstreaming any improvements they make to it.

Have you tried keeping the other changes you made in this PR, but keeping electron_39?

[zaninime]

Also: there's no need to open a new PR every time you want to change something, or a check is failing. Just amend the commits.

[hesprs]

but I haven't seen any mentions made by the maintainers about custom patches made on Electron.

I made this conclusion by doing experiments and control variables. The experiments are:

1. use current nixpkgs-unstable: spell check fails inconsistently
2. use nixpkgs-25.11: spell check works
3. use AppImage of the latest version of Obsidian: spell check works
4. use the derivation in this PR: spell check works

Variables controlled:

1. system environment
2. (in experiment 1 and 2) electron version. The version of electron_39 are the same 39.8.9
3. (in experiment 1 and 2) derivation script. The script only varies slightly about Obsidian CLI between unstable and 25.11, doesn't affect Obsidian Electron APP.

Deduction:

1. near everything was the same in 2 and 4, but the bug disappears in 4.
2. the only difference between 2 and 4 (or 2 and 3) is the Electron used.
3. Electron doesn't change in 1 and 2, but the bug appears in 2.

Obviously, Electron itself is changed (I guess it was a patch by Obsidian team) between 1 and 2.

I am sure they have interest in upstreaming any improvements they make to it.

Maybe we can try, but I think is less feasible to ask a proprietary software team about this.

Have you tried keeping the other changes you made in this PR, but keeping electron_39?

I used to use an overlay: https://forum.obsidian.md/t/how-does-obsidian-linux-find-system-secret-storage/113465/2

Secret store worked, spell check still failed.

Also: there's no need to open a new PR every time you want to change something, or a check is failing. Just amend the commits.

My bad, I'm not so skillful at rewriting Git history though.

[w-lfchen]

enforces Obsidian to use electron_39

you can override it

Obsidian team seems to have patched Electron itself in the updates between v1.11.4 and v1.12.7

proof?

This commit also attempts to improve Electron's secret store auto-detection.

this should be done in a separate commit, maybe even a separate pr.
and shouldn't this be directed at electron?

using obsidian's electron invalidates any nix-specific patches to electron. i am opposed to doing that.

[w-lfchen]

we are in nixpkgs, using pkgs anywhere is just wrong. he derivation is passed to callPackage, which resolves the arguments.

your entire derivation is full of this pattern, and needs to be rewritten without it.

[w-lfchen]

shouldn't the binary already be executable?

[w-lfchen]

also, your commit description is not long enough. it is clearly missing a recipe for a delicious chocolate cake, please amend the commit accordingly.

[hesprs]

I just figured out everything. The bug that I described is not caused by any "patch" from Obsidian team. It is caused by nixpkgs-unstable update timing. Last time I update my flakes was last month, at that time, the Obsidian derivation was:

  fetchurl,
  lib,
  makeWrapper,
  electron,
  makeDesktopItem,
  imagemagick,
  autoPatchelfHook,
  writeScript,
  _7zz,
  commandLineArgs ? "",
}:

electron defaults to Electron 41 while Obsidian is using 39. Causing severe version mismatch.

In #510075, Electron was pinned at 39. But since that happened after my update, my version stopped at the broken 41 version. I didn't know the update and assumed I was seeing the bug with Electron 39 when making this PR.

The correct fix is simply updating my flakes.

I feel extremely sorry about the back and forth and my unreasonable allegations. Please forgive my ignorance and arrogance during the discussion on this issue. I will close this PR right now and there won't be more PRs about this.