threads / patch / 12943

patch, 4 partsRe: [PATCH 1/4] git-gc --auto: add pre-auto-gc hook

Subject: Re: [PATCH 1/4] git-gc --auto: add pre-auto-gc hook

## tl;dr

21 messages between Apr 1, 2008 and Apr 4, 2008. Diffs are folded; open one to read it.

replies: 20people: 3as markdown or json

Junio C Hamano· Apr 1, 2008, 04:51 UTC · lore
Miklos Vajna <vmiklos@frugalware.org> writes:
Show 23 quoted lines
> If such a hook is available and exits with a non-zero status, then
> git-gc --auto won't run.
>
> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
> ---
>  builtin-gc.c |   22 ++++++++++++++++++++++
>  1 files changed, 22 insertions(+), 0 deletions(-)
>
> diff --git a/builtin-gc.c b/builtin-gc.c
> index 8cef36f..acd63be 100644
> --- a/builtin-gc.c
> +++ b/builtin-gc.c
> @@ -157,6 +157,25 @@ static int too_many_packs(void)
>  	return gc_auto_pack_limit <= cnt;
>  }
>  
> +static int run_hook()
> +{
> +	const char *argv[2];
> +	struct child_process hook;
> +
> +	argv[0] = git_path("hooks/pre-auto-gc");
> +	argv[1] = NULL;

Hmm. I wonder if the hook wants any parameters, even though I do not think of any offhand.

Show 7 quoted lines
> +	if (access(argv[0], X_OK) < 0)
> +		return 0;
> +
> +	memset(&hook, 0, sizeof(hook));
> +	hook.argv = argv;
> +	hook.no_stdin = 1;
> +	hook.stdout_to_stderr = 1;
I understand no_stdin, but is there a reason to do stdout_to_stderr?
> +	return run_command(&hook);
> +}

Don't we want to distinguish between the case where start_command() failed, wait_or_whine() failed on waitpid(), the command was killed with signal, or the command actually ran correctly and decided that you should not run "git gc --auto" by exiting non-zero?

I think it is prudent to refrain from running "git gc --auto" in any of the failure cases I listed above, but shouldn't the cases other than the last one at least issue a warning?

Miklos Vajna· Apr 1, 2008, 11:38 UTC · re: Junio C Hamano · lore

[PATCH 0/4] add pre-auto-gc hook for git-gc --auto (try2)

On Mon, Mar 31, 2008 at 09:51:12PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
Show 15 quoted lines
> > +   return run_command(&hook);
> > +}
>
> Don't we want to distinguish between the case where start_command()
> failed, wait_or_whine() failed on waitpid(), the command was killed
> with
> signal, or the command actually ran correctly and decided that you
> should
> not run "git gc --auto" by exiting non-zero?
>
> I think it is prudent to refrain from running "git gc --auto" in any
> of
> the failure cases I listed above, but shouldn't the cases other than
> the
> last one at least issue a warning?

Ok, there are 3 cases here to handle. When wait_or_whine() fails on waitpid() it already prints an error, so that's already handled. I've added two warnings for the other 2 cases.

Other changes:
- try to use on_ac_power when it's available, as suggested by Joey Hess
- mention in the commend of the example pre-auto-gc hook that it's
  Linux-specific, as suggested by Brian Gernhardt
- removed mentioning what the default hook does from hooks.txt, as it's
  an example and it's under contrib/
- moved the battery example to contrib/ and added a minimal example to
  templates/
- removed unnecessary stdout_to_stderr from builtin-gc.c::run_hook()
- removed unnecessary --no-verify option
I hope I haven't missed anything you suggested.
Miklos Vajna (4):
  git-gc --auto: add pre-auto-gc hook
  Documentation/hooks: add pre-auto-gc hook
  templates: add an example pre-auto-gc hook
  contrib/hooks: add an example pre-auto-gc hook
 Documentation/hooks.txt           |    7 +++++
 builtin-gc.c                      |   30 ++++++++++++++++++++++++
 contrib/hooks/pre-auto-gc-battery |   45 +++++++++++++++++++++++++++++++++++++
 templates/hooks--pre-auto-gc      |    9 +++++++
 4 files changed, 91 insertions(+), 0 deletions(-)
 create mode 100644 contrib/hooks/pre-auto-gc-battery
 create mode 100644 templates/hooks--pre-auto-gc
