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 support for XDG_DATA_HOME #95

Closed
wants to merge 2 commits into from
Closed

Add support for XDG_DATA_HOME #95

wants to merge 2 commits into from

Conversation

apiraino
Copy link

Hi,

this is a tentative small PR to add support for the Freedesktop standard XDG_DATA_HOME dir, when the browerpass-native looks for the password-store files.

There is an ongoing discussion on this topic in the password-store mailing list that hopefully will land to a new release at some point.

I tried reproducing the logic in this message, aimed to be fully retrocompatible with existing installations:

	If PASSWORD_STORE_DIR is set use that
	else if ~/.password_store exists use that
	else fallback to the XDG dir behaviour

If the logic and intent of this pr are sound I can submit another small patch to align browserpass-extension.

I'm open to feedbacks and suggestions as I might have missed some bits.

Thanks for the review!

Copy link
Member

@maximbaz maximbaz left a comment

Choose a reason for hiding this comment

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

Thanks for the PR 🙂 The logic make sense to me. Shall we park it until the change is merged into password-store?

@apiraino
Copy link
Author

apiraino commented May 24, 2020

I should have addressed your two suggestions!

I've also added a test (maybe not really needed but it helped me in debugging)

@apiraino
Copy link
Author

Thanks for the PR slightly_smiling_face The logic make sense to me. Shall we park it until the change is merged into password-store?

About parking it: I'd personally merge it now as it should be harmless and future-proof at the same time (and I don't have to rebase future conflicts 😄 ), but feel free to do as you prefer, it's ok for me anyway.

Also I wouldn't hold my breath for a quick new release of pass.

@maximbaz
Copy link
Member

Thanks :) To clarify, I don't want to wait for a whole new release of pass, just to see the code merged in pass git repo (I understood that currently it's still at the proposal stage?). I simply want to avoid the case when during the discussion there might be other suggestions, and the logic in this PR will no longer match the behavior of pass...

@apiraino
Copy link
Author

ah yes very good point 👍 I will keep an eye on the discussion and be back at some point when/if it settles

@maximbaz
Copy link
Member

maximbaz commented Aug 3, 2021

Hi @apiraino, just out of curiosity as I wasn't following the topic myself, do you know if there was any traction on this in the pass-git? Doesn't seem so if I click on the mailing list thread you linked.

No rush, just thought if this was decided, we can merge, and if pass-git still doesn't respect XDG_DATA_HOME then we wait some more 👍

@apiraino
Copy link
Author

apiraino commented Sep 4, 2021

hi @maximbaz apologies for the slow reply and for not reporting back. The topic of Freedesktop compliancy was downvoted by the maintainer (for the record, here is the relevant message).

Those wishing to comply with the XDG spec can work around this limitation of pass with:

export PASSWORD_STORE_DIR=$XDG_DATA_HOME/password-store

and browerpass will work just fine. I don't need this PR either :) At your option to close it, perhaps waiting doesn't make much more sense anymore.

@maximbaz
Copy link
Member

maximbaz commented Sep 7, 2021

I see! Thanks for the update! In that case lets close this, and revisit when relevant.

@maximbaz maximbaz closed this Sep 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants