This patch modifies git gc --auto so that it will not always repack when

a user is on battery.
It introduces the new gc.deferonbattery configuration variable, which

defaults to true. If it's true and the user is on battery, it will not

run git gc --auto.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>

---
Idea is from e2fsprogs, such a repack may take a lot of time and usually

you don't have infinite time when you are on battery.. :)
If the patch looks OK, just it's too late for 1.5.5, then please let me

know and I'll resend after 1.5.5.
Thanks.
 Documentation/git-gc.txt |    4 +++

 builtin-gc.c             |   49 ++++++++++++++++++++++++++++++++++++++++++++++

 2 files changed, 53 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt

index d424a4e..7d54148 100644

--- a/Documentation/git-gc.txt

+++ b/Documentation/git-gc.txt

@@ -104,6 +104,10 @@ The optional configuration variable 'gc.pruneExpire' controls how old

 the unreferenced loose objects have to be before they are pruned.  The

 default is "2 weeks ago".
+The optional configuration variable 'gc.deferonbattery' determines if

+`git gc --auto` should be disabled if the system is running on battery.

+This defaults to true.

+

 See Also

 --------

 linkgit:git-prune[1]

diff --git a/builtin-gc.c b/builtin-gc.c

index 8cef36f..7beb046 100644

--- a/builtin-gc.c

+++ b/builtin-gc.c

@@ -23,6 +23,7 @@ static const char * const builtin_gc_usage[] = {

 };
 static int pack_refs = 1;

+static int defer_on_battery = 1;

 static int aggressive_window = -1;

 static int gc_auto_threshold = 6700;

 static int gc_auto_pack_limit = 50;

@@ -67,6 +68,10 @@ static int gc_config(const char *var, const char *value)

 		prune_expire = xstrdup(value);

 		return 0;

 	}

+	if (!strcmp(var, "gc.deferonbattery")) {

+		defer_on_battery = git_config_bool(var, value);

+		return 0;

+	}

 	return git_default_config(var, value);

 }
@@ -157,6 +162,45 @@ static int too_many_packs(void...
[ message continues ]

Shouldn't the config option have 'auto' in the name? Or in some way convey

that this is _only_ about deferring automatic gc'ing?
-brandon
--

On Mon, Mar 31, 2008 at 11:24:22AM -0500, Brandon Casey <casey@nrlssc.navy.=
That makes sense. Though this patch isn't OK, see my other patch series

in this thread (the pre-auto-gc hook has no config name).

Ah, yes I didn't look closely at the revised series.
-brandon
--

Hi,
If /proc/apm could be opened, should you still try to open

/proc/acpi/ac_adapter?
And what about system dependency?  I mean, if at all, this stuff belongs

to compat/.  Definitely not into builtin-gc.c.  And yes, that means that

you should not call the function is_on_battery() blindly, but _only_ if

defer_on_battery is set.
Ciao,
Style.  As can be seen 3 lines earlier, we put a space after the "if".
Ciao,

Dscho

--

On Mon, Mar 31, 2008 at 01:41:18AM +0200, Johannes Schindelin <Johannes.Sch=
Hm, should I just move it to there or should there be some kind of

check? Currently it just tries to open that file under /sys and if

fails, it just assumes we are not on battery. I think that's the

expected behaviour on systems not having a /sys filesystem.
As far as I see, compat/ is for functions which are available on some

systems but not an all ones. Obviously is_on_battery() won't be available

on any system. :)
The other issues (I hope) are fixed in the second patch.

Hm, maybe move that check into need_to_gc instead? Seems a bit weird to
The /proc stuff is already deprecated IIRC, the new file to check on

Linux is /sys/class/power_supply/AC/online.
--

I would *seriously* suggest making this soem kind of generic callback and

not Linux-specific. 
How about making it more akin to a pre-auto-gc "hook" - run a script

instead of hardcoding something like this!
		Linus

--

FWIW, Debian (and I assume Ubuntu also) systems have a on_ac_power

script that exits 0 or 1 accordingly. It would be a good thing to point

the hook at, or even a good reference when writing your own version of

the hook since it also supports /proc/pmu and apm.
--=20

see shy jo

It disabled git-gc --auto when you are on battery.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>

---

 templates/hooks--pre-auto-gc |   29 +++++++++++++++++++++++++++++

 1 files changed, 29 insertions(+), 0 deletions(-)

 create mode 100644 templates/hooks--pre-auto-gc
diff --git a/templates/hooks--pre-auto-gc b/templates/hooks--pre-auto-gc

new file mode 100644

index 0000000..40c4759

--- /dev/null

+++ b/templates/hooks--pre-auto-gc

@@ -0,0 +1,29 @@

+#!/bin/sh

+#

+# An example hook script to verify if you are on battery.  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.

+

+defer=0

+

