git-credential-keepassxc icon indicating copy to clipboard operation
git-credential-keepassxc copied to clipboard

Add FreeBSD support.

Open lapo-luchini opened this issue 2 years ago • 3 comments

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

  • [x] 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.

lapo-luchini avatar Jun 07 '23 14:06 lapo-luchini

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")
    

Frederick888 avatar Jun 08 '23 09:06 Frederick888

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).

raphaelahrens avatar Jun 09 '23 04:06 raphaelahrens

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.

Frederick888 avatar Jun 13 '23 14:06 Frederick888