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

Add FreeBSD support. #81

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

Conversation

lapo-luchini
Copy link

Description

Using on FreeBSD fails, as there is no specific case that matches.
It works just like on Linux, on path $XDG_RUNTIME_DIR/org.keepassxc.KeePassXC.BrowserServer.

Changes

Checklist

  • I have rebased my branch so that it has no conflicts
  • I have added tests where appropriate
  • Commit messages are compliant with Conventional Commits

Is this a breaking change?

No, it only adds a new case for FreeBSD.

Just like Linux, it uses $XDG_RUNTIME_DIR/org.keepassxc.KeePassXC.BrowserServer
@Frederick888
Copy link
Owner

Thanks for the contribution!

Are you sure the socket you are using is not a symlink to a different
path? I'm asking since KeePassXC does not seem to have a different
set-up for BSD.

If this is the case, we probably don't need a new SocketPath for BSD.
Instead, we can rename the Linux* ones to Unix*, then change their
matches_os to cfg!(target_family = "unix") && !cfg!(target_os = "macos")?

If it turns out that things are indeed different under BSD, then

  1. Could you check if it was different prior to KPXC 2.7.4 under BSD?
    What was it? (The number suffixes indicate when the paths were
    introduced.)
  2. Since it's the same as LinuxSocketPath260, you should be able to do:
    diff --git a/src/utils/socket.rs b/src/utils/socket.rs
    index 9e2d491..67a9203 100644
    --- a/src/utils/socket.rs
    +++ b/src/utils/socket.rs
    @@ -53,15 +53,9 @@ trait SocketPath {
    
     struct FreeBSDSocketPath274;
     impl SocketPath for FreeBSDSocketPath274 {
         fn get_path(&self) -> Result<PathBuf> {
    -        let base_dirs = directories_next::BaseDirs::new()
    -            .ok_or_else(|| anyhow!("Failed to initialise base_dirs"))?;
    -        let result = base_dirs
    -            .runtime_dir()
    -            .ok_or_else(|| anyhow!("Failed to locate runtime_dir automatically"))?
    -            .join(KEEPASS_SOCKET_NAME);
    -        exist_or_error(result)
    +        LinuxSocketPath260.get_path()
         }
    
         fn matches_os(&self) -> bool {
             cfg!(target_os = "freebsd")

@raphaelahrens
Copy link
Contributor

One difference I found is that on my FreeBSD system XDG_RUNTIME_DIR is not set by default and KeePassXC defaults to /tmp/runtime-$USER.
But the crate directories_next does find an empty env var and returns None for directories_next::BaseDirs::new()?.runtime_dir().
Which lets git-credentials-keepassxc error with

ERRO Failed to locate socket, Caused by: N/A

I was not able to figure out, who sets XDG_RUNTIME_DIR on my Linux machine, but it looks like that it gets set by default by systemd (maybe).

@Frederick888
Copy link
Owner

Frederick888 commented Jun 13, 2023

One difference I found is that on my FreeBSD system XDG_RUNTIME_DIR is not set by default. [...] I was not able to figure out, who sets XDG_RUNTIME_DIR on my Linux machine, but it looks like that it gets set by default by systemd (maybe).

Yes it's set by pam_systemd usually: https://man7.org/linux/man-pages/man8/pam_systemd.8.html.

But XDG is in theory a specification that any desktop can implement. Under Linux nowadays it's mostly systemd, however for example I believe OpenRC users are also able to set it up.

But the crate directories_next does find an empty env var and returns None for directories_next::BaseDirs::new()?.runtime_dir().

As mentioned XDG is a specification, so maybe there are also implementations under FreeBSD, and that's probably the case here? Since it apparently worked for @lapo-luchini.

So IIUC, what we should do here is probably:

  1. Update the existing Linux* socket locators to Unix* + cfg!(target_family = "unix") && !cfg!(target_os = "macos") (although directories-next doesn't support *BSD officially; and yeah I know macOS is also Unix, but I don't have a better name in mind)
  2. Add a new FreeBsdSocketPath to check /tmp/runtime-$USER

By the way I made some changes to the Actions, would be nice if you can rebase to test them out.

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.

None yet

3 participants