{"thread":{"id":"37453","subject":"Auto Packing message twice?","startedAt":"2014-08-30T11:28:47Z","lastAt":"2014-08-31T04:26:15Z","messageCount":3,"participants":["Stefan Beller","Duy Nguyen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"248645","messageId":"5401B56F.6000907@gmail.com","threadId":"37453","inReplyTo":null,"subject":"Auto Packing message twice?","fromName":"Stefan Beller","fromEmail":"stefanbeller@gmail.com","sentAt":"2014-08-30T11:28:47Z","receivedAt":"2014-08-30T11:28:47Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Hi,\n\nso I fetched from a repository today and I got the message twice:\n\n> Auto packing the repository in background for optimum performance.\n> See \"git help gc\" for manual housekeeping.\n\nI'm running git version 2.1.0.rc2\n\nFull output:\n\n$ git fetch --all --prune\nFetching origin\nFetching mainline\nremote: Counting objects: 832, done.\nremote: Compressing objects: 100% (332/332), done.\nremote: Total 832 (delta 593), reused 659 (delta 498)\nReceiving objects: 100% (832/832), 955.44 KiB | 660.00 KiB/s, done.\nResolving deltas: 100% (593/593), done.\nFrom git://github.com/danmar/cppcheck\n 7237b01..eace67e master -> mainline/master\n * [new tag] 1.66 -> 1.66\nAuto packing the repository in background for optimum performance.\nSee \"git help gc\" for manual housekeeping.\nAuto packing the repository in background for optimum performance.\nSee \"git help gc\" for manual housekeeping.\n\nObviously it should only report once?\nThe reporting is done from within function cmd_gc(...),\nwhich makes me wonder, if it also packs twice?\n\nThanks,\nStefan\n"},{"id":"248649","messageId":"CACsJy8Ch5WW2rF4vGrM2xyFqn2-1FgPw-kO_wLtBY8-+WqM-1g@mail.gmail.com","threadId":"37453","inReplyTo":"5401B56F.6000907@gmail.com","subject":"Re: Auto Packing message twice?","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-08-30T14:12:57Z","receivedAt":"2014-08-30T14:12:57Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Aug 30, 2014 at 6:28 PM, Stefan Beller wrote:\n> Auto packing the repository in background for optimum performance.\n> See \"git help gc\" for manual housekeeping.\n> Auto packing the repository in background for optimum performance.\n> See \"git help gc\" for manual housekeeping.\n>\n> Obviously it should only report once?\n> The reporting is done from within function cmd_gc(...),\n> which makes me wonder, if it also packs twice?\n\nThe problem is we print this message before checking if gc is running\nin background. Problem is we keep the lockafter forking/detaching\nbecause locking before forking means two processes holding the lock\nand I don't think there's a way to say \"release the lock in parent\nprocess\". And we can't print this message after detaching either\nbecause stdout is already closed then. But this definitely needs some\nimprovement to avoid confusion.\n-- \nDuy\n"},{"id":"248672","messageId":"20140831042615.GA7862@lanh","threadId":"37453","inReplyTo":"CACsJy8Ch5WW2rF4vGrM2xyFqn2-1FgPw-kO_wLtBY8-+WqM-1g@mail.gmail.com","subject":"Re: Auto Packing message twice?","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-08-31T04:26:15Z","receivedAt":"2014-08-31T04:26:15Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Aug 30, 2014 at 09:12:57PM +0700, Duy Nguyen wrote:\n> On Sat, Aug 30, 2014 at 6:28 PM, Stefan Beller wrote:\n> > Auto packing the repository in background for optimum performance.\n> > See \"git help gc\" for manual housekeeping.\n> > Auto packing the repository in background for optimum performance.\n> > See \"git help gc\" for manual housekeeping.\n> >\n> > Obviously it should only report once?\n> > The reporting is done from within function cmd_gc(...),\n> > which makes me wonder, if it also packs twice?\n> \n> The problem is we print this message before checking if gc is running\n> in background. Problem is we keep the lockafter forking/detaching\n> because locking before forking means two processes holding the lock\n> and I don't think there's a way to say \"release the lock in parent\n> process\".\n\nI'm wrong. It's more about the gc.pid update code, which should record\nthe running pid (i.e. after fork), so I can't really move\nlock_repo_for_gc() code to the parent process. If we just peek ahead\nin gc.pid to see if any gc is already running, there's a chance that a\nnew gc will start after the check, and before we update gc.pid. So\nstill false messages.\n\n> And we can't print this message after detaching either\n> because stdout is already closed then. But this definitely needs some\n> improvement to avoid confusion.\n\nIf we do not use daemonized() then it's possible. Though\nNO_POSIX_GOODIES is now sprinkled in more places.\n\n-- 8< --\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 8d219d8..f04b5dc 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -280,6 +280,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \tint force = 0;\n \tconst char *name;\n \tpid_t pid;\n+\tint forked = 0;\n \n \tstruct option builtin_gc_options[] = {\n \t\tOPT__QUIET(&quiet, N_(\"suppress progress reporting\")),\n@@ -327,21 +328,30 @@ 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\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\tif (!quiet && !detach_auto) {\n+\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\tif (gc_before_repack())\n \t\t\t\treturn -1;\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+#ifndef NO_POSIX_GOODIES\n+\t\t\tswitch (fork()) {\n+\t\t\tcase 0:\n+\t\t\t\tforked = 1;\n+\t\t\t\tbreak;\n+\n+\t\t\tcase -1:\n+\t\t\t\t/*\n+\t\t\t\t * failure to daemonize is ok, we'll continue\n+\t\t\t\t * in foreground\n+\t\t\t\t */\n+\t\t\t\tbreak;\n+\n+\t\t\tdefault:\n+\t\t\t\texit(0);\n+\t\t\t}\n+#endif\n \t\t}\n \t} else\n \t\tadd_repack_all_option();\n@@ -354,6 +364,23 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\t name, (uintmax_t)pid);\n \t}\n \n+\tif (auto_gc && !quiet && detach_auto) {\n+\t\tif (forked)\n+\t\t\tfprintf(stderr, _(\"Auto packing the repository in background for optimum performance.\\n\"));\n+\t\telse\n+\t\t\tfprintf(stderr, _(\"Auto packing the repository for optimum performance.\\n\"));\n+\t\tfprintf(stderr, _(\"See \\\"git help gc\\\" for manual housekeeping.\\n\"));\n+\t}\n+#ifndef NO_POSIX_GOODIES\n+\tif (forked) {\n+\t\tif (setsid() == -1)\n+\t\t\tdie_errno(\"setsid failed\");\n+\t\tclose(0);\n+\t\tclose(1);\n+\t\tclose(2);\n+\t\tsanitize_stdfds();\n+\t}\n+#endif\n \tif (gc_before_repack())\n \t\treturn -1;\n \n-- 8< --\n"}]}