pop-launcher: init at 1.2.1 #176303
pop-launcher: init at 1.2.1 #176303
Conversation
|
I tried building it and running it, but unfortunately I couldn't test it with pop-shell. But it builds and starts just fine at least. |
|
Working very well here, thank you! |
|
Works great, but why are you applying a patch? substituteInPlace src/lib.rs \
--replace '/usr/lib/pop-launcher' "$out/share/pop-launcher"
substituteInPlace plugins/src/scripts/mod.rs \
--replace '/usr/lib/pop-launcher' "$out/share/pop-launcher"As far as I know, the DISTRIBUTION constants used, are the locations where pop-launcher expects plugins and scripts within a distribution, and since we use nix as the packagemanager, these should be set to nix specific locations instead, hence |
Hi @Steav005, thanks for testing I left the DISTRIBUTION constants as is so that the package would still find plugins/scripts installed in those paths. I thought that might be useful on a non-NixOS system or if someone wanted to install additional plugins/scripts using a NixOS module |
I understand, but for additional systemwide and per-user plugins/scripts other locations are used by pop-launcher For non-NixOS systems, this may help, but again, there are other locations were additions plugins/scripts are supposed to be placed. For additional NixOS modules providing plugins/scripts On that note, maybe you also want to consider, changing the plugin/script path from These are just my two cents, I dont have any say in here |
That seems reasonable to me - I'll update the PR later today |
|
@ofborg eval |
| nativeBuildInputs = [ just ]; | ||
|
|
||
| buildPhase = '' | ||
| just |
There was a problem hiding this comment.
Does just support -j (jobs) or a similar flag like make does?
There was a problem hiding this comment.
It doesn't appear to. But it also isn't really meant as a replacement for a build tool, since it's more of a command runner.
There was a problem hiding this comment.
I think that is just invoking cargo commands, right? then we should use the nixpkgs tooling for cargo rather than just.
There was a problem hiding this comment.
@SuperSandro2000 it does a bit more than that, and who knows what it might do in the future. However I do agree that as it is now, replacing it for just the nixpkgs handling of cargo with some additional postBuildPhase script might be the ideal and more straightforward way of doing this yes.
There was a problem hiding this comment.
@SuperSandro2000 I changed it to use the default build phase with a postInstall instead of overriding with the upstream build script
|
Thanks for this. I did a quick test with b21aca4, the launcher works great! One issue I noticed, two of the built-in plugins calculator and file search are not working due to missing packages |
I had left There is also https://github.com/pop-os/launcher/blob/master/plugins/src/terminal/mod.rs#L127 which assumes either |
We don't create a
Either we point those paths directly to the binaries or wrap the exectuable to add them to PATH. |
|
I updated this to include |
|
I removed the shebang patch for |
|
Is there any chances of it getting merged? |
|
This builds and runs. I didn't test if pop-shell can discover it, but that'd probably be a fixup in pop-shell anyways, not here. Let's get this in! |
Description of changes
Add package for pop-launcher https://github.com/pop-os/launcher
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes