Plea for mercurial_keyring to be pulled into tree
Marcin.Kasperski at mekk.waw.pl
Sat Sep 3 16:31:24 CDT 2011
(If I missed some parts of this discussion, please let me know, I am
just recovering after month out of the net)
My notes regarding possible merge of mercurial keyring into mercurial:
1) As mercurial patching is considered, I would strongly consider
refactoring mercurial code so the extension is less invasive and
brings less code duplication. The worst example is current
keyring_supported_smtp function which is a (not very up to date) copy
of a (long) _smtp function from mercurial/mail.py used just to add a
few lines in the middle. password_mgr.find_user_password also brings
some code duplication.
The solution to this problem would be factoring out the very logic of
prompting for missing credentials to separate function(s), which would
be given the information which credentials are missing and what parts
of repository context could be read in the "standard way" (so if there
is username in url or hgrc, it would be already known). By default it
would just be combination of self.ui.write, self.ui.prompt and
self.ui.getpass, but it could be overwritten by mercurial_keyring. If
it could be plugged in instead of being monkepatched - the better.
I imagine other extensions could also benefit here (for example it
would make it easy to substitute GUI prompt for credentials instead of
2) Current implementation does not handle all password cases (in
particular I have ticket about providing http proxy password). Here
also, I think refactoring mercurial would be a better way to introduce
it than more monkeypatching.
3) The keyring library as such is unpleasant dependency. It is tough
to configure (especially the logic "which keyring to use" looks ugly),
it has also some binary deps which are not always trivial. I am not
sure whether those are important obstacles for mercurial, but people
may expect support from us here.
4) There is currently in fact no way to manage saved passwords. In
theory standard tools like seahorse or kwallet could do, but keyring
library in current state fails to label passwords properly and we end
up with a blob of undistinguishable passwords. I thought about
extending keyring library to help here, but supporting all keyring
backends is not possible for me (I have no Mac or Win machines in
particular). An alternative could be to provide a mercurial subcommand
for a taks, but this would need to be implemented.
More information about the Mercurial-devel