{"thread":{"id":"35793","subject":"bug? git push triggers auto pack when gc.auto = 0","startedAt":"2014-02-04T02:20:30Z","lastAt":"2014-02-12T17:36:28Z","messageCount":30,"participants":["chris","Duy Nguyen","Nguyễn Thái Ngọc Duy","David Kastrup","Junio C Hamano","Erik Faye-Lund"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"234184","messageId":"loom.20140204T030158-758@post.gmane.org","threadId":"35793","inReplyTo":null,"subject":"bug? git push triggers auto pack when gc.auto = 0","fromName":"chris","fromEmail":"jugg@hotmail.com","sentAt":"2014-02-04T02:20:30Z","receivedAt":"2014-02-04T02:20:30Z","isPatch":false,"sender":{"key":"jugg@hotmail.com","avatar":null},"body":"Hi,\n\nI have garbage collection disabled globally with gc.auto = 0. Today while\npushing a branch remotely, I saw a message \"Auto packing the repository for\noptimum performance.\" which I've never noticed before. Searching for that\nphrase shows me that common knowledge is that 'gc.auto = 0' should disable\nsuch from occurring. Looking at .git/objects/pack/ in the repository show a\nnew pack file created at the time. However, all loose objects still exist\nin the repository, which is what I want, so it is good that no apparent data\nloss occurred.\n\nHere is the relevant command and its output:\n\n$ git push origin next \nCounting objects: 56, done.\nDelta compression using up to 4 threads.\nCompressing objects: 100% (9/9), done.\nWriting objects: 100% (9/9), 895 bytes | 0 bytes/s, done.\nTotal 9 (delta 8), reused 0 (delta 0)\nAuto packing the repository for optimum performance.\nTo ssh://git@my.server.com/my_project\n 3560275..f508080 next -> next\n$ git config gc.auto\n0\n$ git config gc.autopacklimit\n0\n$ git --version\ngit version 1.8.5.3\n\nSo my question is, should gc.auto = 0 disable auto-packing from occurring on\ngit push and other non-gc commands?\n\nThanks,\n\nChris\n"},{"id":"234185","messageId":"CACsJy8Bo4XgA-g2hy+_pVEKLnerL9WNhpWe==zJANmCMdGXuow@mail.gmail.com","threadId":"35793","inReplyTo":"loom.20140204T030158-758@post.gmane.org","subject":"Re: bug? git push triggers auto pack when gc.auto = 0","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-02-04T02:41:43Z","receivedAt":"2014-02-04T02:41:43Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Feb 4, 2014 at 9:20 AM, chris wrote:\n> $ git push origin next\n> Counting objects: 56, done.\n> Delta compression using up to 4 threads.\n> Compressing objects: 100% (9/9), done.\n> Writing objects: 100% (9/9), 895 bytes | 0 bytes/s, done.\n> Total 9 (delta 8), reused 0 (delta 0)\n> Auto packing the repository for optimum performance.\n\nThis string only appears in versions before 1.8.0. It's longer after 1.8.0.\n\n> To ssh://git@my.server.com/my_project\n> 3560275..f508080 next -> next\n> $ git config gc.auto\n> 0\n> $ git config gc.autopacklimit\n> 0\n> $ git --version\n> git version 1.8.5.3\n\nbut your client is after 1.8.0 so the string printed above is from the\nserver side. \"git config gc.auto\" here does not matter. Run that\ncommand again on my.server.com.\n\n> So my question is, should gc.auto = 0 disable auto-packing from occurring on\n> git push and other non-gc commands?\n\nYes it should.\n-- \nDuy\n"},{"id":"234187","messageId":"loom.20140204T055040-646@post.gmane.org","threadId":"35793","inReplyTo":"CACsJy8Bo4XgA-g2hy+_pVEKLnerL9WNhpWe==zJANmCMdGXuow@mail.gmail.com","subject":"Re: bug? git push triggers auto pack when gc.auto = 0","fromName":"chris","fromEmail":"jugg@hotmail.com","sentAt":"2014-02-04T05:13:17Z","receivedAt":"2014-02-04T05:13:17Z","isPatch":false,"sender":{"key":"jugg@hotmail.com","avatar":null},"body":"Duy Nguyen gmail.com> writes:\n> On Tue, Feb 4, 2014 at 9:20 AM, chris hotmail.com> wrote:\n> > $ git push origin next\n> > Counting objects: 56, done.\n> > Delta compression using up to 4 threads.\n> > Compressing objects: 100% (9/9), done.\n> > Writing objects: 100% (9/9), 895 bytes | 0 bytes/s, done.\n> > Total 9 (delta 8), reused 0 (delta 0)\n> > Auto packing the repository for optimum performance.\n> \n> This string only appears in versions before 1.8.0. It's longer after 1.8.0.\n> \n> > To ssh://git my.server.com/my_project\n> > 3560275..f508080 next -> next\n> > $ git config gc.auto\n> > 0\n> > $ git config gc.autopacklimit\n> > 0\n> > $ git --version\n> > git version 1.8.5.3\n> \n> but your client is after 1.8.0 so the string printed above is from the\n> server side. \"git config gc.auto\" here does not matter. Run that\n> command again on my.server.com.\n\nOk, so I can understand if the message is from the server. I'll chalk up\nnever noticing it before to someone else always being the lucky one to\ntrigger it.\n\nHowever, I question why I should even care about this message? I'm going to\nassume that simply it is a lengthy synchronous operation that someone felt\ndeserved some verbosity to why the client push action is taking longer than\nit should. Yet that makes me question why I'm being penalized for this\nserver side operation. My client time should not be consumed for server\nside house keeping.\n\nAn obvious fix is to disable gc on the server and implement a cron job for\nthe house keeping task. However, as often the case one does not have\ncontrol over the server, so it is unfortunate that git has this server side\nhouse keeping as a blocking operation to a client action.\n\n> > So my question is, should gc.auto = 0 disable auto-packing from occurring on\n> > git push and other non-gc commands?\n> \n> Yes it should.\n\nThanks for the confirmation.\n\nRegards,\n\nChris\n"},{"id":"234188","messageId":"CACsJy8B0WKfxSYBSgRZQYz6_h+S9pGd03A=rrWM0_twRdKvyZw@mail.gmail.com","threadId":"35793","inReplyTo":"loom.20140204T055040-646@post.gmane.org","subject":"Re: bug? git push triggers auto pack when gc.auto = 0","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-02-04T06:02:14Z","receivedAt":"2014-02-04T06:02:14Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Feb 4, 2014 at 12:13 PM, chris wrote:\n> However, I question why I should even care about this message? I'm going to\n> assume that simply it is a lengthy synchronous operation that someone felt\n> deserved some verbosity to why the client push action is taking longer than\n> it should. Yet that makes me question why I'm being penalized for this\n> server side operation. My client time should not be consumed for server\n> side house keeping.\n>\n> An obvious fix is to disable gc on the server and implement a cron job for\n> the house keeping task. However, as often the case one does not have\n> control over the server, so it is unfortunate that git has this server side\n> house keeping as a blocking operation to a client action.\n\nI agree it should not block the client. I think you can Ctrl-C \"git\npush\" at this point without losing anything (data has already been\npushed at this point) but that's not a good advice to general cases.\nMaybe we can do something at the server side to not block the client..\n\nAnother thing we could do is put \"remote: \" in front of these strings,\neven in ssh case. They seem to confuse you (and me too) that things\nhappened locally.\n-- \nDuy\n"},{"id":"234189","messageId":"1391496765-29564-1-git-send-email-pclouds@gmail.com","threadId":"35793","inReplyTo":"CACsJy8B0WKfxSYBSgRZQYz6_h+S9pGd03A=rrWM0_twRdKvyZw@mail.gmail.com","subject":"[PATCH 1/2] receive-pack: update $GIT_DIR/info before auto garbage collection","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-02-04T06:52:44Z","receivedAt":"2014-02-04T06:52:44Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Auto gc could take a long time, and it's optional. \"git push\" user\nshould be allowed to stop the program if they don't want to wait. Move\nserver update step before auto gc. So we're ready to die any time\nsince auto gc is kicked off.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/receive-pack.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 85bba35..82e2f76 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1208,6 +1208,8 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n \t\t\treport(commands, unpack_status);\n \t\trun_receive_hook(commands, \"post-receive\", 1);\n \t\trun_update_post_hook(commands);\n+\t\tif (auto_update_server_info)\n+\t\t\tupdate_server_info(0);\n \t\tif (auto_gc) {\n \t\t\tconst char *argv_gc_auto[] = {\n \t\t\t\t\"gc\", \"--auto\", \"--quiet\", NULL,\n@@ -1215,8 +1217,6 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n \t\t\tint opt = RUN_GIT_CMD | RUN_COMMAND_STDOUT_TO_STDERR;\n \t\t\trun_command_v_opt(argv_gc_auto, opt);\n \t\t}\n-\t\tif (auto_update_server_info)\n-\t\t\tupdate_server_info(0);\n \t\tclear_shallow_info(&si);\n \t}\n \tif (use_sideband)\n-- \n1.8.5.2.240.g8478abd\n"},{"id":"234190","messageId":"1391496765-29564-2-git-send-email-pclouds@gmail.com","threadId":"35793","inReplyTo":"1391496765-29564-1-git-send-email-pclouds@gmail.com","subject":"[PATCH/RFC 2/2] receive-pack: hint that the user can stop \"git push\" at auto gc time","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-02-04T06:52:45Z","receivedAt":"2014-02-04T06:52:45Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Housekeeping jobs like auto gc generally should not get in the way.\nUsers who are pushing may not want to wait until auto gc is done on\nthe server. Give a hint for those users that it's safe now to break\n\"git push\" and stop waiting.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n This bandage patch may be a good compromise between running auto gc\n and not annoying users much.\n \n If I'm not mistaken, when ^C on \"git push\" this way, gc will still be\n running until it needs to print something out (which it should not\n normally because of --quiet). The user won't see gc errors, but the\n user generally can't do much anyway.\n\n builtin/gc.c | 9 ++++++++-\n builtin/receive-pack.c | 2 +-\n 2 files changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex c19545d..592271a 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -253,6 +253,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \tint auto_gc = 0;\n \tint quiet = 0;\n \tint force = 0;\n+\tint break_ok = 0;\n \tconst char *name;\n \tpid_t pid;\n \n@@ -263,6 +264,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\t\tPARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire },\n \t\tOPT_BOOL(0, \"aggressive\", &aggressive, N_(\"be more thorough (increased runtime)\")),\n \t\tOPT_BOOL(0, \"auto\", &auto_gc, N_(\"enable auto-gc mode\")),\n+\t\tOPT_HIDDEN_BOOL(0, \"break-ok\", &break_ok,\n+\t\t\t\t\"hint that it is ok to stop the program\"),\n \t\tOPT_BOOL(0, \"force\", &force, N_(\"force running gc even if there may be another gc running\")),\n \t\tOPT_END()\n \t};\n@@ -301,7 +304,11 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\t */\n \t\tif (!need_to_gc())\n \t\t\treturn 0;\n-\t\tif (!quiet)\n+\t\tif (break_ok)\n+\t\t\tfprintf(stderr,\n+\t\t\t\t_(\"Auto packing the repository for optimum performance.\\n\"\n+\t\t\t\t \"It is safe to stop the program with Ctrl-C.\\n\"));\n+\t\telse if (!quiet)\n \t\t\tfprintf(stderr,\n \t\t\t\t\t_(\"Auto packing the repository for optimum performance. You may also\\n\"\n \t\t\t\t\t\"run \\\"git gc\\\" manually. See \"\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 82e2f76..68d16e0 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1212,7 +1212,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n \t\t\tupdate_server_info(0);\n \t\tif (auto_gc) {\n \t\t\tconst char *argv_gc_auto[] = {\n-\t\t\t\t\"gc\", \"--auto\", \"--quiet\", NULL,\n+\t\t\t\t\"gc\", \"--auto\", \"--quiet\", \"--break-ok\", NULL,\n \t\t\t};\n \t\t\tint opt = RUN_GIT_CMD | RUN_COMMAND_STDOUT_TO_STDERR;\n \t\t\trun_command_v_opt(argv_gc_auto, opt);\n-- \n1.8.5.2.240.g8478abd\n"},{"id":"234191","messageId":"loom.20140204T090417-126@post.gmane.org","threadId":"35793","inReplyTo":"CACsJy8B0WKfxSYBSgRZQYz6_h+S9pGd03A=rrWM0_twRdKvyZw@mail.gmail.com","subject":"Re: bug? git push triggers auto pack when gc.auto = 0","fromName":"chris","fromEmail":"jugg@hotmail.com","sentAt":"2014-02-04T08:16:43Z","receivedAt":"2014-02-04T08:16:43Z","isPatch":false,"sender":{"key":"jugg@hotmail.com","avatar":null},"body":"Duy Nguyen gmail.com> writes:\n> On Tue, Feb 4, 2014 at 12:13 PM, chris hotmail.com> wrote:\n> > However, I question why I should even care about this message? I'm going to\n> > assume that simply it is a lengthy synchronous operation that someone felt\n> > deserved some verbosity to why the client push action is taking longer than\n> > it should. Yet that makes me question why I'm being penalized for this\n> > server side operation. My client time should not be consumed for server\n> > side house keeping.\n> >\n> > An obvious fix is to disable gc on the server and implement a cron job for\n> > the house keeping task. However, as often the case one does not have\n> > control over the server, so it is unfortunate that git has this server side\n> > house keeping as a blocking operation to a client action.\n> \n> I agree it should not block the client. I think you can Ctrl-C \"git\n> push\" at this point without losing anything (data has already been\n> pushed at this point) but that's not a good advice to general cases.\n> Maybe we can do something at the server side to not block the client..\n\nI'd like to avoid a Ctrl-C approach, but if an indication existed that\nassured me the push part of the operation had completed successfully, then\nthat would be sufficient for when I'm impatient.\n\n> Another thing we could do is put \"remote: \" in front of these strings,\n> even in ssh case. They seem to confuse you (and me too) that things\n> happened locally.\n\nYes, I would like to see more explicit clarity in what messages are coming\nfrom the server. That has always been a source of uncertainty for me with\nany remote git command output.\n\nThanks for the patches and attention to this issue, I appreciate it.\n\nChris\n"},{"id":"234192","messageId":"87r47jxp6k.fsf@fencepost.gnu.org","threadId":"35793","inReplyTo":"loom.20140204T055040-646@post.gmane.org","subject":"Re: bug? git push triggers auto pack when gc.auto = 0","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-02-04T08:22:27Z","receivedAt":"2014-02-04T08:22:27Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"chris writes:\n\n> Duy Nguyen gmail.com> writes:\n>> On Tue, Feb 4, 2014 at 9:20 AM, chris hotmail.com> wrote:\n>> > $ git push origin next\n>> > Counting objects: 56, done.\n>> > Delta compression using up to 4 threads.\n>> > Compressing objects: 100% (9/9), done.\n>> > Writing objects: 100% (9/9), 895 bytes | 0 bytes/s, done.\n>> > Total 9 (delta 8), reused 0 (delta 0)\n>> > Auto packing the repository for optimum performance.\n\n> However, I question why I should even care about this message? I'm going to\n> assume that simply it is a lengthy synchronous operation that someone felt\n> deserved some verbosity to why the client push action is taking longer than\n> it should. Yet that makes me question why I'm being penalized for this\n> server side operation. My client time should not be consumed for server\n> side house keeping.\n\nYour \"client time\" is not consumed: this is not a busy wait. Git server\nprocesses are synchronous: they are initiated and completed under the\ncontrol of a client. That means that if you run a batch script\nexecuting a number of commands in a client, it will not saturate the\nserver with half-finished processes and/or will refuse to honor requests\nbecause the repository is locked.\n\n> An obvious fix is to disable gc on the server and implement a cron job\n> for the house keeping task. However, as often the case one does not\n> have control over the server, so it is unfortunate that git has this\n> server side house keeping as a blocking operation to a client action.\n\n_Any_ git operation is \"blocking\" the respective initiating client.\n\n>> > So my question is, should gc.auto = 0 disable auto-packing from occurring on\n>> > git push and other non-gc commands?\n>> \n>> Yes it should.\n>\n> Thanks for the confirmation.\n\nAnd indeed, there is no autopacking occuring on your site when doing git\npush. The server administrator will be rather glad that the clients'\nconfiguration variables don't affect his server's operation.\n\n-- \nDavid Kastrup\n"},{"id":"234193","messageId":"loom.20140204T094437-148@post.gmane.org","threadId":"35793","inReplyTo":"87r47jxp6k.fsf@fencepost.gnu.org","subject":"Re: bug? git push triggers auto pack when gc.auto = 0","fromName":"chris","fromEmail":"jugg@hotmail.com","sentAt":"2014-02-04T08:59:45Z","receivedAt":"2014-02-04T08:59:45Z","isPatch":false,"sender":{"key":"jugg@hotmail.com","avatar":null},"body":"David Kastrup gnu.org> writes:\n> chris hotmail.com> writes:\n> > Duy Nguyen gmail.com> writes:\n> >> On Tue, Feb 4, 2014 at 9:20 AM, chris hotmail.com> wrote:\n> >> > $ git push origin next\n> >> > Counting objects: 56, done.\n> >> > Delta compression using up to 4 threads.\n> >> > Compressing objects: 100% (9/9), done.\n> >> > Writing objects: 100% (9/9), 895 bytes | 0 bytes/s, done.\n> >> > Total 9 (delta 8), reused 0 (delta 0)\n> >> > Auto packing the repository for optimum performance.\n> \n> > However, I question why I should even care about this message? I'm going to\n> > assume that simply it is a lengthy synchronous operation that someone felt\n> > deserved some verbosity to why the client push action is taking longer than\n> > it should. Yet that makes me question why I'm being penalized for this\n> > server side operation. My client time should not be consumed for server\n> > side house keeping.\n> \n> Your \"client time\" is not consumed: this is not a busy wait. Git server\n> processes are synchronous: they are initiated and completed under the\n> control of a client. That means that if you run a batch script\n> executing a number of commands in a client, it will not saturate the\n> server with half-finished processes and/or will refuse to honor requests\n> because the repository is locked.\n\nI'm slightly confused by your response. You say \"client time\" is not\nconsumed, but then go on to say that git server processes are synchronous to\navoid build up from batched client requests. I expect you took \"client\ntime\" to have some specific technical meaning, while I simply meant that the\nclient command did not return until the server completed its own house keeping.\n\nBut I do think we are on the same page otherwise in that the client command\nis blocked until the server process completes.\n\nThat said I would naively assume that a server side house keeping operation\nthat does not get invoked with every client request be a nice candidate for\nasynchronous handling without any need to tell the client about it.\n\nRegards,\n\nChris\n"},{"id":"234194","messageId":"87mwi7xm04.fsf@fencepost.gnu.org","threadId":"35793","inReplyTo":"loom.20140204T094437-148@post.gmane.org","subject":"Re: bug? git push triggers auto pack when gc.auto = 0","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-02-04T09:31:07Z","receivedAt":"2014-02-04T09:31:07Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"chris writes:\n\n> David Kastrup gnu.org> writes:\n>> chris hotmail.com> writes:\n>> > Duy Nguyen gmail.com> writes:\n>> >> On Tue, Feb 4, 2014 at 9:20 AM, chris hotmail.com> wrote:\n>> >> > $ git push origin next\n>> >> > Counting objects: 56, done.\n>> >> > Delta compression using up to 4 threads.\n>> >> > Compressing objects: 100% (9/9), done.\n>> >> > Writing objects: 100% (9/9), 895 bytes | 0 bytes/s, done.\n>> >> > Total 9 (delta 8), reused 0 (delta 0)\n>> >> > Auto packing the repository for optimum performance.\n>> \n>> > However, I question why I should even care about this message? I'm going to\n>> > assume that simply it is a lengthy synchronous operation that someone felt\n>> > deserved some verbosity to why the client push action is taking longer than\n>> > it should. Yet that makes me question why I'm being penalized for this\n>> > server side operation. My client time should not be consumed for server\n>> > side house keeping.\n>> \n>> Your \"client time\" is not consumed: this is not a busy wait. Git server\n>> processes are synchronous: they are initiated and completed under the\n>> control of a client. That means that if you run a batch script\n>> executing a number of commands in a client, it will not saturate the\n>> server with half-finished processes and/or will refuse to honor requests\n>> because the repository is locked.\n>\n> I'm slightly confused by your response. You say \"client time\" is not\n> consumed, but then go on to say that git server processes are\n> synchronous to avoid build up from batched client requests. I expect\n> you took \"client time\" to have some specific technical meaning, while\n> I simply meant that the client command did not return until the server\n> completed its own house keeping.\n\nUntil the server completed the house keeping initiated under the control\nof the client and on behalf of its command.\n\n> But I do think we are on the same page otherwise in that the client\n> command is blocked until the server process completes.\n\nSure.\n\n> That said I would naively assume that a server side house keeping\n> operation that does not get invoked with every client request be a\n> nice candidate for asynchronous handling without any need to tell the\n> client about it.\n\nExcept that there are _no_ asynchronously handled repository actions\nexecuted on behalf of a client action. If the repository owner decided\nto disable demand-based garbage collection in favor of a cron job,\nthat's his call to make. It makes some sense when there are frequent\nand multiple accesses to the repository since it avoids getting denied\naccess because of somebody _else_ triggering garbage collection\npredominantly when times are busiest.\n\nUsually you are not denied access by your _own_ garbage collection since\nthe client waits until completion.\n\nIt would be quite bad for scripting git if you constantly had to check\nafter every action whether any associated garbage collection might or\nmight not have completed.\n\nNote also that when pushing without a separate server process (like when\npushing into a local repository), there is no other job which could be\nresponsible for packing the repository rather than the one doing the\npush.\n\n-- \nDavid Kastrup\n"},{"id":"234196","messageId":"loom.20140204T104753-1@post.gmane.org","threadId":"35793","inReplyTo":"87mwi7xm04.fsf@fencepost.gnu.org","subject":"Re: bug? git push triggers auto pack when gc.auto = 0","fromName":"chris","fromEmail":"jugg@hotmail.com","sentAt":"2014-02-04T10:35:53Z","receivedAt":"2014-02-04T10:35:53Z","isPatch":false,"sender":{"key":"jugg@hotmail.com","avatar":null},"body":"David Kastrup gnu.org> writes:\n> chris hotmail.com> writes:\n> > That said I would naively assume that a server side house keeping\n> > operation that does not get invoked with every client request be a\n> > nice candidate for asynchronous handling without any need to tell the\n> > client about it.\n> \n> Except that there are _no_ asynchronously handled repository actions\n> executed on behalf of a client action. If the repository owner decided\n> to disable demand-based garbage collection in favor of a cron job,\n> that's his call to make. It makes some sense when there are frequent\n> and multiple accesses to the repository since it avoids getting denied\n> access because of somebody _else_ triggering garbage collection\n> predominantly when times are busiest.\n> \n> Usually you are not denied access by your _own_ garbage collection since\n> the client waits until completion.\n> \n> It would be quite bad for scripting git if you constantly had to check\n> after every action whether any associated garbage collection might or\n> might not have completed.\n\nI can't comment for every use case, but I find it strange that a client\nscript should need to care whether the server is currently garbage\ncollecting or not. If such a detail must be exposed to a client, then I'd\nput forth that there is a deeper issue here. But any details there are\nmoving well beyond the scope I'm able to comment on.\n\nThat said, I think I understand you that it currently does matter in the\nsense that a client can't perform other actions while garbage collection is\nrunning.\n\n> Note also that when pushing without a separate server process (like when\n> pushing into a local repository), there is no other job which could be\n> responsible for packing the repository rather than the one doing the\n> push.\n\nOk, given your full response, I understand how this is being conceptualized\nnow, thanks. However, if you look at it purely from a user's perspective\nwho is manually invoking these commands for the command's primary purpose,\nthe current behavior is annoying.\n\nIf we assume Git is right in implementing that no server async actions are\nexecuted on behalf of a client action, then this falls under the category of\nan ill-behaved server in my opinion. Anything a server does that is not\ndirectly related to fulfilling the requested client action is now considered\nbad behavior as it blocks the client from continuing whatever it needs to\nget on with. I see such implementation in Git as favoring server's needs\nover clients.\n\nRegards,\n\nChris\n"},{"id":"234197","messageId":"87iosvxhdb.fsf@fencepost.gnu.org","threadId":"35793","inReplyTo":"loom.20140204T104753-1@post.gmane.org","subject":"Re: bug? git push triggers auto pack when gc.auto = 0","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-02-04T11:11:12Z","receivedAt":"2014-02-04T11:11:12Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"chris writes:\n\n> Ok, given your full response, I understand how this is being\n> conceptualized now, thanks. However, if you look at it purely from a\n> user's perspective who is manually invoking these commands for the\n> command's primary purpose, the current behavior is annoying.\n>\n> If we assume Git is right in implementing that no server async actions\n> are executed on behalf of a client action, then this falls under the\n> category of an ill-behaved server in my opinion. Anything a server\n> does that is not directly related to fulfilling the requested client\n> action is now considered bad behavior as it blocks the client from\n> continuing whatever it needs to get on with. I see such\n> implementation in Git as favoring server's needs over clients.\n\nThere are no \"server's needs\" at all. Git only reacts to client\nrequests. It is in the clients' own interest when garbage collection is\nperiodically done since it improves response time.\n\nIt's arguable that it would be nicer to use an incremental compaction\nprocess that hides the periodic costs by distributing them over the\nrequest totality. That replaces the periodic \"why does it have to\ngarbage collect when _I_ am using it\" annoyance with \"why is this\ngenerally slow\". There is no net benefit to that approach safe for\n\na) avoiding complaints of \"smart\" people who have discovered that they\ncan speed up git by disabling garbage collection, but eventually find\nthat git is becoming slow for them but not for others.\nb) avoiding these mailing list discussions.\n\nThe second benefit could likely be achieved by displaying \"Server\nunreachable... retrying...\" instead of reporting about git gc.\n\n-- \nDavid Kastrup\n"},{"id":"234209","messageId":"xmqqha8eag6c.fsf@gitster.dls.corp.google.com","threadId":"35793","inReplyTo":"1391496765-29564-2-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH/RFC 2/2] receive-pack: hint that the user can stop \"git push\" at auto gc time","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-04T18:25:31Z","receivedAt":"2014-02-04T18:25:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> Housekeeping jobs like auto gc generally should not get in the way.\n> Users who are pushing may not want to wait until auto gc is done on\n> the server. Give a hint for those users that it's safe now to break\n> \"git push\" and stop waiting.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> This bandage patch may be a good compromise between running auto gc\n> and not annoying users much.\n> \n> If I'm not mistaken, when ^C on \"git push\" this way, gc will still be\n> running until it needs to print something out (which it should not\n> normally because of --quiet). The user won't see gc errors, but the\n> user generally can't do much anyway.\n\nIf you are over local transport, I would think you would kill the\nboth ends. Also, wouldn't killing \"git push\" before it is done\ntalking with the receive-pack stop it before it has a chance to\nupdate the remote tracking refs to pretend as if it fetched from\nthere immediately after a push?\n\nSo, no. I do not think we should ever encourage \"if this bothers\nyou, you can ^C it\". Making it not to bother is fine, though.\n"},{"id":"234211","messageId":"xmqqd2j2afup.fsf@gitster.dls.corp.google.com","threadId":"35793","inReplyTo":"xmqqha8eag6c.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC 2/2] receive-pack: hint that the user can stop \"git push\" at auto gc time","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-04T18:32:30Z","receivedAt":"2014-02-04T18:32:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> Housekeeping jobs like auto gc generally should not get in the way.\n>> Users who are pushing may not want to wait until auto gc is done on\n>> the server. Give a hint for those users that it's safe now to break\n>> \"git push\" and stop waiting.\n>>\n>> Signed-off-by: Nguyễn Thái Ngọc Duy \n>> ---\n>> This bandage patch may be a good compromise between running auto gc\n>> and not annoying users much.\n>> \n>> If I'm not mistaken, when ^C on \"git push\" this way, gc will still be\n>> running until it needs to print something out (which it should not\n>> normally because of --quiet). The user won't see gc errors, but the\n>> user generally can't do much anyway.\n>\n> If you are over local transport, I would think you would kill the\n> both ends. Also, wouldn't killing \"git push\" before it is done\n> talking with the receive-pack stop it before it has a chance to\n> update the remote tracking refs to pretend as if it fetched from\n> there immediately after a push?\n>\n> So, no. I do not think we should ever encourage \"if this bothers\n> you, you can ^C it\". Making it not to bother is fine, though.\n\nInstead of adding a boolean --break-ok that is hidden, why not\nadding an exposed boolean --daemonize, and let auto-gc run in the\nbackground? With the recent \"do not let more than one gc run at the\nsame time\", that should give a lot more pleasant end user\nexperience, no?\n"},{"id":"234441","messageId":"loom.20140207T133319-524@post.gmane.org","threadId":"35793","inReplyTo":"xmqqd2j2afup.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC 2/2] receive-pack: hint that the user can stop","fromName":"chris","fromEmail":"jugg@hotmail.com","sentAt":"2014-02-07T12:36:22Z","receivedAt":"2014-02-07T12:36:22Z","isPatch":true,"sender":{"key":"jugg@hotmail.com","avatar":null},"body":"Junio C Hamano pobox.com> writes:\n> Instead of adding a boolean --break-ok that is hidden, why not\n> adding an exposed boolean --daemonize, and let auto-gc run in the\n> background? With the recent \"do not let more than one gc run at the\n> same time\", that should give a lot more pleasant end user\n> experience, no?\n\nThat sounds quite useful to me. Duy, are you up for generating such a patch?\n\nThanks,\n\nChris\n"},{"id":"234442","messageId":"CACsJy8CksVA609ksj5BkiOmAzOJ0aoKnHrZfeUSAKzxNy2qO7w@mail.gmail.com","threadId":"35793","inReplyTo":"loom.20140207T133319-524@post.gmane.org","subject":"Re: [PATCH/RFC 2/2] receive-pack: hint that the user can stop","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-02-07T13:05:40Z","receivedAt":"2014-02-07T13:05:40Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Feb 7, 2014 at 7:36 PM, chris wrote:\n> Junio C Hamano pobox.com> writes:\n>> Instead of adding a boolean --break-ok that is hidden, why not\n>> adding an exposed boolean --daemonize, and let auto-gc run in the\n>> background? With the recent \"do not let more than one gc run at the\n>> same time\", that should give a lot more pleasant end user\n>> experience, no?\n>\n> That sounds quite useful to me. Duy, are you up for generating such a patch?\n\nIt would not be so hard for that patch. I'm still thinking whether it\nshould be done if auto-gc is started on the client side too (sometimes\nit does, which is equally annoying)..\n-- \nDuy\n"},{"id":"234445","messageId":"loom.20140207T174346-134@post.gmane.org","threadId":"35793","inReplyTo":"CACsJy8CksVA609ksj5BkiOmAzOJ0aoKnHrZfeUSAKzxNy2qO7w@mail.gmail.com","subject":"Re: [PATCH/RFC 2/2] receive-pack: hint that the user can stop","fromName":"chris","fromEmail":"jugg@hotmail.com","sentAt":"2014-02-07T16:47:24Z","receivedAt":"2014-02-07T16:47:24Z","isPatch":true,"sender":{"key":"jugg@hotmail.com","avatar":null},"body":"Duy Nguyen gmail.com> writes:\n> On Fri, Feb 7, 2014 at 7:36 PM, chris hotmail.com> wrote:\n> > Junio C Hamano pobox.com> writes:\n> >> Instead of adding a boolean --break-ok that is hidden, why not\n> >> adding an exposed boolean --daemonize, and let auto-gc run in the\n> >> background? With the recent \"do not let more than one gc run at the\n> >> same time\", that should give a lot more pleasant end user\n> >> experience, no?\n> >\n> > That sounds quite useful to me. Duy, are you up for generating such a\npatch?\n> \n> It would not be so hard for that patch. I'm still thinking whether it\n> should be done if auto-gc is started on the client side too (sometimes\n> it does, which is equally annoying)..\n\nThat could be nice, but I'd be less concerned about that, as the client has\nthe ability to disable gc for itself. Still pushing it into the background,\nif considered acceptable behavior, seems reasonable. Perhaps two separate\npatches?\n\nChris\n"},{"id":"234490","messageId":"1391843332-20583-1-git-send-email-pclouds@gmail.com","threadId":"35793","inReplyTo":"xmqqd2j2afup.fsf@gitster.dls.corp.google.com","subject":"[PATCH v2 1/2] daemon: move daemonize() to libgit.a","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-02-08T07:08:51Z","receivedAt":"2014-02-08T07:08:51Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n cache.h | 1 +\n daemon.c | 30 ++++--------------------------\n setup.c | 24 ++++++++++++++++++++++++\n 3 files changed, 29 insertions(+), 26 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex dc040fb..264b6f1 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -434,6 +434,7 @@ extern int set_git_dir_init(const char *git_dir, const char *real_git_dir, int);\n extern int init_db(const char *template_dir, unsigned int flags);\n \n extern void sanitize_stdfds(void);\n+extern int daemonize(void);\n \n #define alloc_nr(x) (((x)+16)*3/2)\n \ndiff --git a/daemon.c b/daemon.c\nindex 503e039..eba1255 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -1056,11 +1056,6 @@ static void drop_privileges(struct credentials *cred)\n \t/* nothing */\n }\n \n-static void daemonize(void)\n-{\n-\tdie(\"--detach not supported on this platform\");\n-}\n-\n static struct credentials *prepare_credentials(const char *user_name,\n const char *group_name)\n {\n@@ -1102,24 +1097,6 @@ static struct credentials *prepare_credentials(const char *user_name,\n \n \treturn &c;\n }\n-\n-static void daemonize(void)\n-{\n-\tswitch (fork()) {\n-\t\tcase 0:\n-\t\t\tbreak;\n-\t\tcase -1:\n-\t\t\tdie_errno(\"fork failed\");\n-\t\tdefault:\n-\t\t\texit(0);\n-\t}\n-\tif (setsid() == -1)\n-\t\tdie_errno(\"setsid failed\");\n-\tclose(0);\n-\tclose(1);\n-\tclose(2);\n-\tsanitize_stdfds();\n-}\n #endif\n \n static void store_pid(const char *path)\n@@ -1333,9 +1310,10 @@ int main(int argc, char **argv)\n \tif (inetd_mode || serve_mode)\n \t\treturn execute();\n \n-\tif (detach)\n-\t\tdaemonize();\n-\telse\n+\tif (detach) {\n+\t\tif (daemonize())\n+\t\t\tdie(\"--detach not supported on this platform\");\n+\t} else\n \t\tsanitize_stdfds();\n \n \tif (pid_file)\ndiff --git a/setup.c b/setup.c\nindex 6c3f85f..b09a412 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -787,3 +787,27 @@ void sanitize_stdfds(void)\n \tif (fd > 2)\n \t\tclose(fd);\n }\n+\n+int daemonize(void)\n+{\n+#ifdef NO_POSIX_GOODIES\n+\terrno = -ENOSYS;\n+\treturn -1;\n+#else\n+\tswitch (fork()) {\n+\t\tcase 0:\n+\t\t\tbreak;\n+\t\tcase -1:\n+\t\t\tdie_errno(\"fork failed\");\n+\t\tdefault:\n+\t\t\texit(0);\n+\t}\n+\tif (setsid() == -1)\n+\t\tdie_errno(\"setsid failed\");\n+\tclose(0);\n+\tclose(1);\n+\tclose(2);\n+\tsanitize_stdfds();\n+\treturn 0;\n+#endif\n+}\n-- \n1.8.5.2.240.g8478abd\n"},{"id":"234491","messageId":"1391843332-20583-2-git-send-email-pclouds@gmail.com","threadId":"35793","inReplyTo":"1391843332-20583-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 2/2] gc: config option for running --auto in background","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2014-02-08T07:08:52Z","receivedAt":"2014-02-08T07:08:52Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"`gc --auto` takes time and can block the user temporarily (but not any\nless annoyingly). Make it run in background on systems that support\nit. The only thing lost with running in background is printouts. But\ngc output is not really interesting. You can keep it in foreground by\nchanging gc.autodetach.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/config.txt | 4 ++++\n builtin/gc.c | 23 ++++++++++++++++++-----\n t/t5400-send-pack.sh | 1 +\n 3 files changed, 23 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 5f4d793..4781773 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1167,6 +1167,10 @@ gc.autopacklimit::\n \t--auto` consolidates them into one larger pack. The\n \tdefault\tvalue is 50. Setting this to 0 disables it.\n \n+gc.autodetach::\n+\tMake `git gc --auto` return immediately andrun in background\n+\tif the system supports it. Default is true.\n+\n gc.packrefs::\n \tRunning `git pack-refs` in a repository renders it\n \tunclonable by Git versions prior to 1.5.1.2 over dumb\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex c19545d..ed5cc3c 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -29,6 +29,7 @@ static int pack_refs = 1;\n static int aggressive_window = 250;\n static int gc_auto_threshold = 6700;\n static int gc_auto_pack_limit = 50;\n+static int detach_auto = 1;\n static const char *prune_expire = \"2.weeks.ago\";\n \n static struct argv_array pack_refs_cmd = ARGV_ARRAY_INIT;\n@@ -73,6 +74,10 @@ static int gc_config(const char *var, const char *value, void *cb)\n \t\tgc_auto_pack_limit = git_config_int(var, value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(var, \"gc.autodetach\")) {\n+\t\tdetach_auto = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \tif (!strcmp(var, \"gc.pruneexpire\")) {\n \t\tif (value && strcmp(value, \"now\")) {\n \t\t\tunsigned long now = approxidate(\"now\");\n@@ -301,11 +306,19 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\t */\n \t\tif (!need_to_gc())\n \t\t\treturn 0;\n-\t\tif (!quiet)\n-\t\t\tfprintf(stderr,\n-\t\t\t\t\t_(\"Auto packing the repository for optimum performance. You may also\\n\"\n-\t\t\t\t\t\"run \\\"git gc\\\" manually. See \"\n-\t\t\t\t\t\"\\\"git help gc\\\" for more information.\\n\"));\n+\t\tif (!quiet) {\n+\t\t\tif (detach_auto)\n+\t\t\t\tfprintf(stderr, _(\"Auto packing the repository in background for optimum performance.\\n\"));\n+\t\t\telse\n+\t\t\t\tfprintf(stderr, _(\"Auto packing the repository for optimum performance.\\n\"));\n+\t\t\tfprintf(stderr, _(\"See \\\"git help gc\\\" for manual housekeeping.\\n\"));\n+\t\t}\n+\t\tif (detach_auto)\n+\t\t\t/*\n+\t\t\t * failure to daemonize is ok, we'll continue\n+\t\t\t * in foreground\n+\t\t\t */\n+\t\t\tdaemonize();\n \t} else\n \t\tadd_repack_all_option();\n \ndiff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh\nindex 129fc88..0736bcb 100755\n--- a/t/t5400-send-pack.sh\n+++ b/t/t5400-send-pack.sh\n@@ -164,6 +164,7 @@ test_expect_success 'receive-pack runs auto-gc in remote repo' '\n \t # Set the child to auto-pack if more than one pack exists\n \t cd child &&\n \t git config gc.autopacklimit 1 &&\n+\t git config gc.autodetach false &&\n \t git branch test_auto_gc &&\n \t # And create a file that follows the temporary object naming\n \t # convention for the auto-gc to remove\n-- \n1.8.5.2.240.g8478abd\n"},{"id":"234575","messageId":"CABPQNSb3=i8F+vPEG3RmH+snZVZ-xrPtcVY2Nx9uvyTCLXcy6g@mail.gmail.com","threadId":"35793","inReplyTo":"1391843332-20583-2-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2 2/2] gc: config option for running --auto in background","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2014-02-10T11:03:08Z","receivedAt":"2014-02-10T11:03:08Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Sat, Feb 8, 2014 at 8:08 AM, Nguyễn Thái Ngọc Duy wrote:\n> `gc --auto` takes time and can block the user temporarily (but not any\n> less annoyingly). Make it run in background on systems that support\n> it. The only thing lost with running in background is printouts. But\n> gc output is not really interesting. You can keep it in foreground by\n> changing gc.autodetach.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> Documentation/config.txt | 4 ++++\n> builtin/gc.c | 23 ++++++++++++++++++-----\n> t/t5400-send-pack.sh | 1 +\n> 3 files changed, 23 insertions(+), 5 deletions(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 5f4d793..4781773 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1167,6 +1167,10 @@ gc.autopacklimit::\n> --auto` consolidates them into one larger pack. The\n> default value is 50. Setting this to 0 disables it.\n>\n> +gc.autodetach::\n> + Make `git gc --auto` return immediately andrun in background\n> + if the system supports it. Default is true.\n> +\n> gc.packrefs::\n> Running `git pack-refs` in a repository renders it\n> unclonable by Git versions prior to 1.5.1.2 over dumb\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index c19545d..ed5cc3c 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -29,6 +29,7 @@ static int pack_refs = 1;\n> static int aggressive_window = 250;\n> static int gc_auto_threshold = 6700;\n> static int gc_auto_pack_limit = 50;\n> +static int detach_auto = 1;\n> static const char *prune_expire = \"2.weeks.ago\";\n>\n> static struct argv_array pack_refs_cmd = ARGV_ARRAY_INIT;\n> @@ -73,6 +74,10 @@ static int gc_config(const char *var, const char *value, void *cb)\n> gc_auto_pack_limit = git_config_int(var, value);\n> return 0;\n> }\n> + if (!strcmp(var, \"gc.autodetach\")) {\n> + detach_auto = git_config_bool(var, value);\n> + return 0;\n> + }\n> if (!strcmp(var, \"gc.pruneexpire\")) {\n> if (value && strcmp(value, \"now\")) {\n> unsigned long now = approxidate(\"now\");\n> @@ -301,11 +306,19 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n> */\n> if (!need_to_gc())\n> return 0;\n> - if (!quiet)\n> - fprintf(stderr,\n> - _(\"Auto packing the repository for optimum performance. You may also\\n\"\n> - \"run \\\"git gc\\\" manually. See \"\n> - \"\\\"git help gc\\\" for more information.\\n\"));\n> + if (!quiet) {\n> + if (detach_auto)\n> + fprintf(stderr, _(\"Auto packing the repository in background for optimum performance.\\n\"));\n> + else\n> + fprintf(stderr, _(\"Auto packing the repository for optimum performance.\\n\"));\n> + fprintf(stderr, _(\"See \\\"git help gc\\\" for manual housekeeping.\\n\"));\n> + }\n> + if (detach_auto)\n> + /*\n> + * failure to daemonize is ok, we'll continue\n> + * in foreground\n> + */\n> + daemonize();\n\nWhile I agree that it should be OK, shouldn't we warn the user?\n"},{"id":"234576","messageId":"CABPQNSaNuCM5mNFF_74OPo+RYqZHgUPpw0k5HbaL=p54j48i4g@mail.gmail.com","threadId":"35793","inReplyTo":"1391843332-20583-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2 1/2] daemon: move daemonize() to libgit.a","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2014-02-10T11:04:58Z","receivedAt":"2014-02-10T11:04:58Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Sat, Feb 8, 2014 at 8:08 AM, Nguyễn Thái Ngọc Duy wrote:\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> diff --git a/setup.c b/setup.c\n> index 6c3f85f..b09a412 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -787,3 +787,27 @@ void sanitize_stdfds(void)\n> if (fd > 2)\n> close(fd);\n> }\n> +\n> +int daemonize(void)\n> +{\n> +#ifdef NO_POSIX_GOODIES\n> + errno = -ENOSYS;\n> + return -1;\n> +#else\n> + switch (fork()) {\n> + case 0:\n> + break;\n> + case -1:\n> + die_errno(\"fork failed\");\n> + default:\n> + exit(0);\n> + }\n> + if (setsid() == -1)\n> + die_errno(\"setsid failed\");\n> + close(0);\n> + close(1);\n> + close(2);\n> + sanitize_stdfds();\n> + return 0;\n> +#endif\n> +}\n\nNice change.\n\nJust a nit: When I added the NO_POSIX_GOODIES-flag, Junio wanted the\nimplementations to be separate.\n"},{"id":"234580","messageId":"CACsJy8BBQ3Bh6q6JM8V-QVKfzwp1w99+u4_55jjGbHLV3c62gA@mail.gmail.com","threadId":"35793","inReplyTo":"CABPQNSb3=i8F+vPEG3RmH+snZVZ-xrPtcVY2Nx9uvyTCLXcy6g@mail.gmail.com","subject":"Re: [PATCH v2 2/2] gc: config option for running --auto in background","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-02-10T13:17:28Z","receivedAt":"2014-02-10T13:17:28Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Feb 10, 2014 at 6:03 PM, Erik Faye-Lund wrote:\n>> `gc --auto` takes time and can block the user temporarily (but not any\n>> - if (!quiet)\n>> - fprintf(stderr,\n>> - _(\"Auto packing the repository for optimum performance. You may also\\n\"\n>> - \"run \\\"git gc\\\" manually. See \"\n>> - \"\\\"git help gc\\\" for more information.\\n\"));\n>> + if (!quiet) {\n>> + if (detach_auto)\n>> + fprintf(stderr, _(\"Auto packing the repository in background for optimum performance.\\n\"));\n>> + else\n>> + fprintf(stderr, _(\"Auto packing the repository for optimum performance.\\n\"));\n>> + fprintf(stderr, _(\"See \\\"git help gc\\\" for manual housekeeping.\\n\"));\n>> + }\n>> + if (detach_auto)\n>> + /*\n>> + * failure to daemonize is ok, we'll continue\n>> + * in foreground\n>> + */\n>> + daemonize();\n>\n> While I agree that it should be OK, shouldn't we warn the user?\n\nIf --quiet is set, we should not be printing anyway. If not, I thinkg\nwe could only print \"auto packing in background..\" when we actually\ncan do that, else just print the old message. It means an #ifdef\nNO_POSIX_GOODIES here again though..\n-- \nDuy\n"},{"id":"234581","messageId":"CABPQNSaLQ4aU+46P5ceS1XAzXQ_4HpURF369fQe6ZuLQmFwZmQ@mail.gmail.com","threadId":"35793","inReplyTo":"CACsJy8BBQ3Bh6q6JM8V-QVKfzwp1w99+u4_55jjGbHLV3c62gA@mail.gmail.com","subject":"Re: [PATCH v2 2/2] gc: config option for running --auto in background","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2014-02-10T13:33:15Z","receivedAt":"2014-02-10T13:33:15Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, Feb 10, 2014 at 2:17 PM, Duy Nguyen wrote:\n> On Mon, Feb 10, 2014 at 6:03 PM, Erik Faye-Lund wrote:\n>>> `gc --auto` takes time and can block the user temporarily (but not any\n>>> - if (!quiet)\n>>> - fprintf(stderr,\n>>> - _(\"Auto packing the repository for optimum performance. You may also\\n\"\n>>> - \"run \\\"git gc\\\" manually. See \"\n>>> - \"\\\"git help gc\\\" for more information.\\n\"));\n>>> + if (!quiet) {\n>>> + if (detach_auto)\n>>> + fprintf(stderr, _(\"Auto packing the repository in background for optimum performance.\\n\"));\n>>> + else\n>>> + fprintf(stderr, _(\"Auto packing the repository for optimum performance.\\n\"));\n>>> + fprintf(stderr, _(\"See \\\"git help gc\\\" for manual housekeeping.\\n\"));\n>>> + }\n>>> + if (detach_auto)\n>>> + /*\n>>> + * failure to daemonize is ok, we'll continue\n>>> + * in foreground\n>>> + */\n>>> + daemonize();\n>>\n>> While I agree that it should be OK, shouldn't we warn the user?\n>\n> If --quiet is set, we should not be printing anyway. If not, I thinkg\n> we could only print \"auto packing in background..\" when we actually\n> can do that, else just print the old message. It means an #ifdef\n> NO_POSIX_GOODIES here again though..\n\nYuck, it's probably better to simply silently drop the detaching, I guess.\n"},{"id":"234589","messageId":"xmqqob2est9c.fsf@gitster.dls.corp.google.com","threadId":"35793","inReplyTo":"CACsJy8BBQ3Bh6q6JM8V-QVKfzwp1w99+u4_55jjGbHLV3c62gA@mail.gmail.com","subject":"Re: [PATCH v2 2/2] gc: config option for running --auto in background","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-10T18:43:43Z","receivedAt":"2014-02-10T18:43:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Mon, Feb 10, 2014 at 6:03 PM, Erik Faye-Lund wrote:\n>>> `gc --auto` takes time and can block the user temporarily (but not any\n>>> - if (!quiet)\n>>> - fprintf(stderr,\n>>> - _(\"Auto packing the repository for optimum performance. You may also\\n\"\n>>> - \"run \\\"git gc\\\" manually. See \"\n>>> - \"\\\"git help gc\\\" for more information.\\n\"));\n>>> + if (!quiet) {\n>>> + if (detach_auto)\n>>> + fprintf(stderr, _(\"Auto packing the repository in background for optimum performance.\\n\"));\n>>> + else\n>>> + fprintf(stderr, _(\"Auto packing the repository for optimum performance.\\n\"));\n>>> + fprintf(stderr, _(\"See \\\"git help gc\\\" for manual housekeeping.\\n\"));\n>>> + }\n>>> + if (detach_auto)\n>>> + /*\n>>> + * failure to daemonize is ok, we'll continue\n>>> + * in foreground\n>>> + */\n>>> + daemonize();\n>>\n>> While I agree that it should be OK, shouldn't we warn the user?\n>\n> If --quiet is set, we should not be printing anyway. If not, I thinkg\n> we could only print \"auto packing in background..\" when we actually\n> can do that, else just print the old message. It means an #ifdef\n> NO_POSIX_GOODIES here again though..\n\nDidn't you change it not to die but return nosys or something?\n"},{"id":"234590","messageId":"xmqqha86st5f.fsf@gitster.dls.corp.google.com","threadId":"35793","inReplyTo":"1391843332-20583-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2 1/2] daemon: move daemonize() to libgit.a","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-10T18:46:04Z","receivedAt":"2014-02-10T18:46:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> diff --git a/setup.c b/setup.c\n> index 6c3f85f..b09a412 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -787,3 +787,27 @@ void sanitize_stdfds(void)\n> \tif (fd > 2)\n> \t\tclose(fd);\n> }\n> +\n> +int daemonize(void)\n> +{\n> +#ifdef NO_POSIX_GOODIES\n> +\terrno = -ENOSYS;\n\nNegated?\n\n> +\treturn -1;\n> +#else\n> +\tswitch (fork()) {\n> +\t\tcase 0:\n> +\t\t\tbreak;\n> +\t\tcase -1:\n> +\t\t\tdie_errno(\"fork failed\");\n> +\t\tdefault:\n> +\t\t\texit(0);\n> +\t}\n> +\tif (setsid() == -1)\n> +\t\tdie_errno(\"setsid failed\");\n> +\tclose(0);\n> +\tclose(1);\n> +\tclose(2);\n> +\tsanitize_stdfds();\n> +\treturn 0;\n> +#endif\n> +}\n"},{"id":"234592","messageId":"CAPc5daW3VwLutU8JZu9fBbGtihw5X_bE9M31ugzqN9mEnFYNNQ@mail.gmail.com","threadId":"35793","inReplyTo":"xmqqob2est9c.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 2/2] gc: config option for running --auto in background","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-10T19:11:07Z","receivedAt":"2014-02-10T19:11:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Mon, Feb 10, 2014 at 10:43 AM, Junio C Hamano wrote:\n>> If --quiet is set, we should not be printing anyway. If not, I thinkg\n>> we could only print \"auto packing in background..\" when we actually\n>> can do that, else just print the old message. It means an #ifdef\n>> NO_POSIX_GOODIES here again though..\n>\n> Didn't you change it not to die but return nosys or something?\n\nAh, the problem is that it is too late to take back \"... will do so in\nthe background\" when you noticed that daemonize() did not succeed, so\nyou would need a way to see if we can daemonize() before actually\ndoing so if you want to give different messages.\n\n\"int can_daemonize(void)\" could be an answer that is nicer than\nNO_POSIX_GOODIES, but I am not sure if it is worth it.\n"},{"id":"234601","messageId":"CACsJy8Bq046c19T3RNQCvDHp8dA-Si_8k=R230ZHODiVu-1dZw@mail.gmail.com","threadId":"35793","inReplyTo":"xmqqha86st5f.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v2 1/2] daemon: move daemonize() to libgit.a","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-02-10T23:25:28Z","receivedAt":"2014-02-10T23:25:28Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Feb 11, 2014 at 1:46 AM, Junio C Hamano wrote:\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> diff --git a/setup.c b/setup.c\n>> index 6c3f85f..b09a412 100644\n>> --- a/setup.c\n>> +++ b/setup.c\n>> @@ -787,3 +787,27 @@ void sanitize_stdfds(void)\n>> if (fd > 2)\n>> close(fd);\n>> }\n>> +\n>> +int daemonize(void)\n>> +{\n>> +#ifdef NO_POSIX_GOODIES\n>> + errno = -ENOSYS;\n>\n> Negated?\n\nFacepalm. I remember I wrote this somewhere but don't remember what\ntopic :( Should I resend?\n-- \nDuy\n"},{"id":"234620","messageId":"xmqq61olr08b.fsf@gitster.dls.corp.google.com","threadId":"35793","inReplyTo":"CACsJy8Bq046c19T3RNQCvDHp8dA-Si_8k=R230ZHODiVu-1dZw@mail.gmail.com","subject":"Re: [PATCH v2 1/2] daemon: move daemonize() to libgit.a","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-11T18:08:20Z","receivedAt":"2014-02-11T18:08:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Tue, Feb 11, 2014 at 1:46 AM, Junio C Hamano wrote:\n>> Nguyễn Thái Ngọc Duy writes:\n>>\n>>> diff --git a/setup.c b/setup.c\n>>> index 6c3f85f..b09a412 100644\n>>> --- a/setup.c\n>>> +++ b/setup.c\n>>> @@ -787,3 +787,27 @@ void sanitize_stdfds(void)\n>>> if (fd > 2)\n>>> close(fd);\n>>> }\n>>> +\n>>> +int daemonize(void)\n>>> +{\n>>> +#ifdef NO_POSIX_GOODIES\n>>> + errno = -ENOSYS;\n>>\n>> Negated?\n>\n> Facepalm. I remember I wrote this somewhere but don't remember what\n> topic :( Should I resend?\n\nI've already amended it here.\n"},{"id":"234635","messageId":"CACsJy8CyAsA5VA-s_c7juaRyToRU+QzJsYZDvzD5ggaR6=9FOg@mail.gmail.com","threadId":"35793","inReplyTo":"CAPc5daW3VwLutU8JZu9fBbGtihw5X_bE9M31ugzqN9mEnFYNNQ@mail.gmail.com","subject":"Re: [PATCH v2 2/2] gc: config option for running --auto in background","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-02-12T01:53:05Z","receivedAt":"2014-02-12T01:53:05Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Feb 11, 2014 at 2:11 AM, Junio C Hamano wrote:\n> On Mon, Feb 10, 2014 at 10:43 AM, Junio C Hamano wrote:\n>>> If --quiet is set, we should not be printing anyway. If not, I thinkg\n>>> we could only print \"auto packing in background..\" when we actually\n>>> can do that, else just print the old message. It means an #ifdef\n>>> NO_POSIX_GOODIES here again though..\n>>\n>> Didn't you change it not to die but return nosys or something?\n>\n> Ah, the problem is that it is too late to take back \"... will do so in\n> the background\" when you noticed that daemonize() did not succeed, so\n> you would need a way to see if we can daemonize() before actually\n> doing so if you want to give different messages.\n>\n> \"int can_daemonize(void)\" could be an answer that is nicer than\n> NO_POSIX_GOODIES, but I am not sure if it is worth it.\n\nOr we could pass the \"quiet\" flag to daemonize() and let it print\nsomething in the #ifdef NO_POSIX_GOODIES part.\n-- \nDuy\n"},{"id":"234666","messageId":"xmqqmwhwnsgz.fsf@gitster.dls.corp.google.com","threadId":"35793","inReplyTo":"CACsJy8CyAsA5VA-s_c7juaRyToRU+QzJsYZDvzD5ggaR6=9FOg@mail.gmail.com","subject":"Re: [PATCH v2 2/2] gc: config option for running --auto in background","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-12T17:36:28Z","receivedAt":"2014-02-12T17:36:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Tue, Feb 11, 2014 at 2:11 AM, Junio C Hamano wrote:\n>> On Mon, Feb 10, 2014 at 10:43 AM, Junio C Hamano wrote:\n>>>> If --quiet is set, we should not be printing anyway. If not, I thinkg\n>>>> we could only print \"auto packing in background..\" when we actually\n>>>> can do that, else just print the old message. It means an #ifdef\n>>>> NO_POSIX_GOODIES here again though..\n>>>\n>>> Didn't you change it not to die but return nosys or something?\n>>\n>> Ah, the problem is that it is too late to take back \"... will do so in\n>> the background\" when you noticed that daemonize() did not succeed, so\n>> you would need a way to see if we can daemonize() before actually\n>> doing so if you want to give different messages.\n>>\n>> \"int can_daemonize(void)\" could be an answer that is nicer than\n>> NO_POSIX_GOODIES, but I am not sure if it is worth it.\n>\n> Or we could pass the \"quiet\" flag to daemonize() and let it print\n> something in the #ifdef NO_POSIX_GOODIES part.\n\nHmph... What would that something say? \"I was asked to gc in the\nbackground but I can't here\" is not suitable for daemonize() that is\nnot specific to \"gc\".\n\nThe flow I had in mind was something along the lines of this\n\n\tif (!quiet) {\n \tif (detach_auto && can_daemonize())\n \tsay \"auto packing in the background\";\n\t\telse\n \tsay \"auto packing\"\n\t}\n if (detach_auto && can_daemonize())\n \tdaemonize();\n\nIf we had daemonize(noisy=1) and coded it this way:\n\n\tif (!quiet)\n \tsay \"auto packing\";\n\tif (detach_auto)\n \tdaemonize(!quiet);\n\nwe could do something like:\n\n\tdaemonize(int noisy) {\n \tif (noisy && !defined(NO_POSIX_GOODIES))\n \tsay \", and doing so in the background\";\n\t\t... do the actual daemonizing ...\n\t}\n\nbut that feels ugly.\n"}]}