{"thread":{"id":"40529","subject":"git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","startedAt":"2015-10-10T16:45:03Z","lastAt":"2015-12-04T23:25:46Z","messageCount":29,"participants":["Noam Postavsky","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"271399","messageId":"CAM-tV-_JPazYxeDYogtQTRfBxONpSZwb3u5pPanB=F9XnLnZyg@mail.gmail.com","threadId":"40529","inReplyTo":null,"subject":"git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Noam Postavsky","fromEmail":"npostavs@users.sourceforge.net","sentAt":"2015-10-10T16:45:03Z","receivedAt":"2015-10-10T16:45:03Z","isPatch":false,"sender":{"key":"npostavs@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/287742?v=4"},"body":"I noticed that git-credential-cache--daemon quits on SIGHUP. This\nseems like surprising behaviour for a daemon. Would it be acceptable\nto change it to ignore SIGHUP?\n\n(This came up while investigating a Magit bug[1], we are also\nconsidering ways to avoid sending a SIGHUP in the first place)\n\n[1]: https://github.com/magit/magit/issues/2309\n"},{"id":"271875","messageId":"CAM-tV-_eOgnhqsTFN6kKW=tcS7gAPYaxskBaxnJZo3bsx02HZg@mail.gmail.com","threadId":"40529","inReplyTo":"CAM-tV-_JPazYxeDYogtQTRfBxONpSZwb3u5pPanB=F9XnLnZyg@mail.gmail.com","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Noam Postavsky","fromEmail":"npostavs@users.sourceforge.net","sentAt":"2015-10-18T15:15:59Z","receivedAt":"2015-10-18T15:15:59Z","isPatch":false,"sender":{"key":"npostavs@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/287742?v=4"},"body":"On Sat, Oct 10, 2015 at 12:45 PM, Noam Postavsky\n wrote:\n> I noticed that git-credential-cache--daemon quits on SIGHUP. This\n> seems like surprising behaviour for a daemon. Would it be acceptable\n> to change it to ignore SIGHUP?\n\nping?\n"},{"id":"271881","messageId":"xmqqfv18awj4.fsf@gitster.mtv.corp.google.com","threadId":"40529","inReplyTo":"CAM-tV-_eOgnhqsTFN6kKW=tcS7gAPYaxskBaxnJZo3bsx02HZg@mail.gmail.com","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-18T17:58:39Z","receivedAt":"2015-10-18T17:58:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Noam Postavsky writes:\n\n> On Sat, Oct 10, 2015 at 12:45 PM, Noam Postavsky\n> wrote:\n>> I noticed that git-credential-cache--daemon quits on SIGHUP. This\n>> seems like surprising behaviour for a daemon. Would it be acceptable\n>> to change it to ignore SIGHUP?\n>\n> ping?\n\nThanks for pinging. I guess this either fell in the cracks while\npeople were busy discussing other topics, or nobody agreed with the\nreasoning behind the change, or perhaps a bit of both. In any case,\nit is a prodent thing to ping on the thread after a week or so,\nwhich is what you did. Very much appreciated.\n\nI cannot speak for the person who was primarily responsible for\ndesigning this behaviour, but I happen to agree with the current\nbehaviour in the situation where it was designed to be used. Upon\nthe first use in your session, the \"daemon\" is auto-spawned, you can\nkeep talking with that same instance during your session, and you do\nnot have to do anything special to shut it down when you log out.\nIsn't that what happens here?\n\nIf this were \"when you start the system you start this free-standing\ndaemon once, and it will stay around until it gets shut down. If you\nare staying around in logged-in state is immaterial\" kind of daemon,\nI'd expect it, upon being killed with HUP, to do something useful,\nlike re-reading its configuration file, and continue, instead of\ndying.\n\nPerhaps you can tweak the system to get both, by making it continue\nupon HUP by default, but teaching it an option not to (i.e. the\ncurrent behaviour). Pass that option when spawn_daemon() in\ncredential-cache.c starts the daemon. When using the daemon as a\nfree-standing one (against the way its documentation expects you\nto---see \"git help credential-cache--daemon\"), you do not pass that\noption and your \"daemon\" will ignore HUP.\n\nHmm?\n"},{"id":"271886","messageId":"CAM-tV-8BQPhGXe0nNFHEWPV_SnoOiCd9OxSiSYk22f86F4KtFA@mail.gmail.com","threadId":"40529","inReplyTo":"xmqqfv18awj4.fsf@gitster.mtv.corp.google.com","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Noam Postavsky","fromEmail":"npostavs@users.sourceforge.net","sentAt":"2015-10-19T00:51:19Z","receivedAt":"2015-10-19T00:51:19Z","isPatch":false,"sender":{"key":"npostavs@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/287742?v=4"},"body":"On Sun, Oct 18, 2015 at 1:58 PM, Junio C Hamano wrote:\n> I cannot speak for the person who was primarily responsible for\n> designing this behaviour, but I happen to agree with the current\n> behaviour in the situation where it was designed to be used. Upon\n> the first use in your session, the \"daemon\" is auto-spawned, you can\n> keep talking with that same instance during your session, and you do\n> not have to do anything special to shut it down when you log out.\n\nOh, hmm this does makes sense. Now that I think of the log out case, I\nthink ignoring the SIGHUP would be the wrong thing for us too.\n\n> Isn't that what happens here?\n\nI think our problem is that when Emacs creates the \"git push\" process\nwith a pty[1], it somehow puts that process in its own new session, so\nwhen \"git push\" finishes it takes the daemon down with it. But seeing\nit like this, it seems clear that the problem is from the Emacs side.\n\n[1]: and using pipes instead of a pty doesn't allow entering the\npassword: https://github.com/magit/magit/issues/2309#issuecomment-147101903\n"},{"id":"272016","messageId":"CAM-tV-8VXtB5uRgqP9dFpww6AaLzasPV46tCiquz=nz=ksBNng@mail.gmail.com","threadId":"40529","inReplyTo":"xmqqfv18awj4.fsf@gitster.mtv.corp.google.com","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Noam Postavsky","fromEmail":"npostavs@users.sourceforge.net","sentAt":"2015-10-21T02:35:46Z","receivedAt":"2015-10-21T02:35:46Z","isPatch":false,"sender":{"key":"npostavs@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/287742?v=4"},"body":"On Sun, Oct 18, 2015 at 1:58 PM, Junio C Hamano wrote:\n> I cannot speak for the person who was primarily responsible for\n> designing this behaviour, but I happen to agree with the current\n> behaviour in the situation where it was designed to be used. Upon\n> the first use in your session, the \"daemon\" is auto-spawned, you can\n> keep talking with that same instance during your session, and you do\n> not have to do anything special to shut it down when you log out.\n> Isn't that what happens here?\n\nAfter looking at this some more, I've discovered this is NOT what\nactually happens here. If I \"git push\" from a shell and then log out\nand log in again, another \"git push\" does NOT ask me for a password.\nIn other words, the daemon is NOT shut down automatically when I log\nout. Given that, does it make sense to change the daemon to ignore\nSIGHUP, or is there some way to change it so that it does exit on\nlogout?\n\n---------------------\n\nI don't understand why this happens, but the attached self-contained\npair of programs demonstrate the behaviour:\n\nIf I do\n\n make call-note-sighup note-sighup\n urxvt -e ./call-note-sighup ; cat note-sighup\n\nsighup.log DOES contain \"got sighup\", but if I instead do\n\n make call-note-sighup note-sighup\n ./call-note-sighup ; exit\n\nafterwards sighup.log does NOT contain \"got sighup\" (and I must do\n\"pkill note-sighup\" to get rid of the lingering note-sighup process).\n\n\n#include \n#include \n\nint main(void) {\n\n remove(\"sighup.log\");\n\n int pid = fork();\n if (pid < 0) {\n perror(\"fork\");\n return 1;\n } else if (pid == 0) {\n execl(\"./note-sighup\", \"./note-sighup\", NULL);\n perror(\"execl(./note-sighup)\");\n return 1;\n } else {\n fprintf(stderr, \"waiting for sigup.log to appear\\n\");\n while (!access(\"sighup.log\", F_OK)) {\n }\n fprintf(stderr, \"okay, note-sighup.log has arrived, exiting in 5 seconds...\\n\");\n sleep(5);\n }\n\n return 0;\n}\n\n\n#include \n#include \n#include \n#include \n\nstatic volatile int got_sighup = 0;\n\nvoid sighup_handler(int sig) {\n got_sighup = 1;\n}\n\nint main(void) {\n\n struct sigaction act;\n memset(&act, 0, sizeof act);\n act.sa_handler = sighup_handler;\n sigaction(SIGHUP, &act, NULL);\n\n FILE* out = fopen(\"sighup.log\", \"w\");\n setbuf(out, NULL);\n fprintf(out, \"starting note-sighup\\n\");\n\n for (;;) {\n sleep(1000);\n if (got_sighup) {\n got_sighup = 0;\n fprintf(out, \"got sighup\\n\");\n }\n }\n\n return 0;\n}\n"},{"id":"272178","messageId":"CAM-tV-9sNgHncsWRPh36tEY3YFORUJBA-Q6W5R=mvX_KhSmWEQ@mail.gmail.com","threadId":"40529","inReplyTo":"CAM-tV-8VXtB5uRgqP9dFpww6AaLzasPV46tCiquz=nz=ksBNng@mail.gmail.com","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Noam Postavsky","fromEmail":"npostavs@users.sourceforge.net","sentAt":"2015-10-24T21:47:03Z","receivedAt":"2015-10-24T21:47:03Z","isPatch":false,"sender":{"key":"npostavs@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/287742?v=4"},"body":"On Tue, Oct 20, 2015 at 10:35 PM, Noam Postavsky\n wrote:\n> On Sun, Oct 18, 2015 at 1:58 PM, Junio C Hamano wrote:\n>> I cannot speak for the person who was primarily responsible for\n>> designing this behaviour, but I happen to agree with the current\n>> behaviour in the situation where it was designed to be used. Upon\n>> the first use in your session, the \"daemon\" is auto-spawned, you can\n>> keep talking with that same instance during your session, and you do\n>> not have to do anything special to shut it down when you log out.\n>> Isn't that what happens here?\n>\n> After looking at this some more, I've discovered this is NOT what\n> actually happens here. If I \"git push\" from a shell and then log out\n> and log in again, another \"git push\" does NOT ask me for a password.\n> In other words, the daemon is NOT shut down automatically when I log\n> out. Given that, does it make sense to change the daemon to ignore\n> SIGHUP, or is there some way to change it so that it does exit on\n> logout?\n\nping?\n"},{"id":"272205","messageId":"xmqqfv0ylwa7.fsf@gitster.mtv.corp.google.com","threadId":"40529","inReplyTo":"CAM-tV-9sNgHncsWRPh36tEY3YFORUJBA-Q6W5R=mvX_KhSmWEQ@mail.gmail.com","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-25T16:58:56Z","receivedAt":"2015-10-25T16:58:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Noam Postavsky writes:\n\n> On Tue, Oct 20, 2015 at 10:35 PM, Noam Postavsky\n> wrote:\n>> On Sun, Oct 18, 2015 at 1:58 PM, Junio C Hamano wrote:\n>>> I cannot speak for the person who was primarily responsible for\n>>> designing this behaviour, but I happen to agree with the current\n>>> behaviour in the situation where it was designed to be used. Upon\n>>> the first use in your session, the \"daemon\" is auto-spawned, you can\n>>> keep talking with that same instance during your session, and you do\n>>> not have to do anything special to shut it down when you log out.\n>>> Isn't that what happens here?\n>>\n>> After looking at this some more, I've discovered this is NOT what\n>> actually happens here. If I \"git push\" from a shell and then log out\n>> and log in again, another \"git push\" does NOT ask me for a password.\n>> In other words, the daemon is NOT shut down automatically when I log\n>> out. Given that, does it make sense to change the daemon to ignore\n>> SIGHUP, or is there some way to change it so that it does exit on\n>> logout?\n\nI have a feeling that it would be moving in a wrong direction to\nchange the code to ignore HUP, as I do think \"logout to shutdown\"\nwould be the desired behaviour. If you are not seeing that happen,\nperhaps the first thing to do is to figure out why and fix the code\nso that it happens?\n\nI dunno. I'll cc Peff so that he can take a look when he comes\nback.\n"},{"id":"272276","messageId":"20151026215016.GA17419@sigill.intra.peff.net","threadId":"40529","inReplyTo":"xmqqfv0ylwa7.fsf@gitster.mtv.corp.google.com","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-10-26T21:50:16Z","receivedAt":"2015-10-26T21:50:16Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 25, 2015 at 09:58:56AM -0700, Junio C Hamano wrote:\n\n> >>> I cannot speak for the person who was primarily responsible for\n> >>> designing this behaviour, but I happen to agree with the current\n> >>> behaviour in the situation where it was designed to be used. Upon\n> >>> the first use in your session, the \"daemon\" is auto-spawned, you can\n> >>> keep talking with that same instance during your session, and you do\n> >>> not have to do anything special to shut it down when you log out.\n> >>> Isn't that what happens here?\n> >>\n> >> After looking at this some more, I've discovered this is NOT what\n> >> actually happens here. If I \"git push\" from a shell and then log out\n> >> and log in again, another \"git push\" does NOT ask me for a password.\n> >> In other words, the daemon is NOT shut down automatically when I log\n> >> out. Given that, does it make sense to change the daemon to ignore\n> >> SIGHUP, or is there some way to change it so that it does exit on\n> >> logout?\n> \n> I have a feeling that it would be moving in a wrong direction to\n> change the code to ignore HUP, as I do think \"logout to shutdown\"\n> would be the desired behaviour. If you are not seeing that happen,\n> perhaps the first thing to do is to figure out why and fix the code\n> so that it happens?\n> \n> I dunno. I'll cc Peff so that he can take a look when he comes\n> back.\n\nI could see it going both ways.\n\nIf SIGHUP means \"I am logging out, my session is over\", then I agree it\nmakes sense to drop any credentials. And that is what SIGHUP meant when\npeople logged in through hard-wired terminals.\n\nBut these days, people often have several simultaneous sessions open.\nThey may have multiple ssh sessions to a single machine, or they may\nhave a bunch of terminal windows open, each of which has a login shell\nand will send HUP to its children when it exits. In that case, you have\na meta-session surrounding those individual terminal sessions, and you\nprobably do want to keep the cache going as long as the meta session[1].\n\nThis is all further complicated by bash's huponexit option, which I\nthink is off by default. So I, for example, have never noticed this\nbehavior even with multiple xterms, because my cache never actually gets\nSIGHUP. I don't know what shell Noam is using, but I wonder if tweaking\nthat option (or a similar one if not bash) might be helpful to signal\n\"let this stuff keep running even after I exit\".\n\nBut I am also not opposed to making it configurable somehow in git, if\nthere really are two cases that cannot otherwise be distinguished.\n\n-Peff\n\n[1] Of course we have no idea when that meta-session is closed. But if\n you have a script that runs on X logout, for instance, you could put\n \"git credential-cache exit\" in it.\n"},{"id":"272282","messageId":"CAM-tV--80xt_Ro_vQdgRmRxfy+2k2Ca13gVsjDHK+1pdsB_hqQ@mail.gmail.com","threadId":"40529","inReplyTo":"20151026215016.GA17419@sigill.intra.peff.net","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Noam Postavsky","fromEmail":"npostavs@users.sourceforge.net","sentAt":"2015-10-27T00:50:10Z","receivedAt":"2015-10-27T00:50:10Z","isPatch":false,"sender":{"key":"npostavs@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/287742?v=4"},"body":"On Mon, Oct 26, 2015 at 5:50 PM, Jeff King wrote:\n> But these days, people often have several simultaneous sessions open.\n> They may have multiple ssh sessions to a single machine, or they may\n> have a bunch of terminal windows open, each of which has a login shell\n> and will send HUP to its children when it exits.\n\nYes, except that as far as I can tell the shell never sends HUP.\n\n> This is all further complicated by bash's huponexit option, which I\n> think is off by default. So I, for example, have never noticed this\n> behavior even with multiple xterms, because my cache never actually gets\n> SIGHUP.\n\nInteresting, I was not aware of this option. I tried enabling it, but\nI see no difference, the daemon still receives no SIGHUP. Could it be\na bug in my version of bash?\n\nGNU bash, version 4.3.42(1)-release (x86_64-pc-linux-gnu)\n\nAh, it seems I'm not the only one: (Raphael Ahrens at\nhttp://unix.stackexchange.com/a/85296/47926)\n\n Bash seems to send the SIGHUP only if it self received a SIGHUP[...]\n So if you type exit or press Ctrl+D all background process will\nremain, since this does not send a hang up signal to the Bash.\n [...]\n There is an option to send the SIGHUP on exit, but it does not\nwork on my Bash 4.2.25. Maybe it works for you\n\n> I don't know what shell Noam is using, but I wonder if tweaking\n> that option (or a similar one if not bash) might be helpful to signal\n> \"let this stuff keep running even after I exit\".\n\nMy normal login shell is zsh, but I've been testing with bash and I\nsee the same behaviour with both. But the original reason for this\nwhole thread is that when running from Emacs (not via shell), the\ndaemon *always* get a SIGHUP as soon as \"git push\" finishes, which\nmakes the caching thing not so useful. We do have a workaround for\nthis by now though (start the daemon independently before calling \"git\npush\").\n"},{"id":"272317","messageId":"xmqqoafkci6j.fsf@gitster.mtv.corp.google.com","threadId":"40529","inReplyTo":"20151026215016.GA17419@sigill.intra.peff.net","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-27T17:52:52Z","receivedAt":"2015-10-27T17:52:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> But these days, people often have several simultaneous sessions open.\n> They may have multiple ssh sessions to a single machine, or they may\n> have a bunch of terminal windows open, each of which has a login shell\n> and will send HUP to its children when it exits. In that case, you have\n> a meta-session surrounding those individual terminal sessions, and you\n> probably do want to keep the cache going as long as the meta session[1].\n> ...\n> [1] Of course we have no idea when that meta-session is closed. But if\n> you have a script that runs on X logout, for instance, you could put\n> \"git credential-cache exit\" in it.\n\nYes. Probably the right way forward is to make it a non-issue by\nteaching users how to control the lifetime of the \"daemon\" process,\nand wean them off relying on \"it is auto-spawned if you forgot to\nstart\", as that convenience of auto-spawning is associated with\n\"...but how it is auto-shutdown really depends on many things in\nyour specific environment\", which is the issue.\n"},{"id":"272332","messageId":"20151027184151.GA12717@sigill.intra.peff.net","threadId":"40529","inReplyTo":"CAM-tV--80xt_Ro_vQdgRmRxfy+2k2Ca13gVsjDHK+1pdsB_hqQ@mail.gmail.com","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-10-27T18:41:51Z","receivedAt":"2015-10-27T18:41:51Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 26, 2015 at 08:50:10PM -0400, Noam Postavsky wrote:\n\n> On Mon, Oct 26, 2015 at 5:50 PM, Jeff King wrote:\n> > But these days, people often have several simultaneous sessions open.\n> > They may have multiple ssh sessions to a single machine, or they may\n> > have a bunch of terminal windows open, each of which has a login shell\n> > and will send HUP to its children when it exits.\n> \n> Yes, except that as far as I can tell the shell never sends HUP.\n\nAh, OK, I thought your original problem was too many HUPs. But reading\nmore, it is really \"too many HUPs from Emacs\".\n\nI certainly do not get SIGHUPs when I close a terminal window. But I\nwould not be surprised if that behavior is dependent on shell version,\nshell options, and terminal version.\n\n> > I don't know what shell Noam is using, but I wonder if tweaking\n> > that option (or a similar one if not bash) might be helpful to signal\n> > \"let this stuff keep running even after I exit\".\n> \n> My normal login shell is zsh, but I've been testing with bash and I\n> see the same behaviour with both. But the original reason for this\n> whole thread is that when running from Emacs (not via shell), the\n> daemon *always* get a SIGHUP as soon as \"git push\" finishes, which\n> makes the caching thing not so useful. We do have a workaround for\n> this by now though (start the daemon independently before calling \"git\n> push\").\n\nThat makes sense. According to the emacs docs[1], \"killing a buffer\nsends a SIGHUP signal to all its associated processes\". I don't know if\nthat is configurable or not, but even if it were, it might not do the\nright thing (you probably _do_ want to send a signal to most processes,\njust not the credential daemon).\n\nCertainly the daemon could do more to \"daemonize\" itself. Besides\nignoring SIGHUP, it could detach from the controlling tty, close more\ndescriptors, etc. But along the lines that Junio mentioned, I'm not sure\nif that's what people want. In general, it _is_ kind of associated with\nyour terminal session and should not live on past it.\n\nI wonder if it would work in your case to simply nohup the credential\nhelper. I.e., put this in your git config:\n\n [credential]\n helper = \"!nohup git credential-cache\"\n\nThere are probably some weird things with how nohup works, though (e.g.,\nit redirects stderr to stdout, which is not what you want). Ignored\nsignals are inherited by children, so you could probably just do:\n\n [credential]\n helper = \"!trap '' HUP; git credential-cache\"\n\nThat _almost_ works. Unfortunately, credential-cache installs its own\nSIGHUP handler to clean up its socket. So it cleans up the socket, and\nthen chains to the original handler, which was to ignore the signal.\nMeaning we keep running, but nobody can contact us. Whoops.\n\nSo I dunno. I think it would be reasonable to provide a config option to\ntell the cache-daemon to just ignore SIGHUP (or alternatively, a general\noption to try harder to dissociate itself from the caller). But I'm not\nsure I'd agree with making it the default. I'd want to know if anybody\nis actually _using_ the SIGHUP-exits-daemon feature. Clearly neither of\nus is, but I wouldn't be surprised if other setups are.\n\n-Peff\n\n[1] http://www.gnu.org/s/emacs/manual/html_node/elisp/Signals-to-Processes.html\n"},{"id":"272333","messageId":"20151027184702.GB12717@sigill.intra.peff.net","threadId":"40529","inReplyTo":"xmqqoafkci6j.fsf@gitster.mtv.corp.google.com","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-10-27T18:47:02Z","receivedAt":"2015-10-27T18:47:02Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 27, 2015 at 10:52:52AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > But these days, people often have several simultaneous sessions open.\n> > They may have multiple ssh sessions to a single machine, or they may\n> > have a bunch of terminal windows open, each of which has a login shell\n> > and will send HUP to its children when it exits. In that case, you have\n> > a meta-session surrounding those individual terminal sessions, and you\n> > probably do want to keep the cache going as long as the meta session[1].\n> > ...\n> > [1] Of course we have no idea when that meta-session is closed. But if\n> > you have a script that runs on X logout, for instance, you could put\n> > \"git credential-cache exit\" in it.\n> \n> Yes. Probably the right way forward is to make it a non-issue by\n> teaching users how to control the lifetime of the \"daemon\" process,\n> and wean them off relying on \"it is auto-spawned if you forgot to\n> start\", as that convenience of auto-spawning is associated with\n> \"...but how it is auto-shutdown really depends on many things in\n> your specific environment\", which is the issue.\n\nI dunno. I think the auto-spawn is really what makes it usable; you can\ndrop it in with \"git config credential.helper config\" and forget about\nit. Anything more fancy requires touching your login/startup files.\nCertainly I'm not opposed to people setting it up outside of the\nauto-spawn, but I wouldn't want that feature to go away.\n\nAFAICT, it works pretty well out of the box for most setups (where\nterminals do _not_ send SIGHUP; so we auto-start, and then it holds the\ncredential until the timer expires).\n\nI am a little surprised that credential-cache gets wide use. I would\nthink most people would prefer to use a system-specific secure-storage\nhelper. I don't know what the state of the art is for that on Linux[1], but\nwe seem to have only gnome-keyring in contrib/.\n\n-Peff\n\n[1] I use Linux, but I do not use any of the common desktop\n environments. However, I have my own personal read-only key program\n that speaks the helper protocol.\n"},{"id":"272335","messageId":"xmqqk2q8p1yn.fsf@gitster.mtv.corp.google.com","threadId":"40529","inReplyTo":"20151027184151.GA12717@sigill.intra.peff.net","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-27T19:04:48Z","receivedAt":"2015-10-27T19:04:48Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> So I dunno. I think it would be reasonable to provide a config option to\n> tell the cache-daemon to just ignore SIGHUP (or alternatively, a general\n> option to try harder to dissociate itself from the caller). But I'm not\n> sure I'd agree with making it the default. I'd want to know if anybody\n> is actually _using_ the SIGHUP-exits-daemon feature. Clearly neither of\n> us is, but I wouldn't be surprised if other setups are.\n\nThat also is a very good summary of what I think. I agree it is a\ngood thing to have an option between \"keep the die-on-HUP for those\nwho have a working set-up that rely on it\" and \"daemonize fully and\nrequire the user to explicitly kill it with 'credential-cache\nexit'\".\n\nThanks.\n"},{"id":"272394","messageId":"CAM-tV--B3HaC1DcORfnx9bWW9-quyk0=pQDxmvonc=6dgrMOxA@mail.gmail.com","threadId":"40529","inReplyTo":"20151027184702.GB12717@sigill.intra.peff.net","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Noam Postavsky","fromEmail":"npostavs@users.sourceforge.net","sentAt":"2015-10-28T03:46:20Z","receivedAt":"2015-10-28T03:46:20Z","isPatch":false,"sender":{"key":"npostavs@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/287742?v=4"},"body":"On Tue, Oct 27, 2015 at 2:47 PM, Jeff King wrote:\n> On Tue, Oct 27, 2015 at 10:52:52AM -0700, Junio C Hamano wrote:\n>> Yes. Probably the right way forward is to make it a non-issue by\n>> teaching users how to control the lifetime of the \"daemon\" process,\n>> and wean them off relying on \"it is auto-spawned if you forgot to\n>> start\", as that convenience of auto-spawning is associated with\n>> \"...but how it is auto-shutdown really depends on many things in\n>> your specific environment\", which is the issue.\n>\n> I dunno. I think the auto-spawn is really what makes it usable; you can\n> drop it in with \"git config credential.helper config\" and forget about\n> it. Anything more fancy requires touching your login/startup files.\n> Certainly I'm not opposed to people setting it up outside of the\n> auto-spawn, but I wouldn't want that feature to go away.\n\nPerhaps we could express the auto-spawn more explicitly, something\nlike \"git config credential.pre-helper start-cache-daemon\". A way to\nrun a command before the credential helpers start would be useful to\nour magit workaround for this issue (currently we start the daemon\nbefore \"push\", \"fetch\", and \"pull\", but it won't work with user\naliases that run those commands).\n\n>\n> I am a little surprised that credential-cache gets wide use. I would\n> think most people would prefer to use a system-specific secure-storage\n> helper. I don't know what the state of the art is for that on Linux[1], but\n> we seem to have only gnome-keyring in contrib/.\n\nI'm not sure it's that widely used. Perhaps most people use ssh-agent?\nThat's what I do, though I've been experimenting with credential-cache\nsince it was brought up by a magit user.\n"},{"id":"272503","messageId":"20151030001000.GA2123@sigill.intra.peff.net","threadId":"40529","inReplyTo":"CAM-tV--B3HaC1DcORfnx9bWW9-quyk0=pQDxmvonc=6dgrMOxA@mail.gmail.com","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-10-30T00:10:01Z","receivedAt":"2015-10-30T00:10:01Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 27, 2015 at 11:46:20PM -0400, Noam Postavsky wrote:\n\n> > I dunno. I think the auto-spawn is really what makes it usable; you can\n> > drop it in with \"git config credential.helper config\" and forget about\n> > it. Anything more fancy requires touching your login/startup files.\n> > Certainly I'm not opposed to people setting it up outside of the\n> > auto-spawn, but I wouldn't want that feature to go away.\n> \n> Perhaps we could express the auto-spawn more explicitly, something\n> like \"git config credential.pre-helper start-cache-daemon\". A way to\n> run a command before the credential helpers start would be useful to\n> our magit workaround for this issue (currently we start the daemon\n> before \"push\", \"fetch\", and \"pull\", but it won't work with user\n> aliases that run those commands).\n\nI'm not clear on when the pre-helper would be run. Git runs the helper\nwhen it needs a credential. What git command would start it? Or are you\nproposing a new credential interface for programs (like magit) to call\nalong the lines of \"do any prep work you need; I might be asking for a\ncredential soon\", without having to know the specifics of the user's\nhelper config.\n\nYou can do that already like:\n\n echo url=dummy://a:b@c/ | git credential approve\n\nthough I guess for some helpers, that may actually store the bogus\nvalue (for the cache, it inserts a dummy cache entry, but that's not a\nproblem; something with permanent storage would be more annoying).\n\nI guess the most elegant thing would be to add an \"init\" command to the\nhelper interface. So magit would run:\n\n git credential init\n\nwhich would then see that we have \"credential.helper\" defined as \"foo\",\nand would run:\n\n git credential-foo init\n\nwhich could be a noop, start a daemon, or whatever, depending on how the\nhelper operates.\n\nI dunno. It almost seems like adding a credentialcache.ignoreHUP option\nwould be less hacky. :)\n\n> I'm not sure it's that widely used. Perhaps most people use ssh-agent?\n> That's what I do, though I've been experimenting with credential-cache\n> since it was brought up by a magit user.\n\nI don't know. Certainly up until a few years ago, anybody sane was using\nssh-agent, because the http password stuff was so awful (no storage, and\nwe didn't even respect 401s for a long time). But these days sites like\nGitHub push people into using https as the protocol of choice. Mostly, I\nthink, because there was a lot of support load in teaching people to set\nup ssh. But I guess a lot of those people are on non-Linux platforms.\n\n-Peff\n"},{"id":"272505","messageId":"CAM-tV-_dc_YEE0Dh2T=8+_DcBiq_rvynOw2cFi+8QizkeGTusw@mail.gmail.com","threadId":"40529","inReplyTo":"20151030001000.GA2123@sigill.intra.peff.net","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Noam Postavsky","fromEmail":"npostavs@users.sourceforge.net","sentAt":"2015-10-30T00:43:58Z","receivedAt":"2015-10-30T00:43:58Z","isPatch":false,"sender":{"key":"npostavs@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/287742?v=4"},"body":"On Thu, Oct 29, 2015 at 8:10 PM, Jeff King wrote:\n> On Tue, Oct 27, 2015 at 11:46:20PM -0400, Noam Postavsky wrote:\n>> Perhaps we could express the auto-spawn more explicitly, something\n>> like \"git config credential.pre-helper start-cache-daemon\". A way to\n>> run a command before the credential helpers start would be useful to\n>> our magit workaround for this issue (currently we start the daemon\n>> before \"push\", \"fetch\", and \"pull\", but it won't work with user\n>> aliases that run those commands).\n>\n> I'm not clear on when the pre-helper would be run. Git runs the helper\n> when it needs a credential. What git command would start it?\n\nI was just thinking in terms of our current workaround, it would have\nbeen helpful to be able to run a command just before the helpers are\nrun. Or in other words, as the first helper. (doing \"git -c\ncredential.helper=foo\" puts foo as the last helper).\n\n> I guess the most elegant thing would be to add an \"init\" command to the\n> helper interface. So magit would run:\n>\n> git credential init\n\nAlthough, we could use something like that too, as we're currently\nchecking the helpers configured and then running git\ncredential-cache--daemon directly.\n\n> I dunno. It almost seems like adding a credentialcache.ignoreHUP option\n> would be less hacky. :)\n\nThe pre-helper thing is probably the best way to make the current\nhacks less hacky, but maybe not so great for actually fixing the\nproblem and getting rid of the need for said hacks. :)\n\n> Mostly, I think, because there was a lot of support load in teaching\n> people to set up ssh. But I guess a lot of those people are on\n> non-Linux platforms.\n\nHmm, the pre-helper thing would also help for the hack I wrote getting\nssh-agent to autostart and work from Emacs on Windows (ssh-agent on\nWindows is a total PITA).\n"},{"id":"272506","messageId":"20151030005057.GA23251@sigill.intra.peff.net","threadId":"40529","inReplyTo":"CAM-tV-_dc_YEE0Dh2T=8+_DcBiq_rvynOw2cFi+8QizkeGTusw@mail.gmail.com","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-10-30T00:50:57Z","receivedAt":"2015-10-30T00:50:57Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 29, 2015 at 08:43:58PM -0400, Noam Postavsky wrote:\n\n> > I'm not clear on when the pre-helper would be run. Git runs the helper\n> > when it needs a credential. What git command would start it?\n> \n> I was just thinking in terms of our current workaround, it would have\n> been helpful to be able to run a command just before the helpers are\n> run. Or in other words, as the first helper. (doing \"git -c\n> credential.helper=foo\" puts foo as the last helper).\n\nIf you know the helper you want to run, you are free to just run \"git\ncredential-foo\". So I don't think that helps the inelegance of your\nworkaround (the real inelegance is that you are assuming that \"foo\"\nneeds run in the first place).\n\nI'm still not sure how the pre-helper would work. What git command kicks\noff the pre-helper command? Wouldn't it also be subject to the SIGHUP\nproblem?\n\n-Peff\n"},{"id":"272509","messageId":"CAM-tV-8qSVJFOxLQt9SaYK_WqpxixzPArJnAK=3tHU9inM9Law@mail.gmail.com","threadId":"40529","inReplyTo":"20151030005057.GA23251@sigill.intra.peff.net","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Noam Postavsky","fromEmail":"npostavs@users.sourceforge.net","sentAt":"2015-10-30T01:20:01Z","receivedAt":"2015-10-30T01:20:01Z","isPatch":false,"sender":{"key":"npostavs@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/287742?v=4"},"body":"On Thu, Oct 29, 2015 at 8:50 PM, Jeff King wrote:\n> workaround (the real inelegance is that you are assuming that \"foo\"\n> needs run in the first place).\n\nWell, we currently check the output from \"git config\ncredential.helpers\" to determine what's needed, so the inelegance here\nis that we reimplement git's checking of this option.\n\n> I'm still not sure how the pre-helper would work. What git command kicks\n> off the pre-helper command? Wouldn't it also be subject to the SIGHUP\n> problem?\n\nAh, maybe the missing piece I forgot to mention is that we could make\nour pre/1st-helper be an emacsclient command, which would tell Emacs\nto startup the daemon. So the daemon would be a subprocess of Emacs,\nnot \"git push\", thereby avoiding the SIGHUP. In our current workaround\nwe startup the daemon (if it's not running) before git commands that\nwe think are going to run credential helpers (i.e. \"push\", \"pull\",\n\"fetch\"), hence my thought that it would be nicer if we only did that\nbefore git is *actually* going to run the helpers.\n"},{"id":"272580","messageId":"20151030210849.GA7149@sigill.intra.peff.net","threadId":"40529","inReplyTo":"CAM-tV-8qSVJFOxLQt9SaYK_WqpxixzPArJnAK=3tHU9inM9Law@mail.gmail.com","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-10-30T21:08:49Z","receivedAt":"2015-10-30T21:08:49Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 29, 2015 at 09:20:01PM -0400, Noam Postavsky wrote:\n\n> On Thu, Oct 29, 2015 at 8:50 PM, Jeff King wrote:\n> > workaround (the real inelegance is that you are assuming that \"foo\"\n> > needs run in the first place).\n> \n> Well, we currently check the output from \"git config\n> credential.helpers\" to determine what's needed, so the inelegance here\n> is that we reimplement git's checking of this option.\n\nRight. And not only is that hard to get right (I doubt, for example, you\nsupport the arbitrary \"!\" shell expressions that git does), but it's\nimpossible to know for sure that will be needed, because you cannot know\nall possible helpers (I might even have a helper that is a shell snippet\nthat calls credential-cache).\n\nAs a workaround, I don't think looking for just \"cache\" is unreasonable.\nBut it is not a very robust solution. :)\n\n> > I'm still not sure how the pre-helper would work. What git command kicks\n> > off the pre-helper command? Wouldn't it also be subject to the SIGHUP\n> > problem?\n> \n> Ah, maybe the missing piece I forgot to mention is that we could make\n> our pre/1st-helper be an emacsclient command, which would tell Emacs\n> to startup the daemon. So the daemon would be a subprocess of Emacs,\n> not \"git push\", thereby avoiding the SIGHUP. In our current workaround\n> we startup the daemon (if it's not running) before git commands that\n> we think are going to run credential helpers (i.e. \"push\", \"pull\",\n> \"fetch\"), hence my thought that it would be nicer if we only did that\n> before git is *actually* going to run the helpers.\n\nI don't think even git knows it will need a helper until it is actually\nready to call one (e.g., it may depend on getting an HTTP 401 from the\nserver).\n\nI am leaning more towards ignoring SIGHUP (configurably) being the only\nreally sane path forward. Do you want to try your hand at a patch?\n\n-Peff\n"},{"id":"273072","messageId":"CAM-tV-9CNO_hqnweFpLaRHx4xEA32CPRdq56y6vYMWqURV9kgg@mail.gmail.com","threadId":"40529","inReplyTo":"20151030210849.GA7149@sigill.intra.peff.net","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Noam Postavsky","fromEmail":"npostavs@users.sourceforge.net","sentAt":"2015-11-09T02:58:06Z","receivedAt":"2015-11-09T02:58:06Z","isPatch":false,"sender":{"key":"npostavs@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/287742?v=4"},"body":"On Fri, Oct 30, 2015 at 5:08 PM, Jeff King wrote:\n> Right. And not only is that hard to get right (I doubt, for example, you\n> support the arbitrary \"!\" shell expressions that git does), but it's\n> impossible to know for sure that will be needed, because you cannot know\n> all possible helpers (I might even have a helper that is a shell snippet\n> that calls credential-cache).\n\nYep, in that case the user would have to override the result of parsing.\n\n>> Ah, maybe the missing piece I forgot to mention is that we could make\n>> our pre/1st-helper be an emacsclient command, which would tell Emacs\n>> to startup the daemon. So the daemon would be a subprocess of Emacs,\n>> not \"git push\", thereby avoiding the SIGHUP. In our current workaround\n>> we startup the daemon (if it's not running) before git commands that\n>> we think are going to run credential helpers (i.e. \"push\", \"pull\",\n>> \"fetch\"), hence my thought that it would be nicer if we only did that\n>> before git is *actually* going to run the helpers.\n>\n> I don't think even git knows it will need a helper until it is actually\n> ready to call one (e.g., it may depend on getting an HTTP 401 from the\n> server).\n\nYes, so just call me first. :)\n\n>\n> I am leaning more towards ignoring SIGHUP (configurably) being the only\n> really sane path forward. Do you want to try your hand at a patch?\n\nSomething like this?\n\ndiff --git i/credential-cache--daemon.c w/credential-cache--daemon.c\nindex eef6fce..e3f2612 100644\n--- i/credential-cache--daemon.c\n+++ w/credential-cache--daemon.c\n@@ -256,6 +256,9 @@ int main(int argc, const char **argv)\n OPT_END()\n };\n\n+ int ignore_sighup = 0;\n+ git_config_get_bool(\"credential.cache.ignoreSighup\", &ignore_sighup);\n+\n argc = parse_options(argc, argv, NULL, options, usage, 0);\n socket_path = argv[0];\n\n@@ -264,6 +267,12 @@ int main(int argc, const char **argv)\n\n check_socket_directory(socket_path);\n register_tempfile(&socket_file, socket_path);\n+\n+ if (ignore_sighup) {\n+ sigchain_pop(SIGHUP);\n+ signal(SIGHUP, SIG_IGN);\n+ }\n+\n serve_cache(socket_path, debug);\n delete_tempfile(&socket_file);\n"},{"id":"273081","messageId":"20151109155342.GB27224@sigill.intra.peff.net","threadId":"40529","inReplyTo":"CAM-tV-9CNO_hqnweFpLaRHx4xEA32CPRdq56y6vYMWqURV9kgg@mail.gmail.com","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-11-09T15:53:43Z","receivedAt":"2015-11-09T15:53:43Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 08, 2015 at 09:58:06PM -0500, Noam Postavsky wrote:\n\n> > I am leaning more towards ignoring SIGHUP (configurably) being the only\n> > really sane path forward. Do you want to try your hand at a patch?\n> \n> Something like this?\n\nYes, but with a proper commit message, and an update to\nDocumentation/config.txt. :)\n\nAutomated tests would be nice, but I suspect it may be too complicated\nto be worth it.\n\n> diff --git i/credential-cache--daemon.c w/credential-cache--daemon.c\n> index eef6fce..e3f2612 100644\n> --- i/credential-cache--daemon.c\n> +++ w/credential-cache--daemon.c\n> @@ -256,6 +256,9 @@ int main(int argc, const char **argv)\n> OPT_END()\n> };\n> \n> + int ignore_sighup = 0;\n> + git_config_get_bool(\"credential.cache.ignoreSighup\", &ignore_sighup);\n\nI don't think we should use the credential.X.* namespace here. That is\nalready reserved for credential setup for URLs matching \"X\".\n\nProbably \"credentialCache.ignoreSIGHUP\" would be better. Or maybe\n\"credential-cache\". We usually avoid dashes in our config names, but\nin this case it matches the program name.\n\nAlso, we usually spell config names as all-lowercase in the code. The\nolder callback-interface config code needed this (since we just strcmp'd\nthe keys against a normalized case). I think git_config_get_bool() will\nnormalize the key we feed it, but I'd rather stay consistent.\n\n> @@ -264,6 +267,12 @@ int main(int argc, const char **argv)\n> \n> check_socket_directory(socket_path);\n> register_tempfile(&socket_file, socket_path);\n> +\n> + if (ignore_sighup) {\n> + sigchain_pop(SIGHUP);\n> + signal(SIGHUP, SIG_IGN);\n> + }\n> +\n\nI don't think you need to pop the tempfile handler here. You can simply\nsigchain_push() the SIG_IGN, and since we won't ever pop and propagate\nthat, it doesn't matter what is under it.\n\n-Peff\n"},{"id":"273115","messageId":"CAM-tV--hBSdCJckCnMtKgkQB2f_3eN8sXHdFWwg2hzb6s7ufxw@mail.gmail.com","threadId":"40529","inReplyTo":"20151109155342.GB27224@sigill.intra.peff.net","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Noam Postavsky","fromEmail":"npostavs@users.sourceforge.net","sentAt":"2015-11-10T01:05:25Z","receivedAt":"2015-11-10T01:05:25Z","isPatch":false,"sender":{"key":"npostavs@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/287742?v=4"},"body":"On Mon, Nov 9, 2015 at 10:53 AM, Jeff King wrote:\n> Yes, but with a proper commit message, and an update to\n> Documentation/config.txt. :)\n\nRight, see attached.\n\n>\n> Automated tests would be nice, but I suspect it may be too complicated\n> to be worth it.\n\nI attempted\n\ntest_ignore_sighup ()\n{\n mkdir \"$HOME/.git-credential-cache\" &&\n chmod 0700 \"$HOME/.git-credential-cache\" &&\n git -c credentialCache.ignoreSIGHUP=true credential-cache--daemon\n\"$HOME/.git-credential-cache/socket\" &\n kill -SIGHUP $! &&\n ps $!\n}\n\ntest_expect_success 'credentialCache.ignoreSIGHUP works' 'test_ignore_sighup'\n\nbut it does't pass (testing manually by running\n./git-credential-cache--daemon $HOME/.git-credential-cache/test-socket\n& and then kill -HUP does work).\n\n>\n> I don't think we should use the credential.X.* namespace here. That is\n> already reserved for credential setup for URLs matching \"X\".\n>\n> Probably \"credentialCache.ignoreSIGHUP\" would be better. Or maybe\n> \"credential-cache\". We usually avoid dashes in our config names, but\n> in this case it matches the program name.\n\nI went with \"credentialCache.ignoreSIGHUP\".\n\n>\n> Also, we usually spell config names as all-lowercase in the code. The\n> older callback-interface config code needed this (since we just strcmp'd\n> the keys against a normalized case). I think git_config_get_bool() will\n> normalize the key we feed it, but I'd rather stay consistent.\n\nOh, I didn't even realize git config names were case insensitive.\n\n> I don't think you need to pop the tempfile handler here. You can simply\n> sigchain_push() the SIG_IGN, and since we won't ever pop and propagate\n> that, it doesn't matter what is under it.\n\nYup.\n\n\nFrom 5fc95b6e2f956403da6845fc3ced83b21bee7bb0 Mon Sep 17 00:00:00 2001\nFrom: Noam Postavsky \nDate: Mon, 9 Nov 2015 19:26:29 -0500\nSubject: [PATCH] credential-cache: new option to ignore sighup\n\nIntroduce new option \"credentialCache.ignoreSIGHUP\" which stops\ngit-credential-cache--daemon from quitting on SIGHUP. This is useful\nwhen \"git push\" is started from Emacs, because all child\nprocesses (including the daemon) will receive a SIGHUP when \"git push\"\nexits.\n---\n Documentation/config.txt | 3 +++\n credential-cache--daemon.c | 7 +++++++\n 2 files changed, 10 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 4d3cb10..4444e5b 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1122,6 +1122,9 @@ credential..*::\n \texample.com. See linkgit:gitcredentials[7] for details on how URLs are\n \tmatched.\n \n+credentialCache.ignoreSIGHUP::\n+\tTell git-credential-cache--daemon to ignore SIGHUP, instead of quitting.\n+\n include::diff-config.txt[]\n \n difftool..path::\ndiff --git a/credential-cache--daemon.c b/credential-cache--daemon.c\nindex eef6fce..6cda9c0 100644\n--- a/credential-cache--daemon.c\n+++ b/credential-cache--daemon.c\n@@ -256,6 +256,9 @@ int main(int argc, const char **argv)\n \t\tOPT_END()\n \t};\n \n+\tint ignore_sighup = 0;\n+\tgit_config_get_bool(\"credentialcache.ignoresighup\", &ignore_sighup);\n+\n \targc = parse_options(argc, argv, NULL, options, usage, 0);\n \tsocket_path = argv[0];\n \n@@ -264,6 +267,10 @@ int main(int argc, const char **argv)\n \n \tcheck_socket_directory(socket_path);\n \tregister_tempfile(&socket_file, socket_path);\n+\n+\tif (ignore_sighup)\n+\t\tsignal(SIGHUP, SIG_IGN);\n+\n \tserve_cache(socket_path, debug);\n \tdelete_tempfile(&socket_file);\n \n-- \n2.6.1\n\n"},{"id":"273144","messageId":"20151110122501.GA14418@sigill.intra.peff.net","threadId":"40529","inReplyTo":"CAM-tV--hBSdCJckCnMtKgkQB2f_3eN8sXHdFWwg2hzb6s7ufxw@mail.gmail.com","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-11-10T12:25:02Z","receivedAt":"2015-11-10T12:25:02Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 09, 2015 at 08:05:25PM -0500, Noam Postavsky wrote:\n\n> > Automated tests would be nice, but I suspect it may be too complicated\n> > to be worth it.\n> \n> I attempted\n> \n> test_ignore_sighup ()\n> {\n> mkdir \"$HOME/.git-credential-cache\" &&\n> chmod 0700 \"$HOME/.git-credential-cache\" &&\n> git -c credentialCache.ignoreSIGHUP=true credential-cache--daemon\n> \"$HOME/.git-credential-cache/socket\" &\n> kill -SIGHUP $! &&\n> ps $!\n> }\n> \n> test_expect_success 'credentialCache.ignoreSIGHUP works' 'test_ignore_sighup'\n> \n> but it does't pass (testing manually by running\n> ./git-credential-cache--daemon $HOME/.git-credential-cache/test-socket\n> & and then kill -HUP does work).\n\nYour test looks racy. After the \"&\" in spawning the daemon, we don't\nhave any guarantee that the daemon actually reached the signal() call\nbefore we issued our kill.\n\nThe daemon issues an \"ok\\n\" to stdout to report that it's ready to serve\n(this is how the auto-spawn avoids races). So you could use that with a\nfifo to fix this. It's a little complicated; see lib-git-daemon.sh for\nan example.\n\nI'm also not sure the use of \"ps\" here is portable (i.e., does it always\nreport a useful error code on all systems?).\n\nSo I dunno. Given the complexity of the test, and that it is such a\nsimple feature that is unlikely to be broken, I'm not sure it is worth\nit. A test failure seems like it would more likely be a problem in the\ntest, not the code.\n\n> From 5fc95b6e2f956403da6845fc3ced83b21bee7bb0 Mon Sep 17 00:00:00 2001\n> From: Noam Postavsky \n> Date: Mon, 9 Nov 2015 19:26:29 -0500\n> Subject: [PATCH] credential-cache: new option to ignore sighup\n> \n> Introduce new option \"credentialCache.ignoreSIGHUP\" which stops\n> git-credential-cache--daemon from quitting on SIGHUP. This is useful\n> when \"git push\" is started from Emacs, because all child\n> processes (including the daemon) will receive a SIGHUP when \"git push\"\n> exits.\n> ---\n\nCan you add a signed-off-by here (see the \"sign-off\" section in\nDocumentation/SubmittingPatches).\n\n> diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c\n> index eef6fce..6cda9c0 100644\n> --- a/credential-cache--daemon.c\n> +++ b/credential-cache--daemon.c\n> @@ -256,6 +256,9 @@ int main(int argc, const char **argv)\n> \t\tOPT_END()\n> \t};\n> \n> +\tint ignore_sighup = 0;\n> +\tgit_config_get_bool(\"credentialcache.ignoresighup\", &ignore_sighup);\n> +\n\nStyle-wise, I think the declaration should go above the options-list.\n\n> @@ -264,6 +267,10 @@ int main(int argc, const char **argv)\n> \n> \tcheck_socket_directory(socket_path);\n> \tregister_tempfile(&socket_file, socket_path);\n> +\n> +\tif (ignore_sighup)\n> +\t\tsignal(SIGHUP, SIG_IGN);\n> +\n\nThis part looks obviously correct. :)\n\nI don't think there's any need to use sigchain_push here (we have no\nintention of ever popping it).\n\n-Peff\n"},{"id":"273145","messageId":"20151110122625.GB14418@sigill.intra.peff.net","threadId":"40529","inReplyTo":"20151110122501.GA14418@sigill.intra.peff.net","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-11-10T12:26:26Z","receivedAt":"2015-11-10T12:26:26Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 10, 2015 at 07:25:02AM -0500, Jeff King wrote:\n\n> > Introduce new option \"credentialCache.ignoreSIGHUP\" which stops\n> > git-credential-cache--daemon from quitting on SIGHUP. This is useful\n> > when \"git push\" is started from Emacs, because all child\n> > processes (including the daemon) will receive a SIGHUP when \"git push\"\n> > exits.\n> > ---\n> \n> Can you add a signed-off-by here (see the \"sign-off\" section in\n> Documentation/SubmittingPatches).\n\nTo clarify: just telling me it's OK to add your sign-off is fine here. I\ncan add it (and fix up the style thing) as I apply.\n\n-Peff\n"},{"id":"273166","messageId":"CAM-tV--dCUQwc7dAjgNUpHYx-T4VhdZ_gzcY+=6ZsfLhDF_MEg@mail.gmail.com","threadId":"40529","inReplyTo":"20151110122625.GB14418@sigill.intra.peff.net","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Noam Postavsky","fromEmail":"npostavs@users.sourceforge.net","sentAt":"2015-11-11T00:22:07Z","receivedAt":"2015-11-11T00:22:07Z","isPatch":false,"sender":{"key":"npostavs@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/287742?v=4"},"body":"I think you're right about the automated test not being worth the trouble.\n\nOn Tue, Nov 10, 2015 at 7:26 AM, Jeff King wrote:\n> To clarify: just telling me it's OK to add your sign-off is fine here. I\n> can add it (and fix up the style thing) as I apply.\n\nOk, please do that then, thanks.\n"},{"id":"274015","messageId":"xmqqpoymrql7.fsf@gitster.mtv.corp.google.com","threadId":"40529","inReplyTo":"20151110122501.GA14418@sigill.intra.peff.net","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-04T18:55:32Z","receivedAt":"2015-12-04T18:55:32Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c\n>> index eef6fce..6cda9c0 100644\n>> --- a/credential-cache--daemon.c\n>> +++ b/credential-cache--daemon.c\n>> @@ -256,6 +256,9 @@ int main(int argc, const char **argv)\n>> \t\tOPT_END()\n>> \t};\n>> \n>> +\tint ignore_sighup = 0;\n>> +\tgit_config_get_bool(\"credentialcache.ignoresighup\", &ignore_sighup);\n>> +\n>\n> Style-wise, I think the declaration should go above the options-list.\n\nI was about to merge this to 'master', following your last issue of\n\"What's cooking\" report.\n\nI was puzzled that git_config_get_bool() is used here without even\nchecking if we are inside any Git repository and wondered if it was\ncorrect. I'd imagine this is not a problem, as this process is\nspawned by \"credential-cache\" that was spawned by somebody (either\npush or fetch) who has read $GIT_DIR/config for credential.helper to\ndetermine that credential-cache needs to be used.\n\n>> @@ -264,6 +267,10 @@ int main(int argc, const char **argv)\n>> \n>> \tcheck_socket_directory(socket_path);\n>> \tregister_tempfile(&socket_file, socket_path);\n>> +\n>> +\tif (ignore_sighup)\n>> +\t\tsignal(SIGHUP, SIG_IGN);\n>> +\n>\n> This part looks obviously correct. :)\n>\n> I don't think there's any need to use sigchain_push here (we have no\n> intention of ever popping it).\n>\n> -Peff\n"},{"id":"274016","messageId":"20151204190658.GA16692@sigill.intra.peff.net","threadId":"40529","inReplyTo":"xmqqpoymrql7.fsf@gitster.mtv.corp.google.com","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-12-04T19:06:59Z","receivedAt":"2015-12-04T19:06:59Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 04, 2015 at 10:55:32AM -0800, Junio C Hamano wrote:\n\n> >> +\tint ignore_sighup = 0;\n> >> +\tgit_config_get_bool(\"credentialcache.ignoresighup\", &ignore_sighup);\n> >> +\n> >\n> > Style-wise, I think the declaration should go above the options-list.\n> \n> I was about to merge this to 'master', following your last issue of\n> \"What's cooking\" report.\n> \n> I was puzzled that git_config_get_bool() is used here without even\n> checking if we are inside any Git repository and wondered if it was\n> correct. I'd imagine this is not a problem, as this process is\n> spawned by \"credential-cache\" that was spawned by somebody (either\n> push or fetch) who has read $GIT_DIR/config for credential.helper to\n> determine that credential-cache needs to be used.\n\nThat does not have to be the case; I imagine most people would put\ncredential.helper in their ~/.gitconfig.\n\nBut I'm not sure I understand how that is relevant. The config subsystem\nshould work just fine whether we are in a repository or not (and if not,\nreturn results only from system and user-wide config).\n\nThis probably _does_ trigger setup_git_env() when it was not otherwise\ncalled, and it will back to looking at \".git/config\" for the repo-level\nconfig. That may fail to find the file if we are in a bare repository,\nor a subdirectory of the working tree. IOW, I suspect this:\n\n git init --bare foo.git\n cd foo.git\n git config credential.helper cache\n git config credentialcache.ignoreSIGHUP true ;# goes into local config\n git fetch https://example.com/foo.git\n\nmay fail to respect the ignoreSIGHUP option.\n\nI guess the solution would be to setup_git_director_gently() in the\ndaemon process.\n\n-Peff\n"},{"id":"274018","messageId":"xmqqlh9arnbz.fsf@gitster.mtv.corp.google.com","threadId":"40529","inReplyTo":"20151204190658.GA16692@sigill.intra.peff.net","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-04T20:05:52Z","receivedAt":"2015-12-04T20:05:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Fri, Dec 04, 2015 at 10:55:32AM -0800, Junio C Hamano wrote:\n>> \n>> I was puzzled that git_config_get_bool() is used here without even\n>> checking if we are inside any Git repository and wondered if it was\n>> correct....\n>\n> This probably _does_ trigger setup_git_env() when it was not otherwise\n> called, and it will back to looking at \".git/config\" for the repo-level\n> config. That may fail to find the file if we are in a bare repository,\n> or a subdirectory of the working tree. IOW, I suspect this:\n>\n> git init --bare foo.git\n> cd foo.git\n> git config credential.helper cache\n> git config credentialcache.ignoreSIGHUP true ;# goes into local config\n> git fetch https://example.com/foo.git\n>\n> may fail to respect the ignoreSIGHUP option.\n>\n> I guess the solution would be to setup_git_director_gently() in the\n> daemon process.\n\nSo I guess I did notice the right breakage ;-)\n\nAt least, this won't be a regression but \"a new feature initially\nshipped with a broken corner case\", so a follow-up fix is welcome,\nbut not a big deal that I've already merged it to 'master'.\n\nThanks.\n"},{"id":"274038","messageId":"20151204232546.GC15064@sigill.intra.peff.net","threadId":"40529","inReplyTo":"xmqqlh9arnbz.fsf@gitster.mtv.corp.google.com","subject":"Re: git-credential-cache--daemon quits on SIGHUP, can we change it to ignore instead?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-12-04T23:25:46Z","receivedAt":"2015-12-04T23:25:46Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 04, 2015 at 12:05:52PM -0800, Junio C Hamano wrote:\n\n> > This probably _does_ trigger setup_git_env() when it was not otherwise\n> > called, and it will back to looking at \".git/config\" for the repo-level\n> > config. That may fail to find the file if we are in a bare repository,\n> > or a subdirectory of the working tree. IOW, I suspect this:\n> >\n> > git init --bare foo.git\n> > cd foo.git\n> > git config credential.helper cache\n> > git config credentialcache.ignoreSIGHUP true ;# goes into local config\n> > git fetch https://example.com/foo.git\n> >\n> > may fail to respect the ignoreSIGHUP option.\n> >\n> > I guess the solution would be to setup_git_director_gently() in the\n> > daemon process.\n> \n> So I guess I did notice the right breakage ;-)\n> \n> At least, this won't be a regression but \"a new feature initially\n> shipped with a broken corner case\", so a follow-up fix is welcome,\n> but not a big deal that I've already merged it to 'master'.\n\nYeah, agreed on all counts. Thanks for noticing.\n\nI suspect in practice is a pretty rare corner case, but I will leave it\nto those who are interested in the feature to do the fixup.\n\n-Peff\n"}]}