@matheusbsbs / matheus-bagliano / commits / 4c5baf0273

gc: remove gc.pid file at end of execution

This file isn't really harmful, but isn't useful either, and can create minor annoyance for the user: * It's confusing, as the presence of a *.pid file often implies that a process is currently running. A user running "ls .git/" and finding this file may incorrectly guess that a "git gc" is currently running. * Leaving this file means that a "git gc" in an already gc-ed repo is no-longer a no-op. A user running "git gc" in a set of repositories, and then synchronizing this set (e.g. rsync -av, unison, ...) will see all the gc.pid files as changed, which creates useless noise. This patch unlinks the file after the garbage collection is done, so that gc.pid is actually present only during execution. Future versions of Git may want to use the information left in the gc.pid file (e.g. for policies like "don't attempt to run a gc if one has already been ran less than X hours ago"). If so, this patch can safely be reverted. For now, let's not bother the users. Explained-by: Matthieu Moy <Matthieu.Moy@imag.fr> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Improved-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Nieder committed Oct 16, 2013 at 16:11 UTC 4c5baf0273ae3382dbaca81fee0331cada5899ea

2 files changed +29

builtin/gc.c

+24

@@ -14,6 +14,7 @@

14 #include "cache.h"

15 #include "parse-options.h"

16 #include "run-command.h"

17 +#include "sigchain.h"

18 #include "argv-array.h"

20 #define FAILED_RUN "failed to run %s"

@@ -35,6 +36,21 @@ static struct argv_array repack = ARGV_ARRAY_INIT;

36 static struct argv_array prune = ARGV_ARRAY_INIT;

37 static struct argv_array rerere = ARGV_ARRAY_INIT;

39 +static char *pidfile;

40 +

41 +static void remove_pidfile(void)

42 +{

43 + if (pidfile)

44 + unlink(pidfile);

45 +}

46 +

47 +static void remove_pidfile_on_signal(int signo)

48 +{

49 + remove_pidfile();

50 + sigchain_pop(signo);

51 + raise(signo);

52 +}

53 +

54 static int gc_config(const char *var, const char *value, void *cb)

55 {

56 if (!strcmp(var, "gc.packrefs")) {

@@ -179,6 +195,10 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)

195 FILE *fp;

196 int fd, should_exit;

197

198 + if (pidfile)

199 + /* already locked */

200 + return NULL;

201 +

202 if (gethostname(my_host, sizeof(my_host)))

203 strcpy(my_host, "unknown");

204

@@ -219,6 +239,10 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)

239 strbuf_release(&sb);

240 commit_lock_file(&lock);

241

242 + pidfile = git_pathdup("gc.pid");

243 + sigchain_push_common(remove_pidfile_on_signal);

244 + atexit(remove_pidfile);

245 +

246 return NULL;

247 }

248

t/t6500-gc.sh

@@ -9,6 +9,11 @@ test_expect_success 'gc empty repository' '

9 git gc

10 '

12 +test_expect_success 'gc does not leave behind pid file' '

13 + git gc &&

14 + test_path_is_missing .git/gc.pid

15 +'

16 +

17 test_expect_success 'gc --gobbledegook' '

18 test_expect_code 129 git gc --nonsense 2>err &&

19 test_i18ngrep "[Uu]sage: git gc" err