Junio C Hamano· Apr 1, 2008, 23:18 UTC · re: Miklos Vajna · lore

Re: [PATCH 0/4] add pre-auto-gc hook for git-gc --auto (try2)

Miklos Vajna <vmiklos@frugalware.org> writes:
> - removed unnecessary stdout_to_stderr from builtin-gc.c::run_hook()

Wait, a, minute, please. Not so fast. I did not say "I think this is wrong, please fix it".

I only asked why it is so.

Care to explain why it was thought to be necessary, and why you now think it is unnecessary?

This comment is not just for you, but when I ask why it is so, I expect that the contributor would do the homework and present an argument to defend what was submit (or "it was wrong for such and such reasons so let's do it differently like this because this is better for such and such reasons"). The original contributor has thought about the issues much longer and deeper than I, who only just look at the email and kibitz, after the contribution was perfected into a patch form. There must have been a lot more thinking than what can be seen in the submitted material in the contributor's head. I want to see that in the open. Please do not change your mind and retract your change without explaining why.

The argument to respond to such "why is it so?" question would perhaps look like this for this case.

(begin example #1)

Well, I just cut and pasted from existing code. Because all these hooks redirect their stdout to stderr, I think it is consistent to do so with the new hook.

(example #1 ends here)
(begin example #2)

"git grep -n -e stdout_to_stderr" followed by "git blame" reveals the following pattern:

 - builtin-checkout.c:41
   782c2d6 (Build in checkout, 2008-02-07)
 - builtin-commit:382
   f5bbc32 (Port git commit to C., 2007-11-08)
 - help.c:62
   6494998 (help: add "man.viewer" config var to use "woman" or "konqueror", 2008-03-07)
 - receive-pack.c:120,157
   f43cd49 (Change {pre,post}-receive hooks to use stdin, 2007-03-10)
   1d9e8b5 (Split back out update_hook handling in receive-pack, 2007-03-10)
 - transport.c:165,218,297
   cd547b4 (fetch/push: readd rsync support, 2007-10-01)

I think all of these are the result of cut&paste from the original one in receive-pack.c's hook handling.

The ones in the protocol handler may not write to the standard output, because the other end does not expect random diagnostic string in the protocol message at the point these hooks are called. But the other ones could go either way.

However, because existing hooks all show their script output to stderr, it would make sense for this new pre-auto-gc hook to do so as well from the consistency viewpoint. So use of stdout_to_stderr is the right thing to do here.

(example #2 ends here)
Miklos Vajna· Apr 2, 2008, 01:14 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/4] add pre-auto-gc hook for git-gc --auto (try2)

On Tue, Apr 01, 2008 at 04:18:01PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
Show 9 quoted lines
> > - removed unnecessary stdout_to_stderr from builtin-gc.c::run_hook()
> 
> Wait, a, minute, please.  Not so fast.  I did not say "I think this is
> wrong, please fix it".
> 
> I only asked why it is so.
> 
> Care to explain why it was thought to be necessary, and why you now think
> it is unnecessary?

Ok, let me explain. I think it would be logical to put it to stderr, as the other gc --auto messages are there, as well. See the fprintf() in cmd_gc().

Though, I don't think it's that important, so I thought if you think it's unnecessary, I would not argue for it.

So may I put it back? :)
Also, is the other parts of the series looks correct?
Thanks
Junio C Hamano· Apr 2, 2008, 04:02 UTC · re: Miklos Vajna · lore

Re: [PATCH 0/4] add pre-auto-gc hook for git-gc --auto (try2)

Miklos Vajna <vmiklos@frugalware.org> writes:
Show 6 quoted lines
> Ok, let me explain. I think it would be logical to put it to stderr, as
> the other gc --auto messages are there, as well. See the fprintf() in
> cmd_gc().
>
> Though, I don't think it's that important, so I thought if you think
> it's unnecessary, I would not argue for it.

As I made it clear already, I do _not_ think when I kibitz. *You* think, convinced others will say "Yeah, that's a good idea", I get convinced and the patch is applied.

> So may I put it back? :)

I agree that it makes sense to send the output to standard error for consistency. Many existing hooks called from scripted versions of tools seem to contaminate the standard output, though. That would be a good post 1.5.5 clean-up, perhaps.

> Also, is the other parts of the series looks correct?
I do not think we would want empty templates/hooks--pre-gc-auto file.

+#!/bin/sh +# +# An example hook script to verify if you are on battery, in case you +# are running Linux. Called by git-gc --auto with no arguments. The hook +# should exit with non-zero status after issuing an appropriate message +# if it wants to stop the auto repacking. +# +# This hook is stored in the contrib/hooks directory. Your distribution +# will have put this somewhere standard. You should make this script

s/will/may/; s/standard/else/; s/You/If you want to use this hook, you/

+# executable then link to it in the repository you would like to use it +# in. +# +# For example, if the hook is stored in +# /usr/share/git-core/contrib/hooks/pre-auto-gc-battery: +# +# chmod a+x pre-auto-gc-battery +# cd /path/to/your/repository.git +# ln -sf /usr/share/git-core/contrib/hooks/pre-auto-gc-battery \ +# hooks/pre-auto-gc-battery

s|\(/pre-auto-gc\)-battery|\1|;

+defer=0 + +if [ -e /sbin/on_ac_power ]; then + /sbin/on_ac_power + if [ $? = 1 ]; then + defer = 1 + fi

        if test -x /sbin/on_ac_power && /sbin/on_ac_power
        then
                exit 0
Variable assignment in Bourne shell does not allow SP around '='.

I suspect it would be cleaner to invert the logic and "exit 0" when you know you are on AC, and fall through and err on the safer side (i.e. decline) when you cannot tell for certain. You can lose the variable defer that way.

+elif [ -e /sys/class/power_supply/AC/online ]; then + if [ "`cat /sys/class/power_supply/AC/online`" = 0 ]; then + defer=1 + fi

	elif test "$(cat /sys/class/power_supply/AC/online 2>/dev/null)" = 1
	then
        	exit 0

No need to have two separate tests. I do not know if "= 1" is correct, though.

+elif [ -e /proc/acpi/ac_adapter/AC/state ]; then + if grep -q 'off-line' /proc/acpi/ac_adapter/AC/state; then + defer=1 + fi

	elif grep -q 'on-line' /proc/acpi/ac_adapter/AC/state 2>/dev/null
        then
        	exit 0
Likewise.

+elif [ -e /proc/apm ]; then + if grep -q '0$' /proc/apm; then + defer=1 + fi +fi

	elif grep -q '0x01$' /proc/apm 2>/dev/null
        then
        	exit 0
Likewise.
Miklos Vajna· Apr 2, 2008, 19:02 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/4] add pre-auto-gc hook for git-gc --auto (try2)

On Tue, Apr 01, 2008 at 09:02:15PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
Show 6 quoted lines
> > So may I put it back? :)
> 
> I agree that it makes sense to send the output to standard error for
> consistency.  Many existing hooks called from scripted versions of tools
> seem to contaminate the standard output, though.  That would be a good
> post 1.5.5 clean-up, perhaps.
Ok, I will put it back.
> > Also, is the other parts of the series looks correct?
> 
> I do not think we would want empty templates/hooks--pre-gc-auto file.

I did it this way as we have such an empty post-commit hook as well, so I added it for consistency. Though it's true that for example post-applypatch doesn't have such an empty hook under templates either.

What is your opinion here?
Possibilities I see:
1) Just don't add such an empty template for pre-auto-gc.
2) Remove post-commit as well.
3) Add missing empty templates, like post-applypatch (maybe there are
more, I haven't did a complete research).

I've did the rest of the modifications locally as you suggested just I don't want to spam the list with the series before the empty templates question is not clear to me :)

Thanks
Junio C Hamano· Apr 2, 2008, 19:07 UTC · re: Miklos Vajna · lore

Re: [PATCH 0/4] add pre-auto-gc hook for git-gc --auto (try2)

Miklos Vajna <vmiklos@frugalware.org> writes:
> What is your opinion here?
>
> 1) Just don't add such an empty template for pre-auto-gc.
Miklos Vajna· Apr 2, 2008, 19:34 UTC · re: Junio C Hamano · lore

[PATCH 0/3] add pre-auto-gc hook for git-gc --auto (try3)

On Wed, Apr 02, 2008 at 12:07:46PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> > What is your opinion here?
> >
> > 1) Just don't add such an empty template for pre-auto-gc.

Ok, here it is. Ah and I forgot to mention in my previous mail that yes, checking for '1' in /sys/class/power_supply/AC/online is right, just checked.

Miklos Vajna (3):
  git-gc --auto: add pre-auto-gc hook
  Documentation/hooks: add pre-auto-gc hook
  contrib/hooks: add an example pre-auto-gc hook
 Documentation/hooks.txt           |    7 ++++++
 builtin-gc.c                      |   31 +++++++++++++++++++++++++++++
 contrib/hooks/pre-auto-gc-battery |   39 +++++++++++++++++++++++++++++++++++++
 3 files changed, 77 insertions(+), 0 deletions(-)
 create mode 100644 contrib/hooks/pre-auto-gc-battery
Miklos Vajna· Apr 2, 2008, 19:34 UTC · re: Junio C Hamano · lore

[PATCH 1/3] git-gc --auto: add pre-auto-gc hook

If such a hook is available and exits with a non-zero status, then git-gc --auto won't run.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 builtin-gc.c |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)
Show changes to builtin-gc.c +31 −0
diff --git a/builtin-gc.c b/builtin-gc.c
index 8cef36f..9dc7a37 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -157,6 +157,34 @@ static int too_many_packs(void)
 	return gc_auto_pack_limit <= cnt;
 }
 
