From 7f8508e8d320d768a34483682e9f2dc5af1af04b Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Mon, 31 Jul 2006 09:55:15 -0700 Subject: [PATCH 1/5] Fix double "close()" in ce_compare_data Doing an "strace" on "git diff" shows that we close() a file descriptor twice (getting EBADFD on the second one) when we end up in ce_compare_data if the index does not match the checked-out stat information. The "index_fd()" function will already have closed the fd for us, so we should not close it again. Signed-off-by: Linus Torvalds Signed-off-by: Junio C Hamano --- read-cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/read-cache.c b/read-cache.c index c0b031367b..f92cdaacee 100644 --- a/read-cache.c +++ b/read-cache.c @@ -61,7 +61,7 @@ static int ce_compare_data(struct cache_entry *ce, struct stat *st) unsigned char sha1[20]; if (!index_fd(sha1, fd, st, 0, NULL)) match = memcmp(sha1, ce->sha1, 20); - close(fd); + /* index_fd() closed the file descriptor already */ } return match; } From 3e04228b0c4bbb4b5cd46db9a714dfe699fa4cb8 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Mon, 31 Jul 2006 13:13:55 -0700 Subject: [PATCH 2/5] Fix up some fallout from "setup_git_directory()" cleanups git-ls-files was broken by the setup_git_directory() calling changes, because I had missed the fact that the "prefix" variable in that file was static to the whole file, and unlike git-ls-tree (where I had fixed it up), it ended up using two different variables with the same name depending on what the scoping happened to be. This fixes it up properly (by just removing the static variable, and passing the automatic one around properly), and git-ls-files should work again. Signed-off-by: Linus Torvalds Signed-off-by: Junio C Hamano --- builtin-ls-files.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/builtin-ls-files.c b/builtin-ls-files.c index 79ffe8f423..11386c432b 100644 --- a/builtin-ls-files.c +++ b/builtin-ls-files.c @@ -24,7 +24,6 @@ static int show_valid_bit = 0; static int line_terminator = '\n'; static int prefix_len = 0, prefix_offset = 0; -static const char *prefix = NULL; static const char **pathspec = NULL; static int error_unmatch = 0; static char *ps_matched = NULL; @@ -207,7 +206,7 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce) } } -static void show_files(struct dir_struct *dir) +static void show_files(struct dir_struct *dir, const char *prefix) { int i; @@ -253,7 +252,7 @@ static void show_files(struct dir_struct *dir) /* * Prune the index to only contain stuff starting with "prefix" */ -static void prune_cache(void) +static void prune_cache(const char *prefix) { int pos = cache_name_pos(prefix, prefix_len); unsigned int first, last; @@ -276,7 +275,7 @@ static void prune_cache(void) active_nr = last; } -static void verify_pathspec(void) +static const char *verify_pathspec(const char *prefix) { const char **p, *n, *prev; char *real_prefix; @@ -313,7 +312,7 @@ static void verify_pathspec(void) memcpy(real_prefix, prev, max); real_prefix[max] = 0; } - prefix = real_prefix; + return real_prefix; } static const char ls_files_usage[] = @@ -453,7 +452,7 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix) /* Verify that the pathspec matches the prefix */ if (pathspec) - verify_pathspec(); + prefix = verify_pathspec(prefix); /* Treat unmatching pathspec elements as errors */ if (pathspec && error_unmatch) { @@ -476,8 +475,8 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix) read_cache(); if (prefix) - prune_cache(); - show_files(&dir); + prune_cache(prefix); + show_files(&dir, prefix); if (ps_matched) { /* We need to make sure all pathspec matched otherwise From 9590b041ea464c46d5a6811df5bce83c5dd4d457 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 31 Jul 2006 02:53:46 -0700 Subject: [PATCH 3/5] Builtins: control the use of pager from the command table. This moves the built-in "always-use-pager" logic for log family to the command dispatch table of git wrapper. This makes it easier to change the default use of pager, and has an added benefit that we fork and exec the pager early before packs are mmapped. Pointed out by Juergen Ruehle . Signed-off-by: Junio C Hamano --- builtin-log.c | 1 - git.c | 13 ++++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/builtin-log.c b/builtin-log.c index 82c69d1d05..bba1496bf2 100644 --- a/builtin-log.c +++ b/builtin-log.c @@ -34,7 +34,6 @@ static int cmd_log_walk(struct rev_info *rev) struct commit *commit; prepare_revision_walk(rev); - setup_pager(); while ((commit = get_revision(rev)) != NULL) { log_tree_commit(rev, commit); free(commit->buffer); diff --git a/git.c b/git.c index 7321d6c3f6..d031eb9a18 100644 --- a/git.c +++ b/git.c @@ -211,6 +211,7 @@ static int handle_alias(int *argcp, const char ***argv) const char git_version_string[] = GIT_VERSION; #define NEEDS_PREFIX 1 +#define USE_PAGER 2 static void handle_internal_command(int argc, const char **argv, char **envp) { @@ -218,13 +219,13 @@ static void handle_internal_command(int argc, const char **argv, char **envp) static struct cmd_struct { const char *cmd; int (*fn)(int, const char **, const char *); - int prefix; + int option; } commands[] = { { "version", cmd_version }, { "help", cmd_help }, - { "log", cmd_log, NEEDS_PREFIX }, - { "whatchanged", cmd_whatchanged, NEEDS_PREFIX }, - { "show", cmd_show, NEEDS_PREFIX }, + { "log", cmd_log, NEEDS_PREFIX | USE_PAGER }, + { "whatchanged", cmd_whatchanged, NEEDS_PREFIX | USE_PAGER }, + { "show", cmd_show, NEEDS_PREFIX | USE_PAGER }, { "push", cmd_push }, { "format-patch", cmd_format_patch, NEEDS_PREFIX }, { "count-objects", cmd_count_objects }, @@ -275,8 +276,10 @@ static void handle_internal_command(int argc, const char **argv, char **envp) continue; prefix = NULL; - if (p->prefix) + if (p->option & NEEDS_PREFIX) prefix = setup_git_directory(); + if (p->option & USE_PAGER) + setup_pager(); if (getenv("GIT_TRACE")) { int i; fprintf(stderr, "trace: built-in: git"); From aa086eb813d4fe21aac556a94efe5e29b44d8ca4 Mon Sep 17 00:00:00 2001 From: Matthias Lederhofer Date: Sun, 30 Jul 2006 00:27:43 +0200 Subject: [PATCH 4/5] pager: config variable pager.color enable/disable colored output when the pager is in use Signed-off-by: Matthias Lederhofer Signed-off-by: Junio C Hamano --- Documentation/config.txt | 4 ++++ cache.h | 1 + config.c | 5 +++++ diff.c | 2 +- environment.c | 1 + 5 files changed, 12 insertions(+), 1 deletion(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 465eb13e76..e669003f72 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -116,6 +116,10 @@ apply.whitespace:: Tells `git-apply` how to handle whitespaces, in the same way as the '--whitespace' option. See gitlink:git-apply[1]. +pager.color:: + A boolean to enable/disable colored output when the pager is in + use (default is true). + diff.color:: When true (or `always`), always use colors in patch. When false (or `never`), never. When set to `auto`, use diff --git a/cache.h b/cache.h index c575b8a996..b8c21e07b2 100644 --- a/cache.h +++ b/cache.h @@ -386,6 +386,7 @@ extern int receive_keep_pack(int fd[2], const char *me, int quiet, int); /* pager.c */ extern void setup_pager(void); extern int pager_in_use; +extern int pager_use_color; /* base85 */ int decode_85(char *dst, char *line, int linelen); diff --git a/config.c b/config.c index 0ac6aebbbc..82b3562454 100644 --- a/config.c +++ b/config.c @@ -309,6 +309,11 @@ int git_default_config(const char *var, const char *value) return 0; } + if (!strcmp(var, "pager.color")) { + pager_use_color = git_config_bool(var,value); + return 0; + } + /* Add other config variables here and to Documentation/config.txt. */ return 0; } diff --git a/diff.c b/diff.c index 6a71376483..607c357f5a 100644 --- a/diff.c +++ b/diff.c @@ -175,7 +175,7 @@ int git_diff_ui_config(const char *var, const char *value) diff_use_color_default = 1; /* bool */ else if (!strcasecmp(value, "auto")) { diff_use_color_default = 0; - if (isatty(1) || pager_in_use) { + if (isatty(1) || (pager_in_use && pager_use_color)) { char *term = getenv("TERM"); if (term && strcmp(term, "dumb")) diff_use_color_default = 1; diff --git a/environment.c b/environment.c index 42f39d657e..87162b2572 100644 --- a/environment.c +++ b/environment.c @@ -23,6 +23,7 @@ int shared_repository = PERM_UMASK; const char *apply_default_whitespace = NULL; int zlib_compression_level = Z_DEFAULT_COMPRESSION; int pager_in_use; +int pager_use_color = 1; static char *git_dir, *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file; From c27d205aaefb654c12a4ab9e0b4fae1882c0fc70 Mon Sep 17 00:00:00 2001 From: Matthias Lederhofer Date: Mon, 31 Jul 2006 15:27:00 +0200 Subject: [PATCH 5/5] pager: environment variable GIT_PAGER to override PAGER Signed-off-by: Matthias Lederhofer Signed-off-by: Junio C Hamano --- Documentation/git.txt | 3 +++ pager.c | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Documentation/git.txt b/Documentation/git.txt index 7310a2b8b8..d24388368e 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -627,6 +627,9 @@ git Diffs other ~~~~~ +'GIT_PAGER':: + This environment variable overrides `$PAGER`. + 'GIT_TRACE':: If this variable is set git will print `trace:` messages on stderr telling about alias expansion, built-in command diff --git a/pager.c b/pager.c index 280f57f796..dcb398da8e 100644 --- a/pager.c +++ b/pager.c @@ -15,10 +15,12 @@ void setup_pager(void) { pid_t pid; int fd[2]; - const char *pager = getenv("PAGER"); + const char *pager = getenv("GIT_PAGER"); if (!isatty(1)) return; + if (!pager) + pager = getenv("PAGER"); if (!pager) pager = "less"; else if (!*pager || !strcmp(pager, "cat"))