+if [ -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.g283c6
--

You probably want to mention that this example hook is Linux-specific.
~~ Brian G.

--

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>

---

 Documentation/hooks.txt |   10 ++++++++++

 1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/Documentation/hooks.txt b/Documentation/hooks.txt

index 76b8d77..04ec352 100644

--- a/Documentation/hooks.txt

+++ b/Documentation/hooks.txt

@@ -276,3 +276,13 @@ 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`, and can be bypassed with

+`\--no-verify` option.  It takes no parameter, and exiting with non-zero

+status from this script causes the `git-gc --auto` to abort.

+

+The default 'pre-auto-gc' hook, when enabled, defers auto repacking when

+you are on battery.

--

1.5.5.rc2.4.g283c6
--

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>

---

 Documentation/git-gc.txt |    4 ++++

 builtin-gc.c             |    4 +++-

 2 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt

index d424a4e..396da5c 100644

--- a/Documentation/git-gc.txt

+++ b/Documentation/git-gc.txt

@@ -62,6 +62,10 @@ automatic consolidation of packs.

 --quiet::

 	Suppress all progress reports.
+--no-verify::

+	This option bypasses the pre-auto-gc hook.

+	See also link:hooks.html[hooks].

+

 Configuration

 -------------
diff --git a/builtin-gc.c b/builtin-gc.c

index acd63be..1eca6b2 100644

--- a/builtin-gc.c

+++ b/builtin-gc.c

@@ -27,6 +27,7 @@ static int aggressive_window = -1;

 static int gc_auto_threshold = 6700;

 static int gc_auto_pack_limit = 50;

 static char *prune_expire = "2.weeks.ago";

+static int no_verify;
 #define MAX_ADD 10

 static const char *argv_pack_refs[] = {"pack-refs", "--all", "--prune", NULL};

@@ -196,7 +197,7 @@ static int need_to_gc(void)

 	else if (!too_many_loose_objects())

 		return 0;
-	if (run_hook())

+	if (!no_verify && run_hook())

 		return 0;

 	return 1;

 }

@@ -214,6 +215,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)

 		OPT_BOOLEAN(0, "aggressive", &aggressive, "be more thorough (increased runtime)"),

 		OPT_BOOLEAN(0, "auto", &auto_gc, "enable auto-gc mode"),

 		OPT_BOOLEAN('q', "quiet", &quiet, "suppress progress reports"),

+		OPT_BOOLEAN('n', "no-verify", &no_verify, "bypass pre-auto-gc hook"),

 		OPT_END()

 	};
--

1.5.5.rc2.4.g283c6
--

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;

+

+	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;

+

+	return run_command(&hook);

+}

+

 static int need_to_gc(void)

 {

 	/*

@@ -176,6 +195,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.g283c6
--

Something like this?
Miklos Vajna (4):

  git-gc --auto: add pre-auto-gc hook

  git-gc: add a --no-verify option to bypass the pre-auto-gc hook

  Documentation/hooks: add pre-auto-gc hook

  templates: add an example pre-auto-gc hook
 Documentation/git-gc.txt     |    4 ++++

 Documentation/hooks.txt      |   10 ++++++++++

 builtin-gc.c                 |   24 ++++++++++++++++++++++++

 templates/hooks--pre-auto-gc |   29 +++++++++++++++++++++++++++++

 4 files changed, 67 insertions(+), 0 deletions(-)

 create mode 100644 templates/hooks--pre-auto-gc
--

That would be a sensible approach.
We also would need to make sure that Porcelain that call "gc --auto" does

not have an assumption that auto is ultra-cheap, however, as we are

talking about potentially two fork-exec in the usual "noop" case with such

a change, but we need to do that regardless.
 * git-svn has "every 1000 commits and one at the end" which should be

   Ok.
 * git-cvsimport does "repack -a -d" every 1k commits and once more at the

   end if there are many remaining loose objects.
 * "git-rebase -i" does one at the end, which should be Ok.
 * "git commit" used to have one at the end in the scripted version, but

   seems to have lost it in C rewrite.
So I think we are Ok with an additional hook.
By the way, Linus, is your MUA UTF-8 challenged?  I see Björn's name on

the To: header mangled.
--

From: Johannes Schindelin <johannes.schindelin@gmx.de>
As the scripted version of git-commit did, we now call gc --auto just

before the post-commit hook.
Any errors of gc --auto should be non-fatal, so we do not catch those; the

user should see them anyway.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

---
	Junio wrote:

	>

	>  * "git commit" used to have one [call to 'gc --auto'] at the

	>    end in the scripted version, but seems to have lost it in C

	>    rewrite.
	How about this?
 builtin-commit.c |    3 +++

 1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c

index 660a345..bec62b2 100644

--- a/builtin-commit.c

+++ b/builtin-commit.c

@@ -863,6 +863,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)

 	char *nl, *p;

 	unsigned char commit_sha1[20];

 	struct ref_lock *ref_lock;

+	const char *argv_gc_auto[] = { "gc", "--auto", NULL };
 	git_config(git_commit_config);
@@ -987,6 +988,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)

 		     "not exceeded, and then \"git reset HEAD\" to recover.");
 	rerere();

+	/* We ignore errors in 'gc --auto', since the user should see them. */

+	run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);

 	run_hook(get_index_file(), "post-commit", NULL);

 	if (!quiet)

 		print_summary(prefix, commit_sha1);

--

1.5.5.rc2.30.gf2056
--

Hi,
Any news on this?
Ciao,

Dscho

--

I had an impression that we accepted the hook which made "gc --auto" more

expensive by forcing it to check the hook (and possibly execute it every

time) after vetting am, svn and friends to make sure nobody triggered "gc

--auto" once per every commit, and during that vetting process we noticed

that "git commit" lost the "gc --auto" at the end.
With this patch, we would again have a command that runs "gc --auto" once

per every commit, wouldn't we?  Interactive use of git-commit is fine with

it, but if people drive "git commit" from their scripts in a loop, they

would be hurt.
Having said that, perhaps the overhead of "gc --auto" hook is not such a

big deal.  I dunno.

--

Not sure if we have timing information. E.g. look at the

timestamp of some file that "git gc" touched and only do this if

it's old enought. Or look at the previous commit and only

run "gc --auto" if this is far enought away, e.g. a couple of

hours.

--

Hi,
Ah yes, completely forgot.  Thanks.
Ciao,

Dscho

--

My MUA is not utf-8 challenged per se, but it doesn't seem to like Björn's

headers.
For example, I see "しらいしななこ <nanako3@bluebottle.com>" perfectly

correctly (Content-Type: text/plain; charset=UTF-8), but Björn's emails

tend to be (Content-Type: text/plain; charset=iso-8859-1) but then he has

his email address in utf-8, and that seems to be what makes my MUA

unhappy about it.
			Linus
--

Hm, that's weird. My header shows my name as iso-8859-1, same as the

body. I checked the copy that I got from the list to eliminate any weird

local-copy effects.
From:	=?iso-8859-1?Q?Bj=F6rn?= Steinbrink <B.Steinbrink@gmx.de>
So maybe your MUA is iso-8859-1 challenged instead? I'll send this one

out as UTF-8.
Björn

--

Ahhah! That's it. Not my MUA, but I'm using fetchmail, and I have copied

my .fetchmailrc file around for years. As a result, it has 'mimedecode'

set, because pine used to be really bad at this and obviously all my

original BK (and later git) email scripts didn't do mime decoding either.
So what is probably happening is that my fetchmail setup dropped the

charset information for the header (this is documented by fetchmail, so

it's not a bug, it's just part of the rules) and just turned it into the

raw byte sequence.
I've turned off mimedecode, can you send another email to me (in private)

to see if not doing that just fixes things?
			Linus

--

I didn't meant to make that Linux-specific, I just wanted to mention

that the /proc stuff might break rather "soon", and that the code should
Sounds nice.
Björn

--

This patch modifies git gc --auto so that it will not always repack when

a user is on battery.
It introduces the new gc.deferonbattery configuration variable, which

defaults to true. If it's true and the user is on battery, it will not

run git gc --auto.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>

---
And that makes the patch smaller as well. :)
Something like this?
 Documentation/git-gc.txt |    4 ++++

 builtin-gc.c             |   24 ++++++++++++++++++++++++

 2 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt

index d424a4e..7d54148 100644

--- a/Documentation/git-gc.txt

+++ b/Documentation/git-gc.txt

@@ -104,6 +104,10 @@ The optional configuration variable 'gc.pruneExpire' controls how old

 the unreferenced loose objects have to be before they are pruned.  The

 default is "2 weeks ago".
+The optional configuration variable 'gc.deferonbattery' determines if

+`git gc --auto` should be disabled if the system is running on battery.

+This defaults to true.

+

 See Also

 --------

 linkgit:git-prune[1]

diff --git a/builtin-gc.c b/builtin-gc.c

index 8cef36f..512a357 100644

--- a/builtin-gc.c

+++ b/builtin-gc.c

@@ -23,6 +23,7 @@ static const char * const builtin_gc_usage[] = {

 };
 static int pack_refs = 1;

+static int defer_on_battery = 1;

 static int aggressive_window = -1;

 static int gc_auto_threshold = 6700;

 static int gc_auto_pack_limit = 50;

@@ -67,6 +68,10 @@ static int gc_config(const char *var, const char *value)

 		prune_expire = xstrdup(value);

 		return 0;

 	}

+	if (!strcmp(var, "gc.deferonbattery")) {

+		defer_on_battery = git_config_bool(var, value);

+		return 0;

+	}

 	return git_default_config(var, value);

 }
@@ -157,6 +162,20 @@ static int too_many_packs(void)

 	return gc_auto_pack_limit <= cnt;

 }
+static int is_on_battery(void)

+{

+	FILE *fp;

+	unsigned int state = 1;

+

+	if ((fp = fopen("/sys/class/power_supply/AC/online", "r"))) {

+		if ...
[ message continues ]

Oh, oops, I didn't meant to say that you should remove the /proc/*

checks, just that they'll probably break in the future and that the new

location needs to be added. Those running older kernels should probably

not be excluded ;-)
--