Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?

From

Noam Postavsky <npostavs@users.sourceforge.net>

Date

Nov 10, 2015, 01:05 UTC

Message-ID

<CAM-tV--hBSdCJckCnMtKgkQB2f_3eN8sXHdFWwg2hzb6s7ufxw@mail.gmail.com>

In-Reply-To

<20151109155342.GB27224@sigill.intra.peff.net>

On Mon, Nov 9, 2015 at 10:53 AM, Jeff King <peff@peff.net> wrote:

> Yes, but with a proper commit message, and an update to
> Documentation/config.txt. :)

Right, see attached.

>
> Automated tests would be nice, but I suspect it may be too complicated
> to be worth it.

I attempted

test_ignore_sighup ()
{
    mkdir "$HOME/.git-credential-cache" &&
    chmod 0700 "$HOME/.git-credential-cache" &&
    git -c credentialCache.ignoreSIGHUP=true credential-cache--daemon
"$HOME/.git-credential-cache/socket" &
    kill -SIGHUP $! &&
    ps $!
}

test_expect_success 'credentialCache.ignoreSIGHUP works' 'test_ignore_sighup'

>
> I don't think we should use the credential.X.* namespace here. That is
> already reserved for credential setup for URLs matching "X".
>
> Probably "credentialCache.ignoreSIGHUP" would be better. Or maybe
> "credential-cache". We usually avoid dashes in our config names, but
> in this case it matches the program name.

I went with "credentialCache.ignoreSIGHUP".

>
> Also, we usually spell config names as all-lowercase in the code. The
> older callback-interface config code needed this (since we just strcmp'd
> the keys against a normalized case). I think git_config_get_bool() will
> normalize the key we feed it, but I'd rather stay consistent.

Oh, I didn't even realize git config names were case insensitive.

> I don't think you need to pop the tempfile handler here. You can simply
> sigchain_push() the SIG_IGN, and since we won't ever pop and propagate
> that, it doesn't matter what is under it.

Yup.

From 5fc95b6e2f956403da6845fc3ced83b21bee7bb0 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@users.sourceforge.net>
Date: Mon, 9 Nov 2015 19:26:29 -0500
Subject: [PATCH] credential-cache: new option to ignore sighup

Introduce new option "credentialCache.ignoreSIGHUP" which stops
git-credential-cache--daemon from quitting on SIGHUP.  This is useful
when "git push" is started from Emacs, because all child
processes (including the daemon) will receive a SIGHUP when "git push"
exits.
---
 Documentation/config.txt   | 3 +++
 credential-cache--daemon.c | 7 +++++++
 2 files changed, 10 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 4d3cb10..4444e5b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1122,6 +1122,9 @@ credential.<url>.*::
 	example.com. See linkgit:gitcredentials[7] for details on how URLs are
 	matched.
 
+credentialCache.ignoreSIGHUP::
+	Tell git-credential-cache--daemon to ignore SIGHUP, instead of quitting.
+
 include::diff-config.txt[]
 
 difftool.<tool>.path::
diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c
index eef6fce..6cda9c0 100644
--- a/credential-cache--daemon.c
+++ b/credential-cache--daemon.c
@@ -256,6 +256,9 @@ int main(int argc, const char **argv)
 		OPT_END()
 	};
 
+	int ignore_sighup = 0;
+	git_config_get_bool("credentialcache.ignoresighup", &ignore_sighup);
+
 	argc = parse_options(argc, argv, NULL, options, usage, 0);
 	socket_path = argv[0];
 
@@ -264,6 +267,10 @@ int main(int argc, const char **argv)
 
 	check_socket_directory(socket_path);
 	register_tempfile(&socket_file, socket_path);
+
+	if (ignore_sighup)
+		signal(SIGHUP, SIG_IGN);
+
 	serve_cache(socket_path, debug);
 	delete_tempfile(&socket_file);
 

-- 
2.6.1