+static int run_hook()
+{
+	const char *argv[2];
+	struct child_process hook;
+	int ret;
+
+	argv[0] = git_path("hooks/pre-auto-gc");
+	argv[1] = NULL;
+
+	if (access(argv[0], X_OK) < 0)
+		return 0;
+
+	memset(&hook, 0, sizeof(hook));
+	hook.argv = argv;
+	hook.no_stdin = 1;
+	hook.stdout_to_stderr = 1;
+
+	ret = start_command(&hook);
+	if (ret) {
+		warning("Could not spawn %s", argv[0]);
+		return ret;
+	}
+	ret = finish_command(&hook);
+	if (ret == -ERR_RUN_COMMAND_WAITPID_SIGNAL)
+		warning("%s exited due to uncaught signal", argv[0]);
+	return ret;
+}
+
 static int need_to_gc(void)
 {
 	/*
@@ -176,6 +204,9 @@ static int need_to_gc(void)
 		append_option(argv_repack, "-A", MAX_ADD);
 	else if (!too_many_loose_objects())
 		return 0;
+
+	if (run_hook())
+		return 0;
 	return 1;
 }
 
-- 
1.5.5.rc2.4.gec07.dirty
Miklos Vajna· Apr 2, 2008, 19:34 UTC · re: Junio C Hamano · lore

[PATCH 2/3] Documentation/hooks: add pre-auto-gc hook

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 Documentation/hooks.txt |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
Show changes to Documentation/hooks.txt +7 −0
diff --git a/Documentation/hooks.txt b/Documentation/hooks.txt
index 76b8d77..44fbe58 100644
--- a/Documentation/hooks.txt
+++ b/Documentation/hooks.txt
@@ -276,3 +276,10 @@ probably enable this hook.
 Both standard output and standard error output are forwarded to
 `git-send-pack` on the other end, so you can simply `echo` messages
 for the user.
+
+pre-auto-gc
+-----------
+
+This hook is invoked by `git-gc --auto`. It takes no parameter, and
+exiting with non-zero status from this script causes the `git-gc --auto`
+to abort.
-- 
1.5.5.rc2.4.gec07.dirty
Miklos Vajna· Apr 2, 2008, 19:35 UTC · re: Junio C Hamano · lore

[PATCH 3/3] contrib/hooks: add an example pre-auto-gc hook

It disables git-gc --auto when you are running Linux and you are on battery.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 contrib/hooks/pre-auto-gc-battery |   39 +++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)
 create mode 100644 contrib/hooks/pre-auto-gc-battery
Show changes to contrib/hooks/pre-auto-gc-battery +39 −0
diff --git a/contrib/hooks/pre-auto-gc-battery b/contrib/hooks/pre-auto-gc-battery
new file mode 100644
index 0000000..0084a1f
--- /dev/null
+++ b/contrib/hooks/pre-auto-gc-battery
@@ -0,0 +1,39 @@
+#!/bin/sh
+#
+# An example hook script to verify if you are on battery, in case you
+# are running Linux. Called by git-gc --auto with no arguments. The hook
+# should exit with non-zero status after issuing an appropriate message
+# if it wants to stop the auto repacking.
+#
+# This hook is stored in the contrib/hooks directory. Your distribution
+# may have put this somewhere else. If you want to use this hook, you
+# should make this script executable then link to it in the repository
+# you would like to use it in.
+#
+# For example, if the hook is stored in
+# /usr/share/git-core/contrib/hooks/pre-auto-gc-battery:
+#
+# chmod a+x pre-auto-gc-battery
+# cd /path/to/your/repository.git
+# ln -sf /usr/share/git-core/contrib/hooks/pre-auto-gc-battery \
+#	hooks/pre-auto-gc
+
+if test -x /sbin/on_ac_power && /sbin/on_ac_power
+then
+	/sbin/on_ac_power
+	if [ $? = 1 ]; then
+		exit 0
+	fi
+elif test "$(cat /sys/class/power_supply/AC/online 2>/dev/null)" = 1
+then
+	exit 0
+elif grep -q 'on-line' /proc/acpi/ac_adapter/AC/state 2>/dev/null
+then
+	exit 0
+elif grep -q '0x01$' /proc/apm 2>/dev/null
+then
+	exit 0
+fi
+
+echo "Auto packing deferred; on battery"
+exit 1
-- 
1.5.5.rc2.4.gec07.dirty
Junio C Hamano· Apr 2, 2008, 19:49 UTC · re: Miklos Vajna · lore

Re: [PATCH 3/3] contrib/hooks: add an example pre-auto-gc hook

Miklos Vajna <vmiklos@frugalware.org> writes:
Show 6 quoted lines
> +if test -x /sbin/on_ac_power && /sbin/on_ac_power
> +then
> +	/sbin/on_ac_power
> +	if [ $? = 1 ]; then
> +		exit 0
> +	fi
Shouldn't the above be:
	if test -x /sbin/on_ac_power && /sbin/on_ac_power
        then
        	exit 0

That is, you checked if you have the command to check on-ac-power-ness, and the command says yes, so you happily let "gc --auto" do its thing.

Miklos Vajna· Apr 2, 2008, 20:22 UTC · re: Junio C Hamano · lore

[PATCH 3/3] contrib/hooks: add an example pre-auto-gc hook

It disables git-gc --auto when you are running Linux and you are on battery.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
On Wed, Apr 02, 2008 at 12:49:32PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
Show 18 quoted lines
> Miklos Vajna <vmiklos@frugalware.org> writes:
>
> > +if test -x /sbin/on_ac_power && /sbin/on_ac_power
> > +then
> > +   /sbin/on_ac_power
> > +   if [ $? = 1 ]; then
> > +           exit 0
> > +   fi
>
> Shouldn't the above be:
>
>       if test -x /sbin/on_ac_power && /sbin/on_ac_power
>         then
>               exit 0
>
> That is, you checked if you have the command to check
> on-ac-power-ness,
> and the command says yes, so you happily let "gc --auto" do its thing.
Hm, I must be crazy when I wrote this. It has three exit codes:

0 if on AC power 1 if not on AC power 255 if can't tell

So IMHO the right think to do is to check if the return value is != 1, and if that's true, then do an exit 0.

Here is try4.
 contrib/hooks/pre-auto-gc-battery |   40 +++++++++++++++++++++++++++++++++++++
 1 files changed, 40 insertions(+), 0 deletions(-)
 create mode 100644 contrib/hooks/pre-auto-gc-battery
Show changes to contrib/hooks/pre-auto-gc-battery +40 −0
diff --git a/contrib/hooks/pre-auto-gc-battery b/contrib/hooks/pre-auto-gc-battery
new file mode 100644
index 0000000..c6af057
--- /dev/null
+++ b/contrib/hooks/pre-auto-gc-battery
@@ -0,0 +1,40 @@
+#!/bin/sh
+#
+# An example hook script to verify if you are on battery, in case you
+# are running Linux. Called by git-gc --auto with no arguments. The hook
+# should exit with non-zero status after issuing an appropriate message
+# if it wants to stop the auto repacking.
+#
+# This hook is stored in the contrib/hooks directory. Your distribution
+# may have put this somewhere else. If you want to use this hook, you
+# should make this script executable then link to it in the repository
+# you would like to use it in.
+#
+# For example, if the hook is stored in
+# /usr/share/git-core/contrib/hooks/pre-auto-gc-battery:
+#
+# chmod a+x pre-auto-gc-battery
+# cd /path/to/your/repository.git
+# ln -sf /usr/share/git-core/contrib/hooks/pre-auto-gc-battery \
+#	hooks/pre-auto-gc
+
+if test -x /sbin/on_ac_power
+then
+	/sbin/on_ac_power
+	if test $? -ne 1
+	then
+		exit 0
+	fi
+elif test "$(cat /sys/class/power_supply/AC/online 2>/dev/null)" = 1
+then
+	exit 0
+elif grep -q 'on-line' /proc/acpi/ac_adapter/AC/state 2>/dev/null
+then
+	exit 0
+elif grep -q '0x01$' /proc/apm 2>/dev/null
+then
+	exit 0
+fi
+
+echo "Auto packing deferred; on battery"
+exit 1
-- 
1.5.5.rc2.4.gec07.dirty
Junio C Hamano· Apr 2, 2008, 20:34 UTC · re: Miklos Vajna · lore

Re: [PATCH 3/3] contrib/hooks: add an example pre-auto-gc hook

Miklos Vajna <vmiklos@frugalware.org> writes:
Show 8 quoted lines
> Hm, I must be crazy when I wrote this. It has three exit codes:
>
> 0 if on AC power
> 1 if not on AC power
> 255 if can't tell
>
> So IMHO the right think to do is to check if the return value is != 1,
> and if that's true, then do an exit 0.

If it is "can't tell", then it is "can't tell". Don't you want to err on the safe side?

Miklos Vajna· Apr 2, 2008, 20:45 UTC · re: Junio C Hamano · lore

Re: [PATCH 3/3] contrib/hooks: add an example pre-auto-gc hook

On Wed, Apr 02, 2008 at 01:34:27PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> If it is "can't tell", then it is "can't tell".  Don't you want to err on
> the safe side?

First, I think "can't tell" almost never means "on battery" these days, since the necessary kernel support is enabled by the distro's hotplug scripts.

Second, that's what we do in case we fail to read for example /sys/class/power_supply/AC/online: just silently redirect the error to /dev/null and assume we are not on battery.

I would say it does not worth doing so, but feel free to kick me if it's a must. :)

Miklos Vajna· Apr 3, 2008, 21:26 UTC · re: Miklos Vajna · lore

tests for pre-auto-gc hook (WAS: Re: [PATCH 3/3] contrib/hooks: add an example pre-auto-gc hook)

Hi,

I wanted to create a test like t7503-pre-commit-hook.sh for pre-auto-gc but actually I'm not sure how to trigger git gc --auto to do something.

Here is what I managed to do so far:

---- git init git config gc.auto 1 for i in `seq 1 500`; do echo $i >file; git add file; done git commit -m init ----

After this, 'git gc --auto' seem to print the 'Auto packing your repository for optimum performance.' message, but the return code is still 0, just like if it did not do anything.

So all what I found is that the .git/objects/pack/ directory is empty in a new repo, and if git gc --auto did something, it won't be empty but:

1) this isn't the proper way I'm sure.
2) if I want to do it from a test, then I would need to create the repo
from scratch again and again, which is quite dirty.
Does anyone have better ideas?
Thanks
Johannes Sixt· Apr 4, 2008, 06:34 UTC · re: Miklos Vajna · lore

Re: tests for pre-auto-gc hook

Miklos Vajna schrieb:
Show 11 quoted lines
> I wanted to create a test like t7503-pre-commit-hook.sh for pre-auto-gc
> but actually I'm not sure how to trigger git gc --auto to do something.
> 
> Here is what I managed to do so far:
> 
> ----
> git init
> git config gc.auto 1
> for i in `seq 1 500`; do echo $i >file; git add file; done
> git commit -m init
> ----

Avoid 'seq'; it is not available everywhere. To avoid spawning 500 processes, you could create a pack file from the files using fastimport, then unpack that for each test case. But you must move the pack out of .git/objects/pack before unpacking, otherwise it won't unpack anything.

-- Hannes
Miklos Vajna· Apr 1, 2008, 11:39 UTC · re: Junio C Hamano · lore

[PATCH 1/4] git-gc --auto: add pre-auto-gc hook

If such a hook is available and exits with a non-zero status, then git-gc --auto won't run.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 builtin-gc.c |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)
Show changes to builtin-gc.c +30 −0
diff --git a/builtin-gc.c b/builtin-gc.c
index 8cef36f..6561639 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c
@@ -157,6 +157,33 @@ static int too_many_packs(void)
 	return gc_auto_pack_limit <= cnt;
 }
 
+static int run_hook()
+{
+	const char *argv[2];
+	struct child_process hook;
+	int ret;
+
+	argv[0] = git_path("hooks/pre-auto-gc");
+	argv[1] = NULL;
+
+	if (access(argv[0], X_OK) < 0)
+		return 0;
+
+	memset(&hook, 0, sizeof(hook));
+	hook.argv = argv;
+	hook.no_stdin = 1;
+
+	ret = start_command(&hook);
+	if (ret) {
+		warning("Could not spawn %s", argv[0]);
+		return ret;
+	}
+	ret = finish_command(&hook);
+	if (ret == -ERR_RUN_COMMAND_WAITPID_SIGNAL)
+		warning("%s exited due to uncaught signal", argv[0]);
+	return ret;
+}
+
 static int need_to_gc(void)
 {
 	/*
@@ -176,6 +203,9 @@ static int need_to_gc(void)
 		append_option(argv_repack, "-A", MAX_ADD);
 	else if (!too_many_loose_objects())
 		return 0;
+
+	if (run_hook())
+		return 0;
 	return 1;
 }
 
-- 
1.5.5.rc2.4.gec07.dirty
Miklos Vajna· Apr 1, 2008, 11:39 UTC · re: Junio C Hamano · lore

[PATCH 2/4] Documentation/hooks: add pre-auto-gc hook

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 Documentation/hooks.txt |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
Show changes to Documentation/hooks.txt +7 −0
diff --git a/Documentation/hooks.txt b/Documentation/hooks.txt
index 76b8d77..44fbe58 100644
--- a/Documentation/hooks.txt
+++ b/Documentation/hooks.txt
@@ -276,3 +276,10 @@ probably enable this hook.
 Both standard output and standard error output are forwarded to
 `git-send-pack` on the other end, so you can simply `echo` messages
 for the user.
+
+pre-auto-gc
+-----------
+
+This hook is invoked by `git-gc --auto`. It takes no parameter, and
+exiting with non-zero status from this script causes the `git-gc --auto`
+to abort.
-- 
1.5.5.rc2.4.gec07.dirty
Miklos Vajna· Apr 1, 2008, 11:39 UTC · re: Junio C Hamano · lore

[PATCH 3/4] templates: add an example pre-auto-gc hook

it's empty by default.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 templates/hooks--pre-auto-gc |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)
 create mode 100644 templates/hooks--pre-auto-gc
Show changes to templates/hooks--pre-auto-gc +9 −0
diff --git a/templates/hooks--pre-auto-gc b/templates/hooks--pre-auto-gc
new file mode 100644
index 0000000..8a9bf37
--- /dev/null
+++ b/templates/hooks--pre-auto-gc
@@ -0,0 +1,9 @@
+#!/bin/sh
+#
+# An example hook that is called by git-gc --auto with no arguments. The
+# hook should exit with non-zero status after issuing an appropriate
+# message if it wants to stop the auto repacking.
+#
+# To enable this hook, make this file executable.
+
+: Nothing
-- 
1.5.5.rc2.4.gec07.dirty
Miklos Vajna· Apr 1, 2008, 11:39 UTC · re: Junio C Hamano · lore

[PATCH 4/4] contrib/hooks: add an example pre-auto-gc hook

It disables git-gc --auto when you are running Linux and you are on battery.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 contrib/hooks/pre-auto-gc-battery |   45 +++++++++++++++++++++++++++++++++++++
 1 files changed, 45 insertions(+), 0 deletions(-)
 create mode 100644 contrib/hooks/pre-auto-gc-battery
Show changes to contrib/hooks/pre-auto-gc-battery +45 −0
diff --git a/contrib/hooks/pre-auto-gc-battery b/contrib/hooks/pre-auto-gc-battery
new file mode 100644
index 0000000..4077111
--- /dev/null
+++ b/contrib/hooks/pre-auto-gc-battery
@@ -0,0 +1,45 @@
+#!/bin/sh
+#
+# An example hook script to verify if you are on battery, in case you
+# are running Linux. Called by git-gc --auto with no arguments. The hook
+# should exit with non-zero status after issuing an appropriate message
+# if it wants to stop the auto repacking.
+#
+# This hook is stored in the contrib/hooks directory. Your distribution
+# will have put this somewhere standard. You should make this script
+# executable then link to it in the repository you would like to use it
+# in.
+#
+# For example, if the hook is stored in
+# /usr/share/git-core/contrib/hooks/pre-auto-gc-battery:
+#
+# chmod a+x pre-auto-gc-battery
+# cd /path/to/your/repository.git
+# ln -sf /usr/share/git-core/contrib/hooks/pre-auto-gc-battery \
+#	hooks/pre-auto-gc-battery
+
+defer=0
+
+if [ -e /sbin/on_ac_power ]; then
+	/sbin/on_ac_power
+	if [ $? = 1 ]; then
+		defer = 1
+	fi
+elif [ -e /sys/class/power_supply/AC/online ]; then
+	if [ "`cat /sys/class/power_supply/AC/online`" = 0 ]; then
+		defer=1
+	fi
+elif [ -e /proc/acpi/ac_adapter/AC/state ]; then
+	if grep -q 'off-line' /proc/acpi/ac_adapter/AC/state; then
+		defer=1
+	fi
+elif [ -e /proc/apm ]; then
+	if grep -q '0$' /proc/apm; then
+		defer=1
+	fi
+fi
+
+if [ "$defer" = 1 ]; then
+	echo "Auto packing deferred; on battery"
+	exit 1
+fi
-- 
1.5.5.rc2.4.gec07.dirty

← back to recent threads