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

feat: optionally use POP_LAUNCHER_LIB_DIR env variable #242

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ryanabx
Copy link
Contributor

@ryanabx ryanabx commented Sep 29, 2024

This environment variable describes where the pop-launcher distribution path exists. The default for this is /usr/lib/pop-launcher; however, some distributions would like to use /usr/libexec/pop-launcher instead.

ryanabx added a commit to ryanabx-contrib/cosmic-launcher that referenced this pull request Sep 29, 2024
mmstick
mmstick previously approved these changes Sep 30, 2024
@mmstick mmstick requested a review from a team September 30, 2024 18:11
@ryanabx
Copy link
Contributor Author

ryanabx commented Sep 30, 2024

Note that we will also need to update cosmic-launcher's Cargo.lock to account for the changes here, I think

@mmstick
Copy link
Member

mmstick commented Sep 30, 2024

Shouldn't be necessary since it (cosmic-launcher) communicates to the pop-launcher sub-process via IPC over stdin/stdout.

@ryanabx
Copy link
Contributor Author

ryanabx commented Sep 30, 2024

Shouldn't be necessary since it (cosmic-launcher) communicates to the pop-launcher sub-process via IPC over stdin/stdout.

Oh I didn't know, thanks!

jacobgkau
jacobgkau previously approved these changes Oct 1, 2024
Copy link
Member

@jacobgkau jacobgkau left a comment

Choose a reason for hiding this comment

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

This moves the system-wide pop-launcher/{plugins,scripts} directories from /usr/lib/ to /usr/libexec/. There's no change in behavior.

Regression testing passed:

Launcher

  • All windows on all workspaces appear on launch
  • Choosing an app on another workspace moves workspaces and focus to that app
  • Launching an application works
  • Typing text and then removing it will re-show those windows
  • Search works for applications and windows
  • Search works for GNOME settings panels
  • Search for "Extensions". There should be only one entry.
  • The overlay hint correctly highlights the selected window
  • Open windows are sorted above applications (e.g. "firefox")
  • t: executes a command in a terminal
  • : executes a command in sh
  • = calculates an equation
  • Search results are as expected:
    • cal returns Calendar and Calculator before Color
    • pops returns Popsicle first
    • shop returns the Pop!_Shop first

Copy link
Member

@jackpot51 jackpot51 left a comment

Choose a reason for hiding this comment

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

I believe both Arch and NixOS have issues with /usr/libexec. The best thing to do is make it configurable.

@ryanabx ryanabx dismissed stale reviews from jacobgkau and mmstick via ef33488 October 28, 2024 03:05
@ryanabx
Copy link
Contributor Author

ryanabx commented Oct 28, 2024

@jackpot51 how's this?

@ryanabx ryanabx changed the title fix: use /usr/libexec/.. for plugins instead of /usr/lib/.. feat: introduce POP_LAUNCHER_PLUGIN_DIR to configure plugin directory Oct 28, 2024
@ryanabx ryanabx force-pushed the fix/libexec branch 2 times, most recently from e4dc30b to 2f2e927 Compare October 28, 2024 04:25
@ryanabx ryanabx changed the title feat: introduce POP_LAUNCHER_PLUGIN_DIR to configure plugin directory feat: optionally use POP_LAUNCHER_LIB_DIR env variable Oct 28, 2024
This environment variable describes where the pop-launcher distribution path exists. The default for this is `/usr/lib/pop-launcher`; however, some distributions would like to use `/usr/libexec/pop-launcher` instead.

Signed-off-by: Ryan Brue <[email protected]>
@Conan-Kudo
Copy link

/usr/libexec should be the default since that is the recommended place for the majority of distributions (Fedora, openSUSE, Debian/Ubuntu, etc.) and distributions who want an alternative location can set it otherwise.

@wiiznokes
Copy link
Contributor

I wonder how the POP_LAUNCHER_LIB_DIR even works since it is not exported in a build.rs file.

Except that, i have some suggestion/question

  • why all the paths have been duplicated in the code ?
  • could we change option_env! to env!, and declare the POP_LAUNCHER_LIB_DIR variable in the justfile. This will make things a bit less magic, and easier for packaging and development i think, because we will just need to call change lib-path in the Makefile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants