Michal Nazarewicz <min...@mina86.com> writes:
> When git-send-email uses git-credential to get SMTP password, it will
> communicate SMTP host and port (if both are provided) as a single entry
> ‘host=<host>:<port>’. This trips the ‘git-credential-store’ helper
> which expects those values as separate keys (‘host’ and ‘port’).
>
> Send the values as separate pieces of information so things work
> smoothly.
>
> Signed-off-by: Michał Nazarewicz <min...@mina86.com>
> ---
> git-send-email.perl | 11 ++---------
> 1 file changed, 2 insertions(+), 9 deletions(-)
"git help credential" mentions protocol, host, path, username and
password (and also url which is a short-hand for setting protocol
and host), but not "port". And common sense tells me, when a system
allows setting host but not port, that it would expect host:port to
be given when the service is running a non-standard port, so from
that point of view, I suspect that the current code is working as
expected. In fact, credential.h, which defines the API, does not
have any "port" field, either, so I am not sure how this is expected
to change anything without touching the back-end that talks over the
pipe via _credential_run-->credential_write callchain.
Now, it is a separate matter if it were a better design if the
credential API had 'host' and 'port' defined as separate keys to the
authentication information. Such an alternative design would have
made certain things harder while some other things easier (e.g. "use
this credential to the host no matter what port the service runs"
may be easier to implement if 'host' and 'port' are separate).
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 2fa7818ca..2a9f89a58 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1229,14 +1229,6 @@ sub maildomain {
> return maildomain_net() || maildomain_mta() || 'localhost.localdomain';
> }
>
> -sub smtp_host_string {
> - if (defined $smtp_server_port) {
> - return "$smtp_server:$smtp_server_port";
> - } else {
> - return $smtp_server;
> - }
> -}
> -
> # Returns 1 if authentication succeeded or was not necessary
> # (smtp_user was not specified), and 0 otherwise.
>
> @@ -1263,7 +1255,8 @@ sub smtp_auth_maybe {
> # reject credentials.
> $auth = Git::credential({
> 'protocol' => 'smtp',
> - 'host' => smtp_host_string(),
> + 'host' => $smtp_server,
> + 'port' => $smtp_server_port,
> 'username' => $smtp_authuser,
> # if there's no password, "git credential fill" will
> # give us one, otherwise it'll just pass this one.
