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

logseq: Fix publishing graph #205057

Merged
merged 1 commit into from
Jan 31, 2023

Conversation

LoveIsGrief
Copy link
Contributor

@LoveIsGrief LoveIsGrief commented Dec 7, 2022

Description of changes

Related to logseq/logseq#6880

Logseq publishes graphs by copying application assets into a folder and then the graph files (+/- some operations). In a normal linux distribution the application asset directories are rw but only by root. On nix, the directories are read-only, which leads to the copied directories also being ro and logseq failing
to copy the graph files into the target.

A fix from the logseq team isn't forthcoming (yet?), so we circumvent the entire ro issue by using
run-appimage, which extracts the appimage into a user-writeable directory.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 23.05 Release Notes (or backporting 22.11 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

Fixes #199035

@ofborg ofborg bot requested a review from a user December 7, 2022 23:24
@LoveIsGrief LoveIsGrief force-pushed the logseq-run-appimage branch 3 times, most recently from c2db74c to 108c063 Compare December 8, 2022 18:35
@LoveIsGrief LoveIsGrief marked this pull request as ready for review December 8, 2022 22:18
@LoveIsGrief
Copy link
Contributor Author

Could you review this change please @weihua-lu?

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/1681

pkgs/applications/misc/logseq/default.nix Outdated Show resolved Hide resolved
pkgs/applications/misc/logseq/default.nix Outdated Show resolved Hide resolved
@LoveIsGrief LoveIsGrief force-pushed the logseq-run-appimage branch 3 times, most recently from ed76400 to 39e9d6c Compare January 12, 2023 22:01
@LoveIsGrief LoveIsGrief requested review from SuperSandro2000 and removed request for a user January 12, 2023 22:04
@SuperSandro2000
Copy link
Member

Please rebase

Related to logseq/logseq#6880

Logseq publishes graphs by copying application assets into a folder and then the graph files (+/- some operations).
In a normal linux distribution the application asset directories are rw but only by root.
On nix, the directories are read-only, which leads to the copied directories also being ro and logseq failing
 to copy the graph files into the target.

A fix from the logseq team isn't forthcoming (yet?), so we circumvent the entire ro issue by using
 run-appimage, which extracts the appimage into a user-writeable directory.
@LoveIsGrief
Copy link
Contributor Author

Rebased @SuperSandro2000

@ofborg ofborg bot requested a review from a user January 27, 2023 10:38
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/1772

@SuperSandro2000 SuperSandro2000 merged commit 32af394 into NixOS:master Jan 31, 2023
@LoveIsGrief LoveIsGrief deleted the logseq-run-appimage branch January 31, 2023 19:57
@LoveIsGrief
Copy link
Contributor Author

Thanks for merging @SuperSandro2000 🙏

@necauqua
Copy link
Contributor

necauqua commented Feb 1, 2023

It seems that this change broke Logseq Sync:
On start it asks for the password and then hangs (and sometimes unhangs and you can use logseq, but the sync is off - or you can cancel the password prompt).
The log shows

Error occurred in handler for 'main': [Error: Failed to create napi buffer] { code: 'Unknown' }
Error occurred in handler for 'main': [Error: Failed to create napi buffer] { code: 'Unknown' }
Error occurred in handler for 'main': [Error: cannot age decrypt: Header is invalid] { code: 'InvalidArg' }

Logseq version is 0.8.16 (b5b1a46)

And if I roll back to when it wasn't an appimage the version and commit are the same and everything works (except publishing I guess), although it still shows this one error line in logs

Error occurred in handler for 'main': [Error: cannot age decrypt: Header is invalid] { code: 'InvalidArg' }

Soo creating native nodejs buffers broke or something?.

@LoveIsGrief
Copy link
Contributor Author

LoveIsGrief commented Feb 1, 2023 via email

@necauqua
Copy link
Contributor

necauqua commented Feb 4, 2023

I'm not exactly sure what happened, but once again I've switched nixos to latest unstable, and now it asked me to do a full relogin and now it seems to work.
Also logseq asked for a full relogin on android, I'm thinking they actually might've fixed something on their backend 🤷

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.

logseq: cannot export the graph
4 participants