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
todoist-electron: init at 1.19 #79167
Conversation
11bf902
to
a5cdfa1
Compare
@GrahamcOfBorg eval |
Ah, it's throwing an error because |
a5cdfa1
to
28a77c8
Compare
@GrahamcOfBorg eval |
1 similar comment
@GrahamcOfBorg eval |
ofborg has been slow recently, not sure why |
Yep, that fixed it. Everything's good to go. |
|
||
# Hacky workaround for RPATH problems | ||
makeWrapper $out/opt/Todoist/todoist $out/bin/todoist \ | ||
--prefix LD_LIBRARY_PATH : ${lib.makeLibraryPath [ libpulseaudio udev ]} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't be needed with other patchelf usage, unless another binary is using them.
--prefix LD_LIBRARY_PATH : ${lib.makeLibraryPath [ libpulseaudio udev ]} | |
--prefix LD_LIBRARY_PATH : ${lib.makeLibraryPath [ libpulseaudio udev ]} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually had a problem where Nix would, while it was shrinking the RPATH, remove the library paths for libpulseaudio
and udev
, and thus the instance would crash upon startup. I had discussed this in the IRC and was confused on why they got removed, and couldn't figure out why Nix wanted to remove those paths.
Using a wrapper to export LD_LIBRARY_PATH
before execution was the only solution I was given that worked, I couldn't think of another one (aside from putting the whole libPath
in that wrapper instead).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the output when I build and print the RPATH of the ELF.
28a77c8
to
6887026
Compare
6887026
to
00d67e5
Compare
@worldofpeace how do we handle top-level name conflicts. I feel like this should be called todoist, but there's already a go application by that name. |
I'm just going by how it's called in the AUR, I think it should be kept that way. |
I think todo-electron seems to be the most appropriate: https://repology.org/projects/?search=todoist&maintainer=&category=&inrepo=¬inrepo=&repos=&families=&repos_newest=&families_newest= |
You mean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commits LGTM
gui starts fine
[4 built, 0.0 MiB DL]
https://github.com/NixOS/nixpkgs/pull/79167
1 package built:
todoist-electron
todoist-electron
is an Electron wrapper around the Todoist web app, with some global shortcuts.Note: The source repository does not have a
COPYING
file to denote a license, so I am basing the license off of the AUR Package (ISC license), maintained by the owner of the source repo. Additionally, while the repo's name istodoist-linux
, the AUR package name istodoist-electron
, so I'm going with the latter since it's slightly more descriptive.(this is also my first PR and first derivation for nixpkgs, so I may have gotten some things wrong)
Motivation for this change
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)