{"thread":{"id":"12943","subject":"Re: [PATCH 1/4] git-gc --auto: add pre-auto-gc hook","startedAt":"2008-04-01T04:51:12Z","lastAt":"2008-04-04T06:34:39Z","messageCount":21,"participants":["Junio C Hamano","Miklos Vajna","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"73478","messageId":"7vsky6jhz3.fsf@gitster.siamese.dyndns.org","threadId":"12943","inReplyTo":null,"subject":"Re: [PATCH 1/4] git-gc --auto: add pre-auto-gc hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-04-01T04:51:12Z","receivedAt":"2008-04-01T04:51:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna <vmiklos@frugalware.org> writes:\n\n> If such a hook is available and exits with a non-zero status, then\n> git-gc --auto won't run.\n>\n> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>\n> ---\n>  builtin-gc.c |   22 ++++++++++++++++++++++\n>  1 files changed, 22 insertions(+), 0 deletions(-)\n>\n> diff --git a/builtin-gc.c b/builtin-gc.c\n> index 8cef36f..acd63be 100644\n> --- a/builtin-gc.c\n> +++ b/builtin-gc.c\n> @@ -157,6 +157,25 @@ static int too_many_packs(void)\n>  \treturn gc_auto_pack_limit <= cnt;\n>  }\n>  \n> +static int run_hook()\n> +{\n> +\tconst char *argv[2];\n> +\tstruct child_process hook;\n> +\n> +\targv[0] = git_path(\"hooks/pre-auto-gc\");\n> +\targv[1] = NULL;\n\nHmm.  I wonder if the hook wants any parameters, even though I do not\nthink of any offhand.\n\n> +\tif (access(argv[0], X_OK) < 0)\n> +\t\treturn 0;\n> +\n> +\tmemset(&hook, 0, sizeof(hook));\n> +\thook.argv = argv;\n> +\thook.no_stdin = 1;\n> +\thook.stdout_to_stderr = 1;\n\nI understand no_stdin, but is there a reason to do stdout_to_stderr?\n\n> +\treturn run_command(&hook);\n> +}\n\nDon't we want to distinguish between the case where start_command()\nfailed, wait_or_whine() failed on waitpid(), the command was killed with\nsignal, or the command actually ran correctly and decided that you should\nnot run \"git gc --auto\" by exiting non-zero?\n\nI think it is prudent to refrain from running \"git gc --auto\" in any of\nthe failure cases I listed above, but shouldn't the cases other than the\nlast one at least issue a warning?\n"},{"id":"73497","messageId":"cover.1207049697.git.vmiklos@frugalware.org","threadId":"12943","inReplyTo":"7vsky6jhz3.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 0/4] add pre-auto-gc hook for git-gc --auto (try2)","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-04-01T11:38:55Z","receivedAt":"2008-04-01T11:38:55Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Mon, Mar 31, 2008 at 09:51:12PM -0700, Junio C Hamano <gitster@pobox.com> wrote:\n> > +   return run_command(&hook);\n> > +}\n>\n> Don't we want to distinguish between the case where start_command()\n> failed, wait_or_whine() failed on waitpid(), the command was killed\n> with\n> signal, or the command actually ran correctly and decided that you\n> should\n> not run \"git gc --auto\" by exiting non-zero?\n>\n> I think it is prudent to refrain from running \"git gc --auto\" in any\n> of\n> the failure cases I listed above, but shouldn't the cases other than\n> the\n> last one at least issue a warning?\n\nOk, there are 3 cases here to handle. When wait_or_whine() fails on\nwaitpid() it already prints an error, so that's already handled. I've\nadded two warnings for the other 2 cases.\n\nOther changes:\n\n- try to use on_ac_power when it's available, as suggested by Joey Hess\n\n- mention in the commend of the example pre-auto-gc hook that it's\n  Linux-specific, as suggested by Brian Gernhardt\n\n- removed mentioning what the default hook does from hooks.txt, as it's\n  an example and it's under contrib/\n\n- moved the battery example to contrib/ and added a minimal example to\n  templates/\n\n- removed unnecessary stdout_to_stderr from builtin-gc.c::run_hook()\n\n- removed unnecessary --no-verify option\n\nI hope I haven't missed anything you suggested.\n\nMiklos Vajna (4):\n  git-gc --auto: add pre-auto-gc hook\n  Documentation/hooks: add pre-auto-gc hook\n  templates: add an example pre-auto-gc hook\n  contrib/hooks: add an example pre-auto-gc hook\n\n Documentation/hooks.txt           |    7 +++++\n builtin-gc.c                      |   30 ++++++++++++++++++++++++\n contrib/hooks/pre-auto-gc-battery |   45 +++++++++++++++++++++++++++++++++++++\n templates/hooks--pre-auto-gc      |    9 +++++++\n 4 files changed, 91 insertions(+), 0 deletions(-)\n create mode 100644 contrib/hooks/pre-auto-gc-battery\n create mode 100644 templates/hooks--pre-auto-gc\n"},{"id":"73498","messageId":"a402ae0587d99797b7fb48d7f885add1aaba45e4.1207049697.git.vmiklos@frugalware.org","threadId":"12943","inReplyTo":"7vsky6jhz3.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 1/4] git-gc --auto: add pre-auto-gc hook","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-04-01T11:39:12Z","receivedAt":"2008-04-01T11:39:12Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"If such a hook is available and exits with a non-zero status, then\ngit-gc --auto won't run.\n\nSigned-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n builtin-gc.c |   30 ++++++++++++++++++++++++++++++\n 1 files changed, 30 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-gc.c b/builtin-gc.c\nindex 8cef36f..6561639 100644\n--- a/builtin-gc.c\n+++ b/builtin-gc.c\n@@ -157,6 +157,33 @@ static int too_many_packs(void)\n \treturn gc_auto_pack_limit <= cnt;\n }\n \n+static int run_hook()\n+{\n+\tconst char *argv[2];\n+\tstruct child_process hook;\n+\tint ret;\n+\n+\targv[0] = git_path(\"hooks/pre-auto-gc\");\n+\targv[1] = NULL;\n+\n+\tif (access(argv[0], X_OK) < 0)\n+\t\treturn 0;\n+\n+\tmemset(&hook, 0, sizeof(hook));\n+\thook.argv = argv;\n+\thook.no_stdin = 1;\n+\n+\tret = start_command(&hook);\n+\tif (ret) {\n+\t\twarning(\"Could not spawn %s\", argv[0]);\n+\t\treturn ret;\n+\t}\n+\tret = finish_command(&hook);\n+\tif (ret == -ERR_RUN_COMMAND_WAITPID_SIGNAL)\n+\t\twarning(\"%s exited due to uncaught signal\", argv[0]);\n+\treturn ret;\n+}\n+\n static int need_to_gc(void)\n {\n \t/*\n@@ -176,6 +203,9 @@ static int need_to_gc(void)\n \t\tappend_option(argv_repack, \"-A\", MAX_ADD);\n \telse if (!too_many_loose_objects())\n \t\treturn 0;\n+\n+\tif (run_hook())\n+\t\treturn 0;\n \treturn 1;\n }\n \n-- \n1.5.5.rc2.4.gec07.dirty\n"},{"id":"73499","messageId":"bc9822eda81de116572b756b7f989af602df27e3.1207049697.git.vmiklos@frugalware.org","threadId":"12943","inReplyTo":"7vsky6jhz3.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 2/4] Documentation/hooks: add pre-auto-gc hook","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-04-01T11:39:27Z","receivedAt":"2008-04-01T11:39:27Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n Documentation/hooks.txt |    7 +++++++\n 1 files changed, 7 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/hooks.txt b/Documentation/hooks.txt\nindex 76b8d77..44fbe58 100644\n--- a/Documentation/hooks.txt\n+++ b/Documentation/hooks.txt\n@@ -276,3 +276,10 @@ probably enable this hook.\n Both standard output and standard error output are forwarded to\n `git-send-pack` on the other end, so you can simply `echo` messages\n for the user.\n+\n+pre-auto-gc\n+-----------\n+\n+This hook is invoked by `git-gc --auto`. It takes no parameter, and\n+exiting with non-zero status from this script causes the `git-gc --auto`\n+to abort.\n-- \n1.5.5.rc2.4.gec07.dirty\n"},{"id":"73500","messageId":"3ab373c8a2bbe5d682852050fe0692c78fcc151b.1207049697.git.vmiklos@frugalware.org","threadId":"12943","inReplyTo":"7vsky6jhz3.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 3/4] templates: add an example pre-auto-gc hook","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-04-01T11:39:43Z","receivedAt":"2008-04-01T11:39:43Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"it's empty by default.\n\nSigned-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n templates/hooks--pre-auto-gc |    9 +++++++++\n 1 files changed, 9 insertions(+), 0 deletions(-)\n create mode 100644 templates/hooks--pre-auto-gc\n\ndiff --git a/templates/hooks--pre-auto-gc b/templates/hooks--pre-auto-gc\nnew file mode 100644\nindex 0000000..8a9bf37\n--- /dev/null\n+++ b/templates/hooks--pre-auto-gc\n@@ -0,0 +1,9 @@\n+#!/bin/sh\n+#\n+# An example hook that is called by git-gc --auto with no arguments. The\n+# hook should exit with non-zero status after issuing an appropriate\n+# message if it wants to stop the auto repacking.\n+#\n+# To enable this hook, make this file executable.\n+\n+: Nothing\n-- \n1.5.5.rc2.4.gec07.dirty\n"},{"id":"73501","messageId":"52c671c64b2f203793ee88a84fee6f83cba2a4ff.1207049697.git.vmiklos@frugalware.org","threadId":"12943","inReplyTo":"7vsky6jhz3.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 4/4] contrib/hooks: add an example pre-auto-gc hook","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-04-01T11:39:59Z","receivedAt":"2008-04-01T11:39:59Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"It disables git-gc --auto when you are running Linux and you are on\nbattery.\n\nSigned-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n contrib/hooks/pre-auto-gc-battery |   45 +++++++++++++++++++++++++++++++++++++\n 1 files changed, 45 insertions(+), 0 deletions(-)\n create mode 100644 contrib/hooks/pre-auto-gc-battery\n\ndiff --git a/contrib/hooks/pre-auto-gc-battery b/contrib/hooks/pre-auto-gc-battery\nnew file mode 100644\nindex 0000000..4077111\n--- /dev/null\n+++ b/contrib/hooks/pre-auto-gc-battery\n@@ -0,0 +1,45 @@\n+#!/bin/sh\n+#\n+# An example hook script to verify if you are on battery, in case you\n+# are running Linux. Called by git-gc --auto with no arguments. The hook\n+# should exit with non-zero status after issuing an appropriate message\n+# if it wants to stop the auto repacking.\n+#\n+# This hook is stored in the contrib/hooks directory. Your distribution\n+# will have put this somewhere standard. You should make this script\n+# executable then link to it in the repository you would like to use it\n+# in.\n+#\n+# For example, if the hook is stored in\n+# /usr/share/git-core/contrib/hooks/pre-auto-gc-battery:\n+#\n+# chmod a+x pre-auto-gc-battery\n+# cd /path/to/your/repository.git\n+# ln -sf /usr/share/git-core/contrib/hooks/pre-auto-gc-battery \\\n+#\thooks/pre-auto-gc-battery\n+\n+defer=0\n+\n+if [ -e /sbin/on_ac_power ]; then\n+\t/sbin/on_ac_power\n+\tif [ $? = 1 ]; then\n+\t\tdefer = 1\n+\tfi\n+elif [ -e /sys/class/power_supply/AC/online ]; then\n+\tif [ \"`cat /sys/class/power_supply/AC/online`\" = 0 ]; then\n+\t\tdefer=1\n+\tfi\n+elif [ -e /proc/acpi/ac_adapter/AC/state ]; then\n+\tif grep -q 'off-line' /proc/acpi/ac_adapter/AC/state; then\n+\t\tdefer=1\n+\tfi\n+elif [ -e /proc/apm ]; then\n+\tif grep -q '0$' /proc/apm; then\n+\t\tdefer=1\n+\tfi\n+fi\n+\n+if [ \"$defer\" = 1 ]; then\n+\techo \"Auto packing deferred; on battery\"\n+\texit 1\n+fi\n-- \n1.5.5.rc2.4.gec07.dirty\n"},{"id":"73520","messageId":"7vhceldv12.fsf@gitster.siamese.dyndns.org","threadId":"12943","inReplyTo":"cover.1207049697.git.vmiklos@frugalware.org","subject":"Re: [PATCH 0/4] add pre-auto-gc hook for git-gc --auto (try2)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-04-01T23:18:01Z","receivedAt":"2008-04-01T23:18:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna <vmiklos@frugalware.org> writes:\n\n> - removed unnecessary stdout_to_stderr from builtin-gc.c::run_hook()\n\nWait, a, minute, please.  Not so fast.  I did not say \"I think this is\nwrong, please fix it\".\n\nI only asked why it is so.\n\nCare to explain why it was thought to be necessary, and why you now think\nit is unnecessary?\n\nThis comment is not just for you, but when I ask why it is so, I expect\nthat the contributor would do the homework and present an argument to\ndefend what was submit (or \"it was wrong for such and such reasons so\nlet's do it differently like this because this is better for such and such\nreasons\").  The original contributor has thought about the issues much\nlonger and deeper than I, who only just look at the email and kibitz,\nafter the contribution was perfected into a patch form.  There must have\nbeen a lot more thinking than what can be seen in the submitted material\nin the contributor's head.  I want to see that in the open.  Please do not\nchange your mind and retract your change without explaining why.\n\nThe argument to respond to such \"why is it so?\" question would perhaps\nlook like this for this case.\n\n(begin example #1)\n\nWell, I just cut and pasted from existing code.  Because all these hooks\nredirect their stdout to stderr, I think it is consistent to do so with\nthe new hook.\n\n(example #1 ends here)\n\n(begin example #2)\n\n\"git grep -n -e stdout_to_stderr\" followed by \"git blame\" reveals the\nfollowing pattern:\n\n - builtin-checkout.c:41\n   782c2d6 (Build in checkout, 2008-02-07)\n\n - builtin-commit:382\n   f5bbc32 (Port git commit to C., 2007-11-08)\n\n - help.c:62\n   6494998 (help: add \"man.viewer\" config var to use \"woman\" or \"konqueror\", 2008-03-07)\n\n - receive-pack.c:120,157\n   f43cd49 (Change {pre,post}-receive hooks to use stdin, 2007-03-10)\n   1d9e8b5 (Split back out update_hook handling in receive-pack, 2007-03-10)\n\n - transport.c:165,218,297\n   cd547b4 (fetch/push: readd rsync support, 2007-10-01)\n\nI think all of these are the result of cut&paste from the original one in\nreceive-pack.c's hook handling.\n\nThe ones in the protocol handler may not write to the standard output,\nbecause the other end does not expect random diagnostic string in the\nprotocol message at the point these hooks are called.  But the other ones\ncould go either way.\n\nHowever, because existing hooks all show their script output to stderr, it\nwould make sense for this new pre-auto-gc hook to do so as well from the\nconsistency viewpoint.  So use of stdout_to_stderr is the right thing to\ndo here.\n\n(example #2 ends here)\n"},{"id":"73524","messageId":"20080402011447.GO3264@genesis.frugalware.org","threadId":"12943","inReplyTo":"7vhceldv12.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 0/4] add pre-auto-gc hook for git-gc --auto (try2)","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-04-02T01:14:47Z","receivedAt":"2008-04-02T01:14:47Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Tue, Apr 01, 2008 at 04:18:01PM -0700, Junio C Hamano <gitster@pobox.com> wrote:\n> > - removed unnecessary stdout_to_stderr from builtin-gc.c::run_hook()\n> \n> Wait, a, minute, please.  Not so fast.  I did not say \"I think this is\n> wrong, please fix it\".\n> \n> I only asked why it is so.\n> \n> Care to explain why it was thought to be necessary, and why you now think\n> it is unnecessary?\n\nOk, let me explain. I think it would be logical to put it to stderr, as\nthe other gc --auto messages are there, as well. See the fprintf() in\ncmd_gc().\n\nThough, I don't think it's that important, so I thought if you think\nit's unnecessary, I would not argue for it.\n\nSo may I put it back? :)\n\nAlso, is the other parts of the series looks correct?\n\nThanks\n"},{"id":"73530","messageId":"7vwsngaoqg.fsf@gitster.siamese.dyndns.org","threadId":"12943","inReplyTo":"20080402011447.GO3264@genesis.frugalware.org","subject":"Re: [PATCH 0/4] add pre-auto-gc hook for git-gc --auto (try2)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-04-02T04:02:15Z","receivedAt":"2008-04-02T04:02:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna <vmiklos@frugalware.org> writes:\n\n> Ok, let me explain. I think it would be logical to put it to stderr, as\n> the other gc --auto messages are there, as well. See the fprintf() in\n> cmd_gc().\n>\n> Though, I don't think it's that important, so I thought if you think\n> it's unnecessary, I would not argue for it.\n\nAs I made it clear already, I do _not_ think when I kibitz.  *You* think,\nconvinced others will say \"Yeah, that's a good idea\", I get convinced and\nthe patch is applied.\n\n> So may I put it back? :)\n\nI agree that it makes sense to send the output to standard error for\nconsistency.  Many existing hooks called from scripted versions of tools\nseem to contaminate the standard output, though.  That would be a good\npost 1.5.5 clean-up, perhaps.\n\n> Also, is the other parts of the series looks correct?\n\nI do not think we would want empty templates/hooks--pre-gc-auto file.\n\n+#!/bin/sh\n+#\n+# An example hook script to verify if you are on battery, in case you\n+# are running Linux. Called by git-gc --auto with no arguments. The hook\n+# should exit with non-zero status after issuing an appropriate message\n+# if it wants to stop the auto repacking.\n+#\n+# This hook is stored in the contrib/hooks directory. Your distribution\n+# will have put this somewhere standard. You should make this script\n\ns/will/may/; s/standard/else/; s/You/If you want to use this hook, you/\n\n+# executable then link to it in the repository you would like to use it\n+# in.\n+#\n+# For example, if the hook is stored in\n+# /usr/share/git-core/contrib/hooks/pre-auto-gc-battery:\n+#\n+# chmod a+x pre-auto-gc-battery\n+# cd /path/to/your/repository.git\n+# ln -sf /usr/share/git-core/contrib/hooks/pre-auto-gc-battery \\\n+#\thooks/pre-auto-gc-battery\n\ns|\\(/pre-auto-gc\\)-battery|\\1|;\n\n+defer=0\n+\n+if [ -e /sbin/on_ac_power ]; then\n+\t/sbin/on_ac_power\n+\tif [ $? = 1 ]; then\n+\t\tdefer = 1\n+\tfi\n\n        if test -x /sbin/on_ac_power && /sbin/on_ac_power\n        then\n                exit 0\n\nVariable assignment in Bourne shell does not allow SP around '='.\n\nI suspect it would be cleaner to invert the logic and \"exit 0\" when you\nknow you are on AC, and fall through and err on the safer side\n(i.e. decline) when you cannot tell for certain.  You can lose the\nvariable defer that way.\n\n+elif [ -e /sys/class/power_supply/AC/online ]; then\n+\tif [ \"`cat /sys/class/power_supply/AC/online`\" = 0 ]; then\n+\t\tdefer=1\n+\tfi\n\n\telif test \"$(cat /sys/class/power_supply/AC/online 2>/dev/null)\" = 1\n\tthen\n        \texit 0\n\nNo need to have two separate tests.  I do not know if \"= 1\" is correct,\nthough.\n\n+elif [ -e /proc/acpi/ac_adapter/AC/state ]; then\n+\tif grep -q 'off-line' /proc/acpi/ac_adapter/AC/state; then\n+\t\tdefer=1\n+\tfi\n\n\telif grep -q 'on-line' /proc/acpi/ac_adapter/AC/state 2>/dev/null\n        then\n        \texit 0\n\nLikewise.\n\n+elif [ -e /proc/apm ]; then\n+\tif grep -q '0$' /proc/apm; then\n+\t\tdefer=1\n+\tfi\n+fi\n\n\telif grep -q '0x01$' /proc/apm 2>/dev/null\n        then\n        \texit 0\n\nLikewise.\n"},{"id":"73553","messageId":"20080402190240.GV3264@genesis.frugalware.org","threadId":"12943","inReplyTo":"7vwsngaoqg.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 0/4] add pre-auto-gc hook for git-gc --auto (try2)","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-04-02T19:02:40Z","receivedAt":"2008-04-02T19:02:40Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Tue, Apr 01, 2008 at 09:02:15PM -0700, Junio C Hamano <gitster@pobox.com> wrote:\n> > So may I put it back? :)\n> \n> I agree that it makes sense to send the output to standard error for\n> consistency.  Many existing hooks called from scripted versions of tools\n> seem to contaminate the standard output, though.  That would be a good\n> post 1.5.5 clean-up, perhaps.\n\nOk, I will put it back.\n\n> > Also, is the other parts of the series looks correct?\n> \n> I do not think we would want empty templates/hooks--pre-gc-auto file.\n\nI did it this way as we have such an empty post-commit hook as well, so\nI added it for consistency. Though it's true that for example\npost-applypatch doesn't have such an empty hook under templates either.\n\nWhat is your opinion here?\n\nPossibilities I see:\n\n1) Just don't add such an empty template for pre-auto-gc.\n\n2) Remove post-commit as well.\n\n3) Add missing empty templates, like post-applypatch (maybe there are\nmore, I haven't did a complete research).\n\nI've did the rest of the modifications locally as you suggested just I\ndon't want to spam the list with the series before the empty templates\nquestion is not clear to me :)\n\nThanks\n"},{"id":"73554","messageId":"7vtzik848t.fsf@gitster.siamese.dyndns.org","threadId":"12943","inReplyTo":"20080402190240.GV3264@genesis.frugalware.org","subject":"Re: [PATCH 0/4] add pre-auto-gc hook for git-gc --auto (try2)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-04-02T19:07:46Z","receivedAt":"2008-04-02T19:07:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna <vmiklos@frugalware.org> writes:\n\n> What is your opinion here?\n>\n> 1) Just don't add such an empty template for pre-auto-gc.\n"},{"id":"73556","messageId":"cover.1207164676.git.vmiklos@frugalware.org","threadId":"12943","inReplyTo":"7vtzik848t.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 0/3] add pre-auto-gc hook for git-gc --auto (try3)","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-04-02T19:34:21Z","receivedAt":"2008-04-02T19:34:21Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Wed, Apr 02, 2008 at 12:07:46PM -0700, Junio C Hamano <gitster@pobox.com> wrote:\n> > What is your opinion here?\n> >\n> > 1) Just don't add such an empty template for pre-auto-gc.\n\nOk, here it is. Ah and I forgot to mention in my previous mail that yes,\nchecking for '1' in /sys/class/power_supply/AC/online is right, just\nchecked.\n\nMiklos Vajna (3):\n  git-gc --auto: add pre-auto-gc hook\n  Documentation/hooks: add pre-auto-gc hook\n  contrib/hooks: add an example pre-auto-gc hook\n\n Documentation/hooks.txt           |    7 ++++++\n builtin-gc.c                      |   31 +++++++++++++++++++++++++++++\n contrib/hooks/pre-auto-gc-battery |   39 +++++++++++++++++++++++++++++++++++++\n 3 files changed, 77 insertions(+), 0 deletions(-)\n create mode 100644 contrib/hooks/pre-auto-gc-battery\n"},{"id":"73557","messageId":"4507d1e1d8973a621222c389fa22cd96266312a7.1207164676.git.vmiklos@frugalware.org","threadId":"12943","inReplyTo":"7vtzik848t.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 1/3] git-gc --auto: add pre-auto-gc hook","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-04-02T19:34:38Z","receivedAt":"2008-04-02T19:34:38Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"If such a hook is available and exits with a non-zero status, then\ngit-gc --auto won't run.\n\nSigned-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n builtin-gc.c |   31 +++++++++++++++++++++++++++++++\n 1 files changed, 31 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-gc.c b/builtin-gc.c\nindex 8cef36f..9dc7a37 100644\n--- a/builtin-gc.c\n+++ b/builtin-gc.c\n@@ -157,6 +157,34 @@ static int too_many_packs(void)\n \treturn gc_auto_pack_limit <= cnt;\n }\n \n+static int run_hook()\n+{\n+\tconst char *argv[2];\n+\tstruct child_process hook;\n+\tint ret;\n+\n+\targv[0] = git_path(\"hooks/pre-auto-gc\");\n+\targv[1] = NULL;\n+\n+\tif (access(argv[0], X_OK) < 0)\n+\t\treturn 0;\n+\n+\tmemset(&hook, 0, sizeof(hook));\n+\thook.argv = argv;\n+\thook.no_stdin = 1;\n+\thook.stdout_to_stderr = 1;\n+\n+\tret = start_command(&hook);\n+\tif (ret) {\n+\t\twarning(\"Could not spawn %s\", argv[0]);\n+\t\treturn ret;\n+\t}\n+\tret = finish_command(&hook);\n+\tif (ret == -ERR_RUN_COMMAND_WAITPID_SIGNAL)\n+\t\twarning(\"%s exited due to uncaught signal\", argv[0]);\n+\treturn ret;\n+}\n+\n static int need_to_gc(void)\n {\n \t/*\n@@ -176,6 +204,9 @@ static int need_to_gc(void)\n \t\tappend_option(argv_repack, \"-A\", MAX_ADD);\n \telse if (!too_many_loose_objects())\n \t\treturn 0;\n+\n+\tif (run_hook())\n+\t\treturn 0;\n \treturn 1;\n }\n \n-- \n1.5.5.rc2.4.gec07.dirty\n"},{"id":"73558","messageId":"878a46b80a7b9129b520b522a3f35237c1a01f79.1207164676.git.vmiklos@frugalware.org","threadId":"12943","inReplyTo":"7vtzik848t.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 2/3] Documentation/hooks: add pre-auto-gc hook","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-04-02T19:34:55Z","receivedAt":"2008-04-02T19:34:55Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n Documentation/hooks.txt |    7 +++++++\n 1 files changed, 7 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/hooks.txt b/Documentation/hooks.txt\nindex 76b8d77..44fbe58 100644\n--- a/Documentation/hooks.txt\n+++ b/Documentation/hooks.txt\n@@ -276,3 +276,10 @@ probably enable this hook.\n Both standard output and standard error output are forwarded to\n `git-send-pack` on the other end, so you can simply `echo` messages\n for the user.\n+\n+pre-auto-gc\n+-----------\n+\n+This hook is invoked by `git-gc --auto`. It takes no parameter, and\n+exiting with non-zero status from this script causes the `git-gc --auto`\n+to abort.\n-- \n1.5.5.rc2.4.gec07.dirty\n"},{"id":"73559","messageId":"502492c60a58bdf409b3f6400da25f4a9a97463d.1207164676.git.vmiklos@frugalware.org","threadId":"12943","inReplyTo":"7vtzik848t.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 3/3] contrib/hooks: add an example pre-auto-gc hook","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-04-02T19:35:11Z","receivedAt":"2008-04-02T19:35:11Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"It disables git-gc --auto when you are running Linux and you are on\nbattery.\n\nSigned-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n contrib/hooks/pre-auto-gc-battery |   39 +++++++++++++++++++++++++++++++++++++\n 1 files changed, 39 insertions(+), 0 deletions(-)\n create mode 100644 contrib/hooks/pre-auto-gc-battery\n\ndiff --git a/contrib/hooks/pre-auto-gc-battery b/contrib/hooks/pre-auto-gc-battery\nnew file mode 100644\nindex 0000000..0084a1f\n--- /dev/null\n+++ b/contrib/hooks/pre-auto-gc-battery\n@@ -0,0 +1,39 @@\n+#!/bin/sh\n+#\n+# An example hook script to verify if you are on battery, in case you\n+# are running Linux. Called by git-gc --auto with no arguments. The hook\n+# should exit with non-zero status after issuing an appropriate message\n+# if it wants to stop the auto repacking.\n+#\n+# This hook is stored in the contrib/hooks directory. Your distribution\n+# may have put this somewhere else. If you want to use this hook, you\n+# should make this script executable then link to it in the repository\n+# you would like to use it in.\n+#\n+# For example, if the hook is stored in\n+# /usr/share/git-core/contrib/hooks/pre-auto-gc-battery:\n+#\n+# chmod a+x pre-auto-gc-battery\n+# cd /path/to/your/repository.git\n+# ln -sf /usr/share/git-core/contrib/hooks/pre-auto-gc-battery \\\n+#\thooks/pre-auto-gc\n+\n+if test -x /sbin/on_ac_power && /sbin/on_ac_power\n+then\n+\t/sbin/on_ac_power\n+\tif [ $? = 1 ]; then\n+\t\texit 0\n+\tfi\n+elif test \"$(cat /sys/class/power_supply/AC/online 2>/dev/null)\" = 1\n+then\n+\texit 0\n+elif grep -q 'on-line' /proc/acpi/ac_adapter/AC/state 2>/dev/null\n+then\n+\texit 0\n+elif grep -q '0x01$' /proc/apm 2>/dev/null\n+then\n+\texit 0\n+fi\n+\n+echo \"Auto packing deferred; on battery\"\n+exit 1\n-- \n1.5.5.rc2.4.gec07.dirty\n"},{"id":"73560","messageId":"7vk5jg82b7.fsf@gitster.siamese.dyndns.org","threadId":"12943","inReplyTo":"502492c60a58bdf409b3f6400da25f4a9a97463d.1207164676.git.vmiklos@frugalware.org","subject":"Re: [PATCH 3/3] contrib/hooks: add an example pre-auto-gc hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-04-02T19:49:32Z","receivedAt":"2008-04-02T19:49:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna <vmiklos@frugalware.org> writes:\n\n> +if test -x /sbin/on_ac_power && /sbin/on_ac_power\n> +then\n> +\t/sbin/on_ac_power\n> +\tif [ $? = 1 ]; then\n> +\t\texit 0\n> +\tfi\n\nShouldn't the above be:\n\n\tif test -x /sbin/on_ac_power && /sbin/on_ac_power\n        then\n        \texit 0\n\nThat is, you checked if you have the command to check on-ac-power-ness,\nand the command says yes, so you happily let \"gc --auto\" do its thing.\n"},{"id":"73564","messageId":"96017502f7bca67a14f9a09d19c8ea42bde78949.1207167342.git.vmiklos@frugalware.org","threadId":"12943","inReplyTo":"7vk5jg82b7.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 3/3] contrib/hooks: add an example pre-auto-gc hook","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-04-02T20:22:21Z","receivedAt":"2008-04-02T20:22:21Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"It disables git-gc --auto when you are running Linux and you are on\nbattery.\n\nSigned-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n\nOn Wed, Apr 02, 2008 at 12:49:32PM -0700, Junio C Hamano <gitster@pobox.com> wrote:\n> Miklos Vajna <vmiklos@frugalware.org> writes:\n>\n> > +if test -x /sbin/on_ac_power && /sbin/on_ac_power\n> > +then\n> > +   /sbin/on_ac_power\n> > +   if [ $? = 1 ]; then\n> > +           exit 0\n> > +   fi\n>\n> Shouldn't the above be:\n>\n>       if test -x /sbin/on_ac_power && /sbin/on_ac_power\n>         then\n>               exit 0\n>\n> That is, you checked if you have the command to check\n> on-ac-power-ness,\n> and the command says yes, so you happily let \"gc --auto\" do its thing.\n\nHm, I must be crazy when I wrote this. It has three exit codes:\n\n0 if on AC power\n1 if not on AC power\n255 if can't tell\n\nSo IMHO the right think to do is to check if the return value is != 1,\nand if that's true, then do an exit 0.\n\nHere is try4.\n\n contrib/hooks/pre-auto-gc-battery |   40 +++++++++++++++++++++++++++++++++++++\n 1 files changed, 40 insertions(+), 0 deletions(-)\n create mode 100644 contrib/hooks/pre-auto-gc-battery\n\ndiff --git a/contrib/hooks/pre-auto-gc-battery b/contrib/hooks/pre-auto-gc-battery\nnew file mode 100644\nindex 0000000..c6af057\n--- /dev/null\n+++ b/contrib/hooks/pre-auto-gc-battery\n@@ -0,0 +1,40 @@\n+#!/bin/sh\n+#\n+# An example hook script to verify if you are on battery, in case you\n+# are running Linux. Called by git-gc --auto with no arguments. The hook\n+# should exit with non-zero status after issuing an appropriate message\n+# if it wants to stop the auto repacking.\n+#\n+# This hook is stored in the contrib/hooks directory. Your distribution\n+# may have put this somewhere else. If you want to use this hook, you\n+# should make this script executable then link to it in the repository\n+# you would like to use it in.\n+#\n+# For example, if the hook is stored in\n+# /usr/share/git-core/contrib/hooks/pre-auto-gc-battery:\n+#\n+# chmod a+x pre-auto-gc-battery\n+# cd /path/to/your/repository.git\n+# ln -sf /usr/share/git-core/contrib/hooks/pre-auto-gc-battery \\\n+#\thooks/pre-auto-gc\n+\n+if test -x /sbin/on_ac_power\n+then\n+\t/sbin/on_ac_power\n+\tif test $? -ne 1\n+\tthen\n+\t\texit 0\n+\tfi\n+elif test \"$(cat /sys/class/power_supply/AC/online 2>/dev/null)\" = 1\n+then\n+\texit 0\n+elif grep -q 'on-line' /proc/acpi/ac_adapter/AC/state 2>/dev/null\n+then\n+\texit 0\n+elif grep -q '0x01$' /proc/apm 2>/dev/null\n+then\n+\texit 0\n+fi\n+\n+echo \"Auto packing deferred; on battery\"\n+exit 1\n-- \n1.5.5.rc2.4.gec07.dirty\n"},{"id":"73565","messageId":"7vfxu4808c.fsf@gitster.siamese.dyndns.org","threadId":"12943","inReplyTo":"96017502f7bca67a14f9a09d19c8ea42bde78949.1207167342.git.vmiklos@frugalware.org","subject":"Re: [PATCH 3/3] contrib/hooks: add an example pre-auto-gc hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-04-02T20:34:27Z","receivedAt":"2008-04-02T20:34:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna <vmiklos@frugalware.org> writes:\n\n> Hm, I must be crazy when I wrote this. It has three exit codes:\n>\n> 0 if on AC power\n> 1 if not on AC power\n> 255 if can't tell\n>\n> So IMHO the right think to do is to check if the return value is != 1,\n> and if that's true, then do an exit 0.\n\nIf it is \"can't tell\", then it is \"can't tell\".  Don't you want to err on\nthe safe side?\n"},{"id":"73566","messageId":"20080402204524.GZ3264@genesis.frugalware.org","threadId":"12943","inReplyTo":"7vfxu4808c.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 3/3] contrib/hooks: add an example pre-auto-gc hook","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-04-02T20:45:24Z","receivedAt":"2008-04-02T20:45:24Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Wed, Apr 02, 2008 at 01:34:27PM -0700, Junio C Hamano <gitster@pobox.com> wrote:\n> If it is \"can't tell\", then it is \"can't tell\".  Don't you want to err on\n> the safe side?\n\nFirst, I think \"can't tell\" almost never means \"on battery\" these days,\nsince the necessary kernel support is enabled by the distro's hotplug\nscripts.\n\nSecond, that's what we do in case we fail to read for example\n/sys/class/power_supply/AC/online: just silently redirect the error to\n/dev/null and assume we are not on battery.\n\nI would say it does not worth doing so, but feel free to kick me if it's\na must. :)\n"},{"id":"73631","messageId":"20080403212656.GJ11574@genesis.frugalware.org","threadId":"12943","inReplyTo":"20080402204524.GZ3264@genesis.frugalware.org","subject":"tests for pre-auto-gc hook (WAS: Re: [PATCH 3/3] contrib/hooks: add an example pre-auto-gc hook)","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-04-03T21:26:56Z","receivedAt":"2008-04-03T21:26:56Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Hi,\n\nI wanted to create a test like t7503-pre-commit-hook.sh for pre-auto-gc\nbut actually I'm not sure how to trigger git gc --auto to do something.\n\nHere is what I managed to do so far:\n\n----\ngit init\ngit config gc.auto 1\nfor i in `seq 1 500`; do echo $i >file; git add file; done\ngit commit -m init\n----\n\nAfter this, 'git gc --auto' seem to print the 'Auto packing your\nrepository for optimum performance.' message, but the return code is\nstill 0, just like if it did not do anything.\n\nSo all what I found is that the .git/objects/pack/ directory is empty in\na new repo, and if git gc --auto did something, it won't be empty but:\n\n1) this isn't the proper way I'm sure.\n\n2) if I want to do it from a test, then I would need to create the repo\nfrom scratch again and again, which is quite dirty.\n\nDoes anyone have better ideas?\n\nThanks\n"},{"id":"73645","messageId":"47F5CBFF.1080807@viscovery.net","threadId":"12943","inReplyTo":"20080403212656.GJ11574@genesis.frugalware.org","subject":"Re: tests for pre-auto-gc hook","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-04-04T06:34:39Z","receivedAt":"2008-04-04T06:34:39Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Miklos Vajna schrieb:\n> I wanted to create a test like t7503-pre-commit-hook.sh for pre-auto-gc\n> but actually I'm not sure how to trigger git gc --auto to do something.\n> \n> Here is what I managed to do so far:\n> \n> ----\n> git init\n> git config gc.auto 1\n> for i in `seq 1 500`; do echo $i >file; git add file; done\n> git commit -m init\n> ----\n\nAvoid 'seq'; it is not available everywhere. To avoid spawning 500\nprocesses, you could create a pack file from the files using fastimport,\nthen unpack that for each test case. But you must move the pack out of\n.git/objects/pack before unpacking, otherwise it won't unpack anything.\n\n-- Hannes\n"}]}