biox / pa

a simple password manager. encryption via age, written in portable posix shell
https://passwordass.org
Other
506 stars 21 forks source link

Add support for `pa git <subcommand>` #43

Closed Sjlver closed 1 month ago

Sjlver commented 1 month ago

I would like a built-in version of this:

pa() {
    case $1 in
        g*)
            cd "${PA_DIR:=${XDG_DATA_HOME:=$HOME/.local/share}/pa/passwords}"
            shift
            git "$@"
        ;;

        *)
            command pa "$@"
        ;;
    esac
}

I feel that an alias for this no longer makes sense, now that pa has a built-in git commit. Users should really be able to git push without having to care about pa internals, such as the location of the passwords folder. I think this is a core feature of a password manager, because backing up the passwords (via git push) is critical.

In my opinion, I would prefer plain git subcommands over a "git auto-push" feature. git subcommands cover 100% of my needs, whereas something like autopush would only solve part of the problem. Also, trying to do a git push might trigger an SSH passphrase prompt, might fail if the computer is offline, etc. Just let the user run "pa git push" at their convenience.

Initial discussion in https://github.com/biox/pa/pull/41#issuecomment-2369050602

arcxio commented 1 month ago

git subcommands cover 100% of my needs, whereas something like autopush would only solve part of the problem

I imagine to back up password you'd only need git push, git pull and that's it. do you use pa g for something else and what are those use cases if you do?

Sjlver commented 1 month ago

I notice that I use git subcommands quite frequently with pass. For example: git log to find out when I added a password.

Given that I care about git log being meaningful, I also occasionally run things like git reset HEAD^ to undo a commit, for example if I made a mistake when adding a password.

I might need git add in a few cases too, like when renaming a password file.

Message ID: @.***>

arcxio commented 1 month ago

git log to find out when I added a password

this makes sense. I thought we could add a script that would sync local password store and the remote, essentially just doing pull and push with a single command, but yeah it's useful to run git directly too. I understand the argument for it being a built-in feature, but I also don't like adding a new command just to pass it onto git, I feel like it just doesn't sit right with pa's design philosophy.

one idea I have is trying pa git the other way around. leveraging git's extension system you can do:

$ git config --global alias.pa '!git -C "${PA_DIR:-${XDG_DATA_HOME:-$HOME/.local/share}/pa/pa
sswords}"'

then you'd use git pa to achieve the same effect of pa git.

alternatively, to streamline this approach a bit, we could have contrib/git-pa with the same command, then you'd only need to copy it to your $PATH to use git pa. it still wouldn't be built-in, but I couldn't come up with a simpler UX without adding a new command. but this has a problem of a potential conflict with already existing alias pa in user's gitconfig.

Sjlver commented 1 month ago

These arguments are okay; I think I see where you come from and that you want to keep pa as small as possible.

The crux is: you've already added support for git, It increased pa's code size by about 10%, and even added another environment variable (despite pa wanting to have as few options as possible). As it is currently, this git support feels incomplete to me; pa git would complete it.

One could argue that the current git support could also be replaced by aliases and wrapper scripts. Something as simple as pa() { command pa "$@" && (cd "${PA_DIR:=${XDG_DATA_HOME:=$HOME/.local/share}/pa/passwords}"; git commit -am "$@" 2>/dev/null) } might work.

So, the boundary of what belongs into pa and what should be outside is... difficult to define. I'll stop here since I think I've said all my arguments. It's up to you as maintainers now :)

biox commented 1 month ago

i think i'm in favor of adding pa git as you've described @Sjlver. it doesn't sit well with the total minimalism, but git is just one of those tools that we can't really meaningfully abstract, and i suspect that many pa users would appreciate having a predefined "wrapper script" for running git in the pa passwords repo. this would also help prevent user mistakes like pushing identity files.

so i guess i'm in favor of a light wrapper, provided it really is just basically cd $PA_DIR && git <user_input>. i would personally use it, which probably means it's valuable. :D

a case of bloat, maybe, but i think git is ubiquitous enough that it's worth trading some LOC for.

@arcxio - if we do add pa git <arbitrary command> support, does that mean we should reconsider how auto-committing currently works?

arcxio commented 1 month ago

does that mean we should reconsider how auto-committing currently works?

I don't think so, why?