Using hg-git version 1.0b1-release (9967db0e26f7), when I push to a remote that requires authentication, the push succeeds using my credentials from mercurial-keyringthen Mercurial asks me for my password. If I hit Ctrl-C at the password prompt, Mercurial exits and my pushed commits are present on the remote.
Expected behaviour: the operation should succeed without asking for my password. This used to work with hg-git version 0.10.3.
Thanks for the bug report — I've been thinking about what to do about this…
Some context: The extra operation occurs in order to determine what the remote head is, for possible publishing. Unfortunately, extensions cannot actually detect whether an HTTP operation succeeded, so they simply assume that any duplicate password prompts represent an error…
So, a bit of context: This was caused by !31 (merged) which fixed publishing the remote HEAD on push, by issuing another request to the server to determine it. Dulwich does not tell us the remote symbolic references on push, so if the user pushes to e.g. master, and remote HEAD points to master, we don't know that until the next pull. As a result, pushing master wouldn't actually publish it until the next pull. Internally, hg-git tries its best not to save passwords and credentials — conceptually, it shouldn't — so we just try every request, and if it fails, we prompt for password and try again.
Apart from the aforementioned issues, this is also complicated by password handling within Mercurial being really weird. Specifically, the extensions that help you save passwords — such as mercurial_keyring and hg-credentials — don't actually know whether a given password is valid or not. Mercurial does know that, internally, but essentially disregards this and just prompts for another password. As a result, the extensions must assume that any subsequent requests to the same URI constitute a failed authentication, and prompt for a password.
(As an aside, I may be able to fix this for the hg-credentials by relying on the fact that only failed authentication provides a realm. mercurial_keyring is much more “clever” though, and actually remembers what was prompted for.)
So, a few solutions I can think of:
Back out 5f1536c9a387, and accept that we cannot publish the remote HEAD on push.
Apply a partial check, and only expose the bug when using phases.
Refactor our client handling to save it in the GitHandler instance.
Fix this, somehow, in Dulwich. If we can know the symrefs on push, the solution is much easier.
Use Mercurial's HTTP infrastructure, which is at least possible from Dulwich 0.20.24 and onwards. That might not fix this, but it'd probably help.
Fix password handling in Mercurial core to mostly resemble the git-credential “protocol”.
Of these, I'd consider the two first acceptable for 1.0.0; the remainder are much too invasive for post-beta. What are you thoughts, @gracinet?
First reaction thoughts without knowing much about the internals:
This is removal of functionality (even if new in 1.0.0) and should be only last-resort.
👍 Should probably be done no matter what, and would protect long-time users that don't even know about the new phases stuff (yet), and in particular scripted use cases if any
Do you mean saving the password as an attribute of GitHandler? Is there a promise that there will be a single GitHandler for the whole process?
Wouldn't it be possible to query the remote HEAD first (using the password) then reuse the password immediately?
Sounds like something for 1.1 maybe?
I don't know about the git-credential on top of my head, but it looks like some short-term persistence would be useful. The problem here is that in general, one user action results into several requests on the wire protocol, and I'm not sure if there is a sound way to group them logically (especially if chg comes into play)
Overall, if you consider 3 to be too dangerous for post-beta (even with a b2 version?), then I'd go for 2, especially if that's enough for the original poster.
@jmb would that fix your use case? In other words, do you also (plan to) have hggit.usephases=yes?
Just a bit of background: I found an easier fix where we simply cache the Dulwich client, and assume that we always issue the most privileged operation first. Which we do.