* [PATCH 06/10] git_config_colorbool: refactor stdout_is_tty handling
From: Jeff King @ 2011-08-18 5:03 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Steffen Daode Nurpmeso, Ingo Brückl
In-Reply-To: <20110818045821.GA17377@sigill.intra.peff.net>
Usually this function figures out for itself whether stdout
is a tty. However, it has an extra parameter just to allow
git-config to override the auto-detection for its
--get-colorbool option.
Instead of an extra parameter, let's just use a global
variable. This makes calling easier in the common case, and
will make refactoring the colorbool code much simpler.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/branch.c | 2 +-
builtin/commit.c | 2 +-
builtin/config.c | 23 +++++++----------------
builtin/grep.c | 2 +-
builtin/show-branch.c | 2 +-
color.c | 11 ++++++-----
color.h | 8 +++++++-
diff.c | 4 ++--
parse-options.c | 2 +-
9 files changed, 27 insertions(+), 29 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 3142daa..b15fee5 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -71,7 +71,7 @@ static int parse_branch_color_slot(const char *var, int ofs)
static int git_branch_config(const char *var, const char *value, void *cb)
{
if (!strcmp(var, "color.branch")) {
- branch_use_color = git_config_colorbool(var, value, -1);
+ branch_use_color = git_config_colorbool(var, value);
return 0;
}
if (!prefixcmp(var, "color.branch.")) {
diff --git a/builtin/commit.c b/builtin/commit.c
index e1af9b1..295803a 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1144,7 +1144,7 @@ static int git_status_config(const char *k, const char *v, void *cb)
return 0;
}
if (!strcmp(k, "status.color") || !strcmp(k, "color.status")) {
- s->use_color = git_config_colorbool(k, v, -1);
+ s->use_color = git_config_colorbool(k, v);
return 0;
}
if (!prefixcmp(k, "status.color.") || !prefixcmp(k, "color.status.")) {
diff --git a/builtin/config.c b/builtin/config.c
index 211e118..5505ced 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -303,24 +303,17 @@ static void get_color(const char *def_color)
fputs(parsed_color, stdout);
}
-static int stdout_is_tty;
static int get_colorbool_found;
static int get_diff_color_found;
static int git_get_colorbool_config(const char *var, const char *value,
void *cb)
{
- if (!strcmp(var, get_colorbool_slot)) {
- get_colorbool_found =
- git_config_colorbool(var, value, stdout_is_tty);
- }
- if (!strcmp(var, "diff.color")) {
- get_diff_color_found =
- git_config_colorbool(var, value, stdout_is_tty);
- }
- if (!strcmp(var, "color.ui")) {
- git_use_color_default = git_config_colorbool(var, value, stdout_is_tty);
- return 0;
- }
+ if (!strcmp(var, get_colorbool_slot))
+ get_colorbool_found = git_config_colorbool(var, value);
+ else if (!strcmp(var, "diff.color"))
+ get_diff_color_found = git_config_colorbool(var, value);
+ else if (!strcmp(var, "color.ui"))
+ git_use_color_default = git_config_colorbool(var, value);
return 0;
}
@@ -510,9 +503,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
}
else if (actions == ACTION_GET_COLORBOOL) {
if (argc == 1)
- stdout_is_tty = git_config_bool("command line", argv[0]);
- else if (argc == 0)
- stdout_is_tty = isatty(1);
+ color_stdout_is_tty = git_config_bool("command line", argv[0]);
return get_colorbool(argc != 0);
}
diff --git a/builtin/grep.c b/builtin/grep.c
index cccf8da..d80db22 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -325,7 +325,7 @@ static int grep_config(const char *var, const char *value, void *cb)
}
if (!strcmp(var, "color.grep"))
- opt->color = git_config_colorbool(var, value, -1);
+ opt->color = git_config_colorbool(var, value);
else if (!strcmp(var, "color.grep.context"))
color = opt->color_context;
else if (!strcmp(var, "color.grep.filename"))
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index facc63a..e6650b4 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -573,7 +573,7 @@ static int git_show_branch_config(const char *var, const char *value, void *cb)
}
if (!strcmp(var, "color.showbranch")) {
- showbranch_use_color = git_config_colorbool(var, value, -1);
+ showbranch_use_color = git_config_colorbool(var, value);
return 0;
}
diff --git a/color.c b/color.c
index 3db214c..67affa4 100644
--- a/color.c
+++ b/color.c
@@ -2,6 +2,7 @@
#include "color.h"
int git_use_color_default = 0;
+int color_stdout_is_tty = -1;
/*
* The list of available column colors.
@@ -157,7 +158,7 @@ bad:
die("bad color value '%.*s' for variable '%s'", value_len, value, var);
}
-int git_config_colorbool(const char *var, const char *value, int stdout_is_tty)
+int git_config_colorbool(const char *var, const char *value)
{
if (value) {
if (!strcasecmp(value, "never"))
@@ -177,9 +178,9 @@ int git_config_colorbool(const char *var, const char *value, int stdout_is_tty)
/* any normal truth value defaults to 'auto' */
auto_color:
- if (stdout_is_tty < 0)
- stdout_is_tty = isatty(1);
- if (stdout_is_tty || (pager_in_use() && pager_use_color)) {
+ if (color_stdout_is_tty < 0)
+ color_stdout_is_tty = isatty(1);
+ if (color_stdout_is_tty || (pager_in_use() && pager_use_color)) {
char *term = getenv("TERM");
if (term && strcmp(term, "dumb"))
return 1;
@@ -190,7 +191,7 @@ int git_config_colorbool(const char *var, const char *value, int stdout_is_tty)
int git_color_default_config(const char *var, const char *value, void *cb)
{
if (!strcmp(var, "color.ui")) {
- git_use_color_default = git_config_colorbool(var, value, -1);
+ git_use_color_default = git_config_colorbool(var, value);
return 0;
}
diff --git a/color.h b/color.h
index 68a926a..a190a25 100644
--- a/color.h
+++ b/color.h
@@ -58,11 +58,17 @@ extern const char *column_colors_ansi[];
extern const int column_colors_ansi_max;
/*
+ * Generally the color code will lazily figure this out itself, but
+ * this provides a mechanism for callers to override autodetection.
+ */
+extern int color_stdout_is_tty;
+
+/*
* Use this instead of git_default_config if you need the value of color.ui.
*/
int git_color_default_config(const char *var, const char *value, void *cb);
-int git_config_colorbool(const char *var, const char *value, int stdout_is_tty);
+int git_config_colorbool(const char *var, const char *value);
void color_parse(const char *value, const char *var, char *dst);
void color_parse_mem(const char *value, int len, const char *var, char *dst);
__attribute__((format (printf, 3, 4)))
diff --git a/diff.c b/diff.c
index 2d86abe..2dfc359 100644
--- a/diff.c
+++ b/diff.c
@@ -137,7 +137,7 @@ static int git_config_rename(const char *var, const char *value)
int git_diff_ui_config(const char *var, const char *value, void *cb)
{
if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) {
- diff_use_color_default = git_config_colorbool(var, value, -1);
+ diff_use_color_default = git_config_colorbool(var, value);
return 0;
}
if (!strcmp(var, "diff.renames")) {
@@ -3409,7 +3409,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
else if (!strcmp(arg, "--color"))
options->use_color = 1;
else if (!prefixcmp(arg, "--color=")) {
- int value = git_config_colorbool(NULL, arg+8, -1);
+ int value = git_config_colorbool(NULL, arg+8);
if (value == 0)
options->use_color = 0;
else if (value > 0)
diff --git a/parse-options.c b/parse-options.c
index 879ea82..be4383e 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -621,7 +621,7 @@ int parse_opt_color_flag_cb(const struct option *opt, const char *arg,
if (!arg)
arg = unset ? "never" : (const char *)opt->defval;
- value = git_config_colorbool(NULL, arg, -1);
+ value = git_config_colorbool(NULL, arg);
if (value < 0)
return opterror(opt,
"expects \"always\", \"auto\", or \"never\"", 0);
--
1.7.6.10.g62f04
^ permalink raw reply related
* [PATCH 05/10] diff: refactor COLOR_DIFF from a flag into an int
From: Jeff King @ 2011-08-18 5:03 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Steffen Daode Nurpmeso, Ingo Brückl
In-Reply-To: <20110818045821.GA17377@sigill.intra.peff.net>
This lets us store more than just a bit flag for whether we
want color; we can also store whether we want automatic
colors. This can be useful for making the automatic-color
decision closer to the point of use.
This mostly just involves replacing DIFF_OPT_* calls with
manipulations of the flag. The biggest exception is that
calls to DIFF_OPT_TST must check for "o->use_color > 0",
which lets an "unknown" value (i.e., the default) stay at
"no color". In the previous code, a value of "-1" was not
propagated at all.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/merge.c | 2 --
combine-diff.c | 7 +++----
diff.c | 41 +++++++++++++++++++----------------------
diff.h | 5 +++--
graph.c | 2 +-
log-tree.c | 4 ++--
wt-status.c | 2 +-
7 files changed, 29 insertions(+), 34 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 325891e..7209edf 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -390,8 +390,6 @@ static void finish(const unsigned char *new_head, const char *msg)
opts.output_format |=
DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
opts.detect_rename = DIFF_DETECT_RENAME;
- if (diff_use_color_default > 0)
- DIFF_OPT_SET(&opts, COLOR_DIFF);
if (diff_setup_done(&opts) < 0)
die(_("diff_setup_done failed"));
diff_tree_sha1(head, new_head, "", &opts);
diff --git a/combine-diff.c b/combine-diff.c
index be67cfc..c588c79 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -702,9 +702,8 @@ static void show_combined_header(struct combine_diff_path *elem,
int abbrev = DIFF_OPT_TST(opt, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
const char *a_prefix = opt->a_prefix ? opt->a_prefix : "a/";
const char *b_prefix = opt->b_prefix ? opt->b_prefix : "b/";
- int use_color = DIFF_OPT_TST(opt, COLOR_DIFF);
- const char *c_meta = diff_get_color(use_color, DIFF_METAINFO);
- const char *c_reset = diff_get_color(use_color, DIFF_RESET);
+ const char *c_meta = diff_get_color_opt(opt, DIFF_METAINFO);
+ const char *c_reset = diff_get_color_opt(opt, DIFF_RESET);
const char *abb;
int added = 0;
int deleted = 0;
@@ -964,7 +963,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
show_combined_header(elem, num_parent, dense, rev,
mode_differs, 1);
dump_sline(sline, cnt, num_parent,
- DIFF_OPT_TST(opt, COLOR_DIFF), result_deleted);
+ opt->use_color, result_deleted);
}
free(result);
diff --git a/diff.c b/diff.c
index 93ef9a2..2d86abe 100644
--- a/diff.c
+++ b/diff.c
@@ -583,11 +583,10 @@ static void emit_rewrite_diff(const char *name_a,
struct diff_options *o)
{
int lc_a, lc_b;
- int color_diff = DIFF_OPT_TST(o, COLOR_DIFF);
const char *name_a_tab, *name_b_tab;
- const char *metainfo = diff_get_color(color_diff, DIFF_METAINFO);
- const char *fraginfo = diff_get_color(color_diff, DIFF_FRAGINFO);
- const char *reset = diff_get_color(color_diff, DIFF_RESET);
+ const char *metainfo = diff_get_color(o->use_color, DIFF_METAINFO);
+ const char *fraginfo = diff_get_color(o->use_color, DIFF_FRAGINFO);
+ const char *reset = diff_get_color(o->use_color, DIFF_RESET);
static struct strbuf a_name = STRBUF_INIT, b_name = STRBUF_INIT;
const char *a_prefix, *b_prefix;
char *data_one, *data_two;
@@ -623,7 +622,7 @@ static void emit_rewrite_diff(const char *name_a,
size_two = fill_textconv(textconv_two, two, &data_two);
memset(&ecbdata, 0, sizeof(ecbdata));
- ecbdata.color_diff = color_diff;
+ ecbdata.color_diff = o->use_color > 0;
ecbdata.found_changesp = &o->found_changes;
ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);
ecbdata.opt = o;
@@ -1004,7 +1003,7 @@ static void free_diff_words_data(struct emit_callback *ecbdata)
const char *diff_get_color(int diff_use_color, enum color_diff ix)
{
- if (diff_use_color)
+ if (diff_use_color > 0)
return diff_colors[ix];
return "";
}
@@ -1808,11 +1807,10 @@ static int is_conflict_marker(const char *line, int marker_size, unsigned long l
static void checkdiff_consume(void *priv, char *line, unsigned long len)
{
struct checkdiff_t *data = priv;
- int color_diff = DIFF_OPT_TST(data->o, COLOR_DIFF);
int marker_size = data->conflict_marker_size;
- const char *ws = diff_get_color(color_diff, DIFF_WHITESPACE);
- const char *reset = diff_get_color(color_diff, DIFF_RESET);
- const char *set = diff_get_color(color_diff, DIFF_FILE_NEW);
+ const char *ws = diff_get_color(data->o->use_color, DIFF_WHITESPACE);
+ const char *reset = diff_get_color(data->o->use_color, DIFF_RESET);
+ const char *set = diff_get_color(data->o->use_color, DIFF_FILE_NEW);
char *err;
char *line_prefix = "";
struct strbuf *msgbuf;
@@ -2157,7 +2155,7 @@ static void builtin_diff(const char *name_a,
memset(&xecfg, 0, sizeof(xecfg));
memset(&ecbdata, 0, sizeof(ecbdata));
ecbdata.label_path = lbl;
- ecbdata.color_diff = DIFF_OPT_TST(o, COLOR_DIFF);
+ ecbdata.color_diff = o->use_color > 0;
ecbdata.found_changesp = &o->found_changes;
ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);
if (ecbdata.ws_rule & WS_BLANK_AT_EOF)
@@ -2205,7 +2203,7 @@ static void builtin_diff(const char *name_a,
break;
}
}
- if (DIFF_OPT_TST(o, COLOR_DIFF)) {
+ if (o->use_color > 0) {
struct diff_words_style *st = ecbdata.diff_words->style;
st->old.color = diff_get_color_opt(o, DIFF_FILE_OLD);
st->new.color = diff_get_color_opt(o, DIFF_FILE_NEW);
@@ -2855,7 +2853,7 @@ static void run_diff_cmd(const char *pgm,
*/
fill_metainfo(msg, name, other, one, two, o, p,
&must_show_header,
- DIFF_OPT_TST(o, COLOR_DIFF) && !pgm);
+ o->use_color > 0 && !pgm);
xfrm_msg = msg->len ? msg->buf : NULL;
}
@@ -3021,8 +3019,7 @@ void diff_setup(struct diff_options *options)
options->change = diff_change;
options->add_remove = diff_addremove;
- if (diff_use_color_default > 0)
- DIFF_OPT_SET(options, COLOR_DIFF);
+ options->use_color = diff_use_color_default;
options->detect_rename = diff_detect_rename_default;
if (diff_no_prefix) {
@@ -3410,24 +3407,24 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
else if (!strcmp(arg, "--follow"))
DIFF_OPT_SET(options, FOLLOW_RENAMES);
else if (!strcmp(arg, "--color"))
- DIFF_OPT_SET(options, COLOR_DIFF);
+ options->use_color = 1;
else if (!prefixcmp(arg, "--color=")) {
int value = git_config_colorbool(NULL, arg+8, -1);
if (value == 0)
- DIFF_OPT_CLR(options, COLOR_DIFF);
+ options->use_color = 0;
else if (value > 0)
- DIFF_OPT_SET(options, COLOR_DIFF);
+ options->use_color = 1;
else
return error("option `color' expects \"always\", \"auto\", or \"never\"");
}
else if (!strcmp(arg, "--no-color"))
- DIFF_OPT_CLR(options, COLOR_DIFF);
+ options->use_color = 0;
else if (!strcmp(arg, "--color-words")) {
- DIFF_OPT_SET(options, COLOR_DIFF);
+ options->use_color = 1;
options->word_diff = DIFF_WORDS_COLOR;
}
else if (!prefixcmp(arg, "--color-words=")) {
- DIFF_OPT_SET(options, COLOR_DIFF);
+ options->use_color = 1;
options->word_diff = DIFF_WORDS_COLOR;
options->word_regex = arg + 14;
}
@@ -3440,7 +3437,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
if (!strcmp(type, "plain"))
options->word_diff = DIFF_WORDS_PLAIN;
else if (!strcmp(type, "color")) {
- DIFF_OPT_SET(options, COLOR_DIFF);
+ options->use_color = 1;
options->word_diff = DIFF_WORDS_COLOR;
}
else if (!strcmp(type, "porcelain"))
diff --git a/diff.h b/diff.h
index b920a20..8c66b59 100644
--- a/diff.h
+++ b/diff.h
@@ -58,7 +58,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
#define DIFF_OPT_SILENT_ON_REMOVE (1 << 5)
#define DIFF_OPT_FIND_COPIES_HARDER (1 << 6)
#define DIFF_OPT_FOLLOW_RENAMES (1 << 7)
-#define DIFF_OPT_COLOR_DIFF (1 << 8)
+/* (1 << 8) unused */
/* (1 << 9) unused */
#define DIFF_OPT_HAS_CHANGES (1 << 10)
#define DIFF_OPT_QUICK (1 << 11)
@@ -101,6 +101,7 @@ struct diff_options {
const char *single_follow;
const char *a_prefix, *b_prefix;
unsigned flags;
+ int use_color;
int context;
int interhunkcontext;
int break_opt;
@@ -160,7 +161,7 @@ enum color_diff {
};
const char *diff_get_color(int diff_use_color, enum color_diff ix);
#define diff_get_color_opt(o, ix) \
- diff_get_color(DIFF_OPT_TST((o), COLOR_DIFF), ix)
+ diff_get_color((o)->use_color, ix)
extern const char mime_boundary_leader[];
diff --git a/graph.c b/graph.c
index 2f6893d..556834a 100644
--- a/graph.c
+++ b/graph.c
@@ -347,7 +347,7 @@ static struct commit_list *first_interesting_parent(struct git_graph *graph)
static unsigned short graph_get_current_column_color(const struct git_graph *graph)
{
- if (!DIFF_OPT_TST(&graph->revs->diffopt, COLOR_DIFF))
+ if (graph->revs->diffopt.use_color <= 0)
return column_colors_max;
return graph->default_column_color;
}
diff --git a/log-tree.c b/log-tree.c
index e945701..9ba8fb2 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -31,7 +31,7 @@ static char decoration_colors[][COLOR_MAXLEN] = {
static const char *decorate_get_color(int decorate_use_color, enum decoration_type ix)
{
- if (decorate_use_color)
+ if (decorate_use_color > 0)
return decoration_colors[ix];
return "";
}
@@ -77,7 +77,7 @@ int parse_decorate_color_config(const char *var, const int ofs, const char *valu
* for showing the commit sha1, use the same check for --decorate
*/
#define decorate_get_color_opt(o, ix) \
- decorate_get_color(DIFF_OPT_TST((o), COLOR_DIFF), ix)
+ decorate_get_color((o)->use_color, ix)
static void add_name_decoration(enum decoration_type type, const char *name, struct object *obj)
{
diff --git a/wt-status.c b/wt-status.c
index 0237772..ee03431 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -681,7 +681,7 @@ static void wt_status_print_verbose(struct wt_status *s)
* will have checked isatty on stdout).
*/
if (s->fp != stdout)
- DIFF_OPT_CLR(&rev.diffopt, COLOR_DIFF);
+ rev.diffopt.use_color = 0;
run_diff_index(&rev, 1);
}
--
1.7.6.10.g62f04
^ permalink raw reply related
* [PATCH 04/10] setup_pager: set GIT_PAGER_IN_USE
From: Jeff King @ 2011-08-18 5:02 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Steffen Daode Nurpmeso, Ingo Brückl
In-Reply-To: <20110818045821.GA17377@sigill.intra.peff.net>
We have always set a global "spawned_pager" variable when we
start the pager. This lets us make the auto-color decision
later in the program as as "we are outputting to a terminal,
or to a pager which can handle colors".
Commit 6e9af86 added support for the GIT_PAGER_IN_USE
environment variable. An external program calling git (e.g.,
git-svn) could set this variable to indicate that it had
already started the pager, and that the decision about
auto-coloring should take that into account.
However, 6e9af86 failed to do the reverse, which is to tell
external programs when git itself has started the pager.
Thus a git command implemented as an external script that
has the pager turned on (e.g., "git -p stash show") would
not realize it was going to a pager, and would suppress
colors.
This patch remedies that; we always set GIT_PAGER_IN_USE
when we start the pager, and the value is respected by both
this program and any spawned children.
Signed-off-by: Jeff King <peff@peff.net>
---
| 8 +-------
| 11 +++++++++++
2 files changed, 12 insertions(+), 7 deletions(-)
--git a/pager.c b/pager.c
index dac358f..975955b 100644
--- a/pager.c
+++ b/pager.c
@@ -11,8 +11,6 @@
* something different on Windows.
*/
-static int spawned_pager;
-
#ifndef WIN32
static void pager_preexec(void)
{
@@ -78,7 +76,7 @@ void setup_pager(void)
if (!pager)
return;
- spawned_pager = 1; /* means we are emitting to terminal */
+ setenv("GIT_PAGER_IN_USE", "true", 1);
/* spawn the pager */
pager_argv[0] = pager;
@@ -109,10 +107,6 @@ void setup_pager(void)
int pager_in_use(void)
{
const char *env;
-
- if (spawned_pager)
- return 1;
-
env = getenv("GIT_PAGER_IN_USE");
return env ? git_config_bool("GIT_PAGER_IN_USE", env) : 0;
}
--git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 2ac729f..4884e1b 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -181,6 +181,17 @@ test_expect_success 'color when writing to a file intended for a pager' '
colorful colorful.log
'
+test_expect_success TTY 'colors are sent to pager for external commands' '
+ test_config alias.externallog "!git log" &&
+ test_config color.ui auto &&
+ (
+ TERM=vt100 &&
+ export TERM &&
+ test_terminal git -p externallog
+ ) &&
+ colorful paginated.out
+'
+
# Use this helper to make it easy for the caller of your
# terminal-using function to specify whether it should fail.
# If you write
--
1.7.6.10.g62f04
^ permalink raw reply related
* [PATCH 03/10] t7006: use test_config helpers
From: Jeff King @ 2011-08-18 5:02 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Steffen Daode Nurpmeso, Ingo Brückl
In-Reply-To: <20110818045821.GA17377@sigill.intra.peff.net>
In some cases, this is just making the test script a little
shorter and easier to read. However, there are several
places where we didn't take proper precautions against
polluting downstream tests with our config; this fixes them,
too.
Signed-off-by: Jeff King <peff@peff.net>
---
| 39 ++++++++++++++++++---------------------
1 files changed, 18 insertions(+), 21 deletions(-)
--git a/t/t7006-pager.sh b/t/t7006-pager.sh
index c0a3135..2ac729f 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -13,7 +13,7 @@ cleanup_fail() {
test_expect_success 'setup' '
sane_unset GIT_PAGER GIT_PAGER_IN_USE &&
- test_might_fail git config --unset core.pager &&
+ test_unconfig core.pager &&
PAGER="cat >paginated.out" &&
export PAGER &&
@@ -94,21 +94,19 @@ test_expect_success TTY 'no pager with --no-pager' '
test_expect_success TTY 'configuration can disable pager' '
rm -f paginated.out &&
- test_might_fail git config --unset pager.grep &&
+ test_unconfig pager.grep &&
test_terminal git grep initial &&
test -e paginated.out &&
rm -f paginated.out &&
- git config pager.grep false &&
- test_when_finished "git config --unset pager.grep" &&
+ test_config pager.grep false &&
test_terminal git grep initial &&
! test -e paginated.out
'
test_expect_success TTY 'git config uses a pager if configured to' '
rm -f paginated.out &&
- git config pager.config true &&
- test_when_finished "git config --unset pager.config" &&
+ test_config pager.config true &&
test_terminal git config --list &&
test -e paginated.out
'
@@ -116,8 +114,7 @@ test_expect_success TTY 'git config uses a pager if configured to' '
test_expect_success TTY 'configuration can enable pager (from subdir)' '
rm -f paginated.out &&
mkdir -p subdir &&
- git config pager.bundle true &&
- test_when_finished "git config --unset pager.bundle" &&
+ test_config pager.bundle true &&
git bundle create test.bundle --all &&
rm -f paginated.out subdir/paginated.out &&
@@ -150,7 +147,7 @@ test_expect_success 'tests can detect color' '
test_expect_success 'no color when stdout is a regular file' '
rm -f colorless.log &&
- git config color.ui auto ||
+ test_config color.ui auto ||
cleanup_fail &&
git log >colorless.log &&
@@ -159,7 +156,7 @@ test_expect_success 'no color when stdout is a regular file' '
test_expect_success TTY 'color when writing to a pager' '
rm -f paginated.out &&
- git config color.ui auto ||
+ test_config color.ui auto ||
cleanup_fail &&
(
@@ -172,7 +169,7 @@ test_expect_success TTY 'color when writing to a pager' '
test_expect_success 'color when writing to a file intended for a pager' '
rm -f colorful.log &&
- git config color.ui auto ||
+ test_config color.ui auto ||
cleanup_fail &&
(
@@ -221,7 +218,7 @@ test_default_pager() {
$test_expectation SIMPLEPAGER,TTY "$cmd - default pager is used by default" "
sane_unset PAGER GIT_PAGER &&
- test_might_fail git config --unset core.pager &&
+ test_unconfig core.pager &&
rm -f default_pager_used ||
cleanup_fail &&
@@ -244,7 +241,7 @@ test_PAGER_overrides() {
$test_expectation TTY "$cmd - PAGER overrides default pager" "
sane_unset GIT_PAGER &&
- test_might_fail git config --unset core.pager &&
+ test_unconfig core.pager &&
rm -f PAGER_used ||
cleanup_fail &&
@@ -277,7 +274,7 @@ test_core_pager() {
PAGER=wc &&
export PAGER &&
- git config core.pager 'wc >core.pager_used' &&
+ test_config core.pager 'wc >core.pager_used' &&
$full_command &&
${if_local_config}test -e core.pager_used
"
@@ -307,7 +304,7 @@ test_pager_subdir_helper() {
PAGER=wc &&
stampname=\$(pwd)/core.pager_used &&
export PAGER stampname &&
- git config core.pager 'wc >\"\$stampname\"' &&
+ test_config core.pager 'wc >\"\$stampname\"' &&
mkdir sub &&
(
cd sub &&
@@ -324,7 +321,7 @@ test_GIT_PAGER_overrides() {
rm -f GIT_PAGER_used ||
cleanup_fail &&
- git config core.pager wc &&
+ test_config core.pager wc &&
GIT_PAGER='wc >GIT_PAGER_used' &&
export GIT_PAGER &&
$full_command &&
@@ -405,8 +402,8 @@ test_expect_success TTY 'command-specific pager' '
sane_unset PAGER GIT_PAGER &&
echo "foo:initial" >expect &&
>actual &&
- git config --unset core.pager &&
- git config pager.log "sed s/^/foo:/ >actual" &&
+ test_unconfig core.pager &&
+ test_config pager.log "sed s/^/foo:/ >actual" &&
test_terminal git log --format=%s -1 &&
test_cmp expect actual
'
@@ -415,8 +412,8 @@ test_expect_success TTY 'command-specific pager overrides core.pager' '
sane_unset PAGER GIT_PAGER &&
echo "foo:initial" >expect &&
>actual &&
- git config core.pager "exit 1"
- git config pager.log "sed s/^/foo:/ >actual" &&
+ test_config core.pager "exit 1"
+ test_config pager.log "sed s/^/foo:/ >actual" &&
test_terminal git log --format=%s -1 &&
test_cmp expect actual
'
@@ -425,7 +422,7 @@ test_expect_success TTY 'command-specific pager overridden by environment' '
GIT_PAGER="sed s/^/foo:/ >actual" && export GIT_PAGER &&
>actual &&
echo "foo:initial" >expect &&
- git config pager.log "exit 1" &&
+ test_config pager.log "exit 1" &&
test_terminal git log --format=%s -1 &&
test_cmp expect actual
'
--
1.7.6.10.g62f04
^ permalink raw reply related
* [PATCH 02/10] test-lib: add helper functions for config
From: Jeff King @ 2011-08-18 5:01 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Steffen Daode Nurpmeso, Ingo Brückl
In-Reply-To: <20110818045821.GA17377@sigill.intra.peff.net>
There are a few common tasks when working with configuration
variables in tests; this patch aims to make them a little
easier to write and less error-prone.
When setting a variable, you should typically make sure to
clean it up after the test is finished, so as not to pollute
other tests. Like:
test_when_finished 'git config --unset foo.bar' &&
git config foo.bar baz
This patch lets you just write:
test_config foo.bar baz
When clearing a variable that does not exist, git-config
will report a specific non-zero error code. Meaning that
tests which call "git config --unset" often either rely on
the prior tests having actually set it, or must use
test_might_fail. With this patch, the previous:
test_might_fail git config --unset foo.bar
becomes:
test_unconfig foo.bar
Not only is this easier to type, but it is more robust; it
will correctly detect errors from git-config besides "key
was not set".
Signed-off-by: Jeff King <peff@peff.net>
---
t/test-lib.sh | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index df25f17..926667a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -357,6 +357,24 @@ test_chmod () {
git update-index --add "--chmod=$@"
}
+# Unset a configuration variable, but don't fail if it doesn't exist.
+test_unconfig () {
+ git config --unset-all "$@"
+ config_status=$?
+ case "$config_status" in
+ 5) # ok, nothing to usnet
+ config_status=0
+ ;;
+ esac
+ return $config_status
+}
+
+# Set git config, automatically unsetting it after the test is over.
+test_config () {
+ test_when_finished "test_unconfig '$1'" &&
+ git config "$@"
+}
+
# Use test_set_prereq to tell that a particular prerequisite is available.
# The prerequisite can later be checked for in two ways:
#
--
1.7.6.10.g62f04
^ permalink raw reply related
* [PATCH 01/10] t7006: modernize calls to unset
From: Jeff King @ 2011-08-18 5:00 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Steffen Daode Nurpmeso, Ingo Brückl
In-Reply-To: <20110818045821.GA17377@sigill.intra.peff.net>
These tests break &&-chaining to deal with broken "unset"
implementations. Instead, they should just use sane_unset.
Signed-off-by: Jeff King <peff@peff.net>
---
| 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
--git a/t/t7006-pager.sh b/t/t7006-pager.sh
index ed7575d..c0a3135 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -402,7 +402,7 @@ test_core_pager_subdir expect_success test_must_fail \
'git -p apply </dev/null'
test_expect_success TTY 'command-specific pager' '
- unset PAGER GIT_PAGER;
+ sane_unset PAGER GIT_PAGER &&
echo "foo:initial" >expect &&
>actual &&
git config --unset core.pager &&
@@ -412,7 +412,7 @@ test_expect_success TTY 'command-specific pager' '
'
test_expect_success TTY 'command-specific pager overrides core.pager' '
- unset PAGER GIT_PAGER;
+ sane_unset PAGER GIT_PAGER &&
echo "foo:initial" >expect &&
>actual &&
git config core.pager "exit 1"
--
1.7.6.10.g62f04
^ permalink raw reply related
* [PATCH 0/10] color and pager improvements
From: Jeff King @ 2011-08-18 4:58 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Steffen Daode Nurpmeso, Ingo Brückl
While looking at the pager and color code today, I decided to tackle two
long-standing bugs, which entailed a lot of refactoring of the color
code. The result fixes some minor bugs, and is a little nicer for
calling code to use.
[01/10]: t7006: modernize calls to unset
[02/10]: test-lib: add helper functions for config
[03/10]: t7006: use test_config helpers
These three are just cleanup I noticed before adding new tests to t7006;
I hope that the helpers in 02/10 will be useful in a lot of other
places, though.
[04/10]: setup_pager: set GIT_PAGER_IN_USE
This fixes Ingo's problem from:
http://article.gmane.org/gmane.comp.version-control.git/179430
Namely that:
git -p stash show
fails to use colors properly.
[05/10]: diff: refactor COLOR_DIFF from a flag into an int
[06/10]: git_config_colorbool: refactor stdout_is_tty handling
[07/10]: color: delay auto-color decision until point of use
These three fix the problem Steffen mentioned here:
http://article.gmane.org/gmane.comp.version-control.git/177792
Namely that pager.color doesn't work in many cases. This has been a
problem for years, but spread due to some pager-ordering changes late
last year (see the comments in 07/10). I actually wonder if anyone is
really using pager.color, as we haven't seen many complaints about it.
[08/10]: config: refactor get_colorbool function
[09/10]: diff: don't load color config in plumbing
[10/10]: want_color: automatically fallback to color.ui
These three are refactoring that is made possible by 07/10. I think they
make the code cleaner, and hopefully the diffstat of 10/10 speaks for
itself.
-Peff
^ permalink raw reply
* Re: Branches & directories
From: Jonathan Nieder @ 2011-08-18 4:45 UTC (permalink / raw)
To: Hilco Wijbenga; +Cc: Junio C Hamano, Evan Shelhamer, Git Mailing List
In-Reply-To: <CAE1pOi3rqqcz_6QxB8=g2jWOF-4SRZee7t8NXN1md2C4DL7wug@mail.gmail.com>
Hi Hilco,
Hilco Wijbenga wrote:
> It would be really nice, though, if Git could somehow
> "stash" such files when checking out a different branch. In general, I
> would prefer if uncommitted changes and untracked and/or ignored files
> stuck to the branch where they were created.
This is just a random guess, but: wouldn't it be convenient in this
workflow to have a separate worktree for each branch you are working
on? That way, switching branches would not carry over unwanted state
or throw away valuable state, clobber timestamps that "make" pays
attention to, etc.
If I am understanding correctly, then the git-new-workdir script
from contrib/workdir might help. (Note, though, that it comes with
some caveats. A quick mailing list search should find them.)
^ permalink raw reply
* Re: [PATCH] stash: Utilize config variable pager.stash.list in stash list command
From: Jeff King @ 2011-08-18 4:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ingo Brückl, git
In-Reply-To: <7vd3g3ev3j.fsf@alter.siamese.dyndns.org>
On Wed, Aug 17, 2011 at 11:44:32AM -0700, Junio C Hamano wrote:
> At least "pager.stash.list" should be spelled as "pager.stashList" or
> something. It is not like there are multitude of arbigrary choices that
> may match "pager.*.list" pattern.
The pager code would then assume that was for a command "stashlist". It
probably doesn't matter in practice, but I think it's a little nicer to
keep the namespace properly separated unless there is a good reason not
to.
There are other places where something like this might be handy, too.
For example, auto-pagination of "git branch" or "git tag -l" (but you
wouldn't want to paginate "git branch foo").
> Also a variable can be set to false by setting it to 0, no, etc., so
> you need to inspect it with "git config --bool" to get the canonical
> version of its value.
>
> What's so difficult to say "git stash list | less" or even "git -p
> stash list"?
Couldn't one make the same argument about git's entire use of the pager?
Anyway, I think his problem is not "I want a pager but I am too lazy to
type it", but rather that "git stash list" will auto-paginate by
default, because it is chaining to "log", which auto-paginates. You can
turn it off with "--no-pager", but pager.stash seems to have no effect.
-Peff
^ permalink raw reply
* Re: [PATCH] Disallow creating ambiguous branch names by default
From: Vijay Lakshminarayanan @ 2011-08-18 3:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Conrad Irwin, git
In-Reply-To: <7vhb5fev8a.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> We should be giving these warning messages immediately after creating
> potentially problematic refs, i.e. just after "git branch v1.0.0" and
> "git checkout -b v1.0.0". The user experience should look like this
> instead:
>
> $ git branch v1.0.0
> warning: refname 'v1.0.0' is ambiguous.
> advice: you may want to rename it to an unambigous name with
> advice: git branch -m v1.0.0 v1.0.0-branch
> $ git branch -m v1.0.0 v1.0.0-branch ;# thanks for an advice
>
> $ git checkout -b v1.0.0
> warning: refname 'v1.0.0' is ambiguous.
> advice: you may want to rename it to an unambigous name with
> advice: git branch -m v1.0.0-branch-2
> $ git branch -m v1.0.0-branch-2 ;# thanks for an advice
I'm not familiar with the git codebase, but I'm guessing this is
ambiguous because there's already a tag by name v1.0.0. /If/ that's the
case, wouldn't be be prudent to explain why the branch name is
ambiguous?
Just my 2c.
--
Cheers
~vijay
Gnus should be more complicated.
^ permalink raw reply
* Re: [PATCH v2] commit: accept tag objects in HEAD/MERGE_HEAD
From: Nguyen Thai Ngoc Duy @ 2011-08-18 2:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy5yrex70.fsf@alter.siamese.dyndns.org>
2011/8/18 Junio C Hamano <gitster@pobox.com>:
> By the way, what happens when you try to merge when HEAD points at a tag
> that points at a commit? Would we end up creating a commit that points at
> a bogus parent?
I guess I'd segfault the same way commit does. It does
lookup_commit(head) without checking the result.
>> diff --git a/builtin/commit.c b/builtin/commit.c
>> index 2088b6b..f327595 100644
>> --- a/builtin/commit.c
>> +++ b/builtin/commit.c
>> @@ -1387,6 +1387,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>> unsigned char commit_sha1[20];
>> struct ref_lock *ref_lock;
>> struct commit_list *parents = NULL, **pptr = &parents;
>> + struct commit *commit;
>> struct stat statbuf;
>> int allow_fast_forward = 1;
>> struct wt_status s;
>
> Here, you are being inconsistent with your own argument you made in your
> previous message "later somebody may forget to update the former while
> updating the latter" when I suggested to separate the two logically
> separate operations (grab the head_commit object when necessary, and
> decide how the commit is made). By hoisting of the scope of "commit", you
> made the variable undefined when dealing with the initial_commit, exposing
> the code to the same risk that somebody may try to use "commit" variable
> after the if/elseif/... cascade, where it may or may not be defined.
Fair enough.
>> @@ -1423,12 +1424,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>> reflog_msg = "commit (initial)";
>> } else if (amend) {
>> struct commit_list *c;
>> - struct commit *commit;
>>
>> if (!reflog_msg)
>> reflog_msg = "commit (amend)";
>> - commit = lookup_commit(head_sha1);
>> - if (!commit || parse_commit(commit))
>> + commit = lookup_expect_commit(head_sha1, "HEAD");
>> + if (parse_commit(commit))
>> die(_("could not parse HEAD commit"));
>
> Is this still necessary? I think your lookup_expect_commit() already
> checks this condition and barfs.
It is. lookup_expect_commit() does not parse commit content, only
makes sure it's a commit object.
--
Duy
^ permalink raw reply
* Re: [PATCH] log: decorate grafted commits with "grafted"
From: Nguyen Thai Ngoc Duy @ 2011-08-18 2:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8vqreuwk.fsf@alter.siamese.dyndns.org>
2011/8/18 Junio C Hamano <gitster@pobox.com>:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> @@ -638,6 +640,9 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
>> log.parent = NULL;
>> opt->loginfo = &log;
>>
>> + if (!commit->parents && lookup_commit_graft(commit->object.sha1))
>> + add_name_decoration(DECORATION_GRAFTED, "grafted",
>> + &commit->object);
>
> I am not very enthused about this change.
>
> We have already looked up the commit when we parsed it, and then we again
> have to call lookup_commit_graft() which would yield false for most of the
> commits?
which is why there's "!commit->parents" check. I made this patch with
shallow clone in mind. You probably have seen that if grafts are used
to extend history instead of cutting it, it won't show.
> Does this work with replacements and shallows, by the way?
Shallows, yes (that was my aim). Replacements, no. A better way would
be go over commit_graft[] and replace_object[] arrays, decorate all
grafted/replaced commits, instead of checking in log_tree_commit().
--
Duy
^ permalink raw reply
* Re: [PATCH] rev-parse: Allow @{U} as a synonym for @{u}
From: Nguyen Thai Ngoc Duy @ 2011-08-18 1:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Conrad Irwin, Sitaram Chamarty
In-Reply-To: <7vhb5fd4zy.fsf@alter.siamese.dyndns.org>
On Thu, Aug 18, 2011 at 5:53 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Letting u/upstream spelled case-insensitively does improve consistency
> among the above, but at the same time if we ever wanted to enhance @{...}
> notation even further in the future, we are restricted to a payload that
> is case insensitive to retain the consistency.
>
> The only remotely semi-plausible enhancement I could think of is perhaps
> to allow @{/regexp} to find a reflog entry that matches the given pattern,
> and in such a use case we would certainly want to take the pattern in a
> case sensitive way. This change closes the door to that, and that is the
> only downside I can think of right now.
>
> I hate to declare that we will never support such a feature with this
> change, but at the same time, I do not think I would need such a feature
> that often. But for that matter, I do not think I would miss @{UpStREAM},
> either, and that takes me back to "Meh" for this change.
>
> What do people think?
The itch is probably because people have to release shift, then press
shift again to type "@{u}". How about allow case insensitive match if
there's only _one_ letter inside @{}?
That would solve the shift problem while leaving door for @{...} extension.
--
Duy
^ permalink raw reply
* [PATCH 4/4] git-p4: Add test case for copy detection.
From: Vitor Antunes @ 2011-08-18 1:04 UTC (permalink / raw)
To: git; +Cc: Pete Wyckoff, Tor Arvid Lund, Vitor Antunes
In-Reply-To: <1313629459-22937-1-git-send-email-vitor.hda@gmail.com>
Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
---
t/t9800-git-p4.sh | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 71 insertions(+), 0 deletions(-)
diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index d01a1cb..a4f3d66 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -319,6 +319,77 @@ test_expect_success 'detect renames' '
rm -rf "$git" && mkdir "$git"
'
+# Copy a file and confirm that copy is not detected in P4.
+# Copy a file with detectCopies option enabled and confirm that copy is not
+# detected in P4.
+# Modify and copy a file with detectCopies option enabled and confirm that copy
+# is detected in P4.
+# Copy a file with detectCopies and detectCopiesHarder options enabled and
+# confirm that copy is detected in P4.
+# Modify and copy a file, configure a big threshold in detectCopies and confirm
+# that copy is not detected in P4.
+# Modify and copy a file, configure a small threshold in detectCopies and
+# confirm that copy is detected in P4.
+test_expect_success 'detect copies' '
+ git init "$git" &&
+ cd "$git" &&
+ cd "$TRASH_DIRECTORY" &&
+ "$GITP4" clone --dest="$git" //depot@all &&
+ cd "$git" &&
+ cp file2 file8 &&
+ git add file8 &&
+ git commit -a -m "Copy file2 to file8" &&
+ git diff-tree -r -C HEAD
+ git config git-p4.skipSubmitEditCheck true &&
+ "$GITP4" submit &&
+ p4 filelog //depot/file8 &&
+ ! p4 filelog //depot/file8 | grep -q "branch from //depot/file" &&
+ cp file2 file9 &&
+ git add file9 &&
+ git commit -a -m "Copy file2 to file9" &&
+ git diff-tree -r -C HEAD
+ git config git-p4.detectCopies true &&
+ "$GITP4" submit &&
+ p4 filelog //depot/file9 &&
+ ! p4 filelog //depot/file9 | grep -q "branch from //depot/file" &&
+ echo file2 >> file2 &&
+ cp file2 file10 &&
+ git add file2 file10 &&
+ git commit -a -m "Modify and copy file2 to file10" &&
+ git diff-tree -r -C HEAD
+ "$GITP4" submit &&
+ p4 filelog //depot/file10 &&
+ p4 filelog //depot/file10 | grep -q "branch from //depot/file" &&
+ cp file2 file11 &&
+ git add file11 &&
+ git commit -a -m "Copy file2 to file11" &&
+ git diff-tree -r -C --find-copies-harder HEAD
+ git config git-p4.detectCopiesHarder true &&
+ "$GITP4" submit &&
+ p4 filelog //depot/file11 &&
+ p4 filelog //depot/file11 | grep -q "branch from //depot/file" &&
+ cp file2 file12 &&
+ echo >> file12 &&
+ git add file12 &&
+ git commit -a -m "Copy file2 to file12 with changes" &&
+ git diff-tree -r -C --find-copies-harder HEAD
+ git config git-p4.detectCopies 98 &&
+ "$GITP4" submit &&
+ p4 filelog //depot/file12 &&
+ ! p4 filelog //depot/file12 | grep -q "branch from //depot/file" &&
+ cp file2 file13 &&
+ echo >> file13 &&
+ git add file13 &&
+ git commit -a -m "Copy file2 to file13 with changes" &&
+ git diff-tree -r -C --find-copies-harder HEAD
+ git config git-p4.detectCopies 80 &&
+ "$GITP4" submit &&
+ p4 filelog //depot/file13 &&
+ p4 filelog //depot/file13 | grep -q "branch from //depot/file" &&
+ cd "$TRASH_DIRECTORY" &&
+ rm -rf "$git" && mkdir "$git"
+'
+
test_expect_success 'shutdown' '
pid=`pgrep -f p4d` &&
test -n "$pid" &&
--
1.7.5.4
^ permalink raw reply related
* [PATCH 3/4] git-p4: Add test case for rename detection.
From: Vitor Antunes @ 2011-08-18 1:04 UTC (permalink / raw)
To: git; +Cc: Pete Wyckoff, Tor Arvid Lund, Vitor Antunes
In-Reply-To: <1313629459-22937-1-git-send-email-vitor.hda@gmail.com>
Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
---
t/t9800-git-p4.sh | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 50 insertions(+), 0 deletions(-)
diff --git a/t/t9800-git-p4.sh b/t/t9800-git-p4.sh
index 97ec975..d01a1cb 100755
--- a/t/t9800-git-p4.sh
+++ b/t/t9800-git-p4.sh
@@ -269,6 +269,56 @@ test_expect_success 'initial import time from top change time' '
test $p4time = $gittime
'
+# Rename a file and confirm that rename is not detected in P4.
+# Rename the new file again with detectRenames option enabled and confirm that
+# this is detected in P4.
+# Rename the new file again adding an extra blank line, configure a big
+# threshold in detectRenames and confirm that rename is not detected in P4.
+# Rename the new file again adding another extra blank line, configure a small
+# threshold in detectRenames and confirm that rename is not detected in P4.
+test_expect_success 'detect renames' '
+ git init "$git" &&
+ cd "$git" &&
+ cd "$TRASH_DIRECTORY" &&
+ "$GITP4" clone --dest="$git" //depot@all &&
+ p4 files //depot/* &&
+ cd "$git" &&
+ git mv file1 file4 &&
+ git commit -a -m "Rename file1 to file4" &&
+ git diff-tree -r -M HEAD &&
+ git config git-p4.skipSubmitEditCheck true &&
+ "$GITP4" submit &&
+ p4 filelog //depot/file4 &&
+ ! p4 filelog //depot/file4 | grep -q "branch from //depot/file1" &&
+ git mv file4 file5 &&
+ git commit -a -m "Rename file4 to file5" &&
+ git diff-tree -r -M HEAD &&
+ git config git-p4.detectRenames true &&
+ "$GITP4" submit &&
+ p4 filelog //depot/file5 &&
+ p4 filelog //depot/file5 | grep -q "branch from //depot/file4" &&
+ git mv file5 file6 &&
+ echo update >> file6 &&
+ git add file6 &&
+ git commit -a -m "Rename file5 to file6 with changes" &&
+ git diff-tree -r -M HEAD &&
+ git config git-p4.detectRenames 95 &&
+ "$GITP4" submit &&
+ p4 filelog //depot/file6 &&
+ ! p4 filelog //depot/file6 | grep -q "branch from //depot/file5" &&
+ git mv file6 file7 &&
+ echo update >> file7 &&
+ git add file7 &&
+ git commit -a -m "Rename file6 to file7 with changes" &&
+ git diff-tree -r -M HEAD &&
+ git config git-p4.detectRenames 80 &&
+ "$GITP4" submit &&
+ p4 filelog //depot/file7 &&
+ p4 filelog //depot/file7 | grep -q "branch from //depot/file6" &&
+ cd "$TRASH_DIRECTORY" &&
+ rm -rf "$git" && mkdir "$git"
+'
+
test_expect_success 'shutdown' '
pid=`pgrep -f p4d` &&
test -n "$pid" &&
--
1.7.5.4
^ permalink raw reply related
* [PATCH 2/4] git-p4: Add description of rename/copy detection options.
From: Vitor Antunes @ 2011-08-18 1:04 UTC (permalink / raw)
To: git; +Cc: Pete Wyckoff, Tor Arvid Lund, Vitor Antunes
In-Reply-To: <1313629459-22937-1-git-send-email-vitor.hda@gmail.com>
Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
---
contrib/fast-import/git-p4.txt | 25 +++++++++++++++++++++++++
1 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/contrib/fast-import/git-p4.txt b/contrib/fast-import/git-p4.txt
index caa4bb3..2ffbccc 100644
--- a/contrib/fast-import/git-p4.txt
+++ b/contrib/fast-import/git-p4.txt
@@ -232,6 +232,31 @@ git-p4.skipUserNameCheck
When submitting, git-p4 checks that the git commits are authored by the current
p4 user, and warns if they are not. This disables the check.
+git-p4.detectRenames
+
+Detect renames when submitting changes to Perforce server. Will enable -M git
+argument. Can be optionally set to a number representing the threshold
+percentage value of the rename detection.
+
+ git config [--global] git-p4.detectRenames true
+ git config [--global] git-p4.detectRenames 50
+
+git-p4.detectCopies
+
+Detect copies when submitting changes to Perforce server. Will enable -C git
+argument. Can be optionally set to a number representing the threshold
+percentage value of the copy detection.
+
+ git config [--global] git-p4.detectCopies true
+ git config [--global] git-p4.detectCopies 80
+
+git-p4.detectCopiesHarder
+
+Detect copies even between files that did not change when submitting changes to
+Perforce server. Will enable --find-copies-harder git argument.
+
+ git config [--global] git-p4.detectCopies true
+
Implementation Details...
=========================
--
1.7.5.4
^ permalink raw reply related
* [PATCH 1/4] git-p4: Allow setting rename/copy detection threshold.
From: Vitor Antunes @ 2011-08-18 1:04 UTC (permalink / raw)
To: git; +Cc: Pete Wyckoff, Tor Arvid Lund, Vitor Antunes
In-Reply-To: <1313629459-22937-1-git-send-email-vitor.hda@gmail.com>
Copy and rename detection arguments (-C and -M) allow setting a threshold value
for the similarity ratio. If the similarity is below this threshold the rename
or copy is ignored and the file is added as new.
This patch allows setting git-p4.detectRenames and git-p4.detectCopies options
to an integer value to set the respective threshold.
Signed-off-by: Vitor Antunes <vitor.hda@gmail.com>
---
contrib/fast-import/git-p4 | 18 +++++++++++++-----
1 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index 6b9de9e..cf719be 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -774,15 +774,23 @@ class P4Submit(Command, P4UserMap):
if not self.detectRenames:
# If not explicitly set check the config variable
- self.detectRenames = gitConfig("git-p4.detectRenames").lower() == "true"
+ self.detectRenames = gitConfig("git-p4.detectRenames")
- if self.detectRenames:
+ diffOpts = ""
+ if self.detectRenames.lower() == "true":
diffOpts = "-M"
- else:
- diffOpts = ""
+ elif self.detectRenames != "":
+ self.detectRenames = int(self.detectRenames)
+ if self.detectRenames >= 0 and self.detectRenames <= 100:
+ diffOpts = "-M%d" % self.detectRenames
- if gitConfig("git-p4.detectCopies").lower() == "true":
+ detectCopies = gitConfig("git-p4.detectCopies")
+ if detectCopies.lower() == "true":
diffOpts += " -C"
+ elif detectCopies != "":
+ detectCopies = int(detectCopies)
+ if detectCopies >= 0 and detectCopies <= 100:
+ diffOpts += " -C%d" % detectCopies
if gitConfig("git-p4.detectCopiesHarder").lower() == "true":
diffOpts += " --find-copies-harder"
--
1.7.5.4
^ permalink raw reply related
* [PATCH 0/4] Support threshold in copy/rename detection
From: Vitor Antunes @ 2011-08-18 1:04 UTC (permalink / raw)
To: git; +Cc: Pete Wyckoff, Tor Arvid Lund, Vitor Antunes
Add possibility of configuring a threshold in the copy and rename detections.
Copy and rename detection options were not documented and no test cases existed
for it. This is corrected in this set of patches.
Vitor Antunes (4):
git-p4: Allow setting rename/copy detection threshold.
git-p4: Add description of rename/copy detection options.
git-p4: Add test case for rename detection.
git-p4: Add test case for copy detection.
contrib/fast-import/git-p4 | 18 ++++--
contrib/fast-import/git-p4.txt | 25 ++++++++
t/t9800-git-p4.sh | 121 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 159 insertions(+), 5 deletions(-)
--
1.7.5.4
^ permalink raw reply
* Re: [PATCH] update-index: add --swap to swap index and worktree content
From: Martin von Zweigbergk @ 2011-08-18 1:01 UTC (permalink / raw)
To: Jeff King; +Cc: Nguyen Thai Ngoc Duy, Junio C Hamano, git
In-Reply-To: <20110817194621.GA352@sigill.intra.peff.net>
On Wed, Aug 17, 2011 at 3:46 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Aug 17, 2011 at 10:13:08AM -0400, Martin von Zweigbergk wrote:
>
>> >> Two reasons. I already mentioned the ability to quickly checkout index
>> >> for a quick test (though there could be more problems down that road).
>> >
>> > That's a good thought. However, in practice, I find I also need other
>> > files from the index to do a successful test. So I end up just
>> > committing what I think is right, and then afterwards do:
>>
>> Maybe a stupid question, but "update-index --swap" would swap all
>> files, right? So what "other files from the index" would there be?
>
> Hmm. I hadn't really considered swapping everything.
Ah, then I see where the misunderstanding lay. I didn't read the
patch; maybe it would have been clear if I did (and understood it).
> I suppose that
> would work, though I do worry about getting into a confused state when
> you have swapped one or more files individually, and then want to swap
> the rest out for testing.
I have a feeling that I would get confused if I started swapping more
than one individual file, so to me it feels like it's more the
possibility of swapping individual files that could cause confusion,
whether or not it is possible to swap all.
I think stashing the work tree feels more right, though. All you
really want is to get the work tree version out of the way for a
while, you don't really want it in the index.
Martin
^ permalink raw reply
* Re: Malformed branch name in fast-export when specifying non-HEAD/branch revision
From: Jeff King @ 2011-08-17 23:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Elijah Newren, Owen Stephens, git, Sverre Rabbelier
In-Reply-To: <7vliurd62x.fsf@alter.siamese.dyndns.org>
On Wed, Aug 17, 2011 at 03:30:14PM -0700, Junio C Hamano wrote:
> You can think of "fast-export" an off-line "push" command [*1*]; instead
> of giving a random commit object, e.g. "git fast-export HEAD~1", that can
> not be used as a ref, you can use the refspec notation to tell where the
> result should go, e.g. "git fast-export HEAD~1:refs/heads/a-bit-older",
> from the command line of fast-export.
>
> I suspect that also may clarify what Sverre was trying to do in his recent
> series. The root cause of both this and the issue Sverre wanted to fix is
> the design mistake of fast-export that tries to reuse the notation of
> object range specification for a different purpose of telling which "ref"
> to update, I think.
Yes, this was the conclusion I came to when I looked at this a month or
so ago. You really need to give fast-export a mapping of objects to
refnames, and it should output ref names _only_ for the mapping. That
would handle this "not a ref" case, but would also let you push
"refs/heads/foo" when it is equivalent to "refs/heads/master", without
fast-export mentioning "refs/heads/master" at all.
-Peff
^ permalink raw reply
* Re: [PATCH] rev-parse: Allow @{U} as a synonym for @{u}
From: Junio C Hamano @ 2011-08-17 22:53 UTC (permalink / raw)
To: git; +Cc: Conrad Irwin, Sitaram Chamarty
In-Reply-To: <1313287071-7851-1-git-send-email-conrad.irwin@gmail.com>
Conrad Irwin <conrad.irwin@gmail.com> writes:
> This facilitates specifying the upstream of a branch if you have
> difficulty synchronising your shift-key and other characters, if you
> just want to SHOUT at git, or if you're expecting consistency with
> @{1DAY}.
> +test_expect_success '@{U} resolves to correct full name' '
> + test refs/remotes/origin/master = "$(full_name @{U})"
> +'
> +
> +test_expect_success '@{Upstream} resolves to correct full name' '
> + test refs/remotes/origin/master = "$(full_name @{Upstream})"
> +'
> +
> test_expect_success 'my-side@{upstream} resolves to correct full name' '
> test refs/remotes/origin/side = "$(full_name my-side@{u})"
> '
Even though I sometimes use @{upstream} these days to keep track of
origins of various topic branches (some are fixes and fork from maint
while others are features and fork from master), I never felt a need for
@{U} or @{uPStreAM} myself, so as a new feature, this change is a "Meh"
for me personally.
Currently, we use @{...} for:
- Negative integers are "-N branch-switching ago" (only without any ref
on the left);
- Non-negative integers "The tip of the named ref before it was changed N
times";
- An approxidate that is case insensitive; or
- "u" and "upstream".
Letting u/upstream spelled case-insensitively does improve consistency
among the above, but at the same time if we ever wanted to enhance @{...}
notation even further in the future, we are restricted to a payload that
is case insensitive to retain the consistency.
The only remotely semi-plausible enhancement I could think of is perhaps
to allow @{/regexp} to find a reflog entry that matches the given pattern,
and in such a use case we would certainly want to take the pattern in a
case sensitive way. This change closes the door to that, and that is the
only downside I can think of right now.
I hate to declare that we will never support such a feature with this
change, but at the same time, I do not think I would need such a feature
that often. But for that matter, I do not think I would miss @{UpStREAM},
either, and that takes me back to "Meh" for this change.
What do people think?
^ permalink raw reply
* Re: Malformed branch name in fast-export when specifying non-HEAD/branch revision
From: Junio C Hamano @ 2011-08-17 22:30 UTC (permalink / raw)
To: Elijah Newren; +Cc: Owen Stephens, git, Sverre Rabbelier
In-Reply-To: <CABPp-BFRZMZjhWuUUeD7Oa1HbWQMnZot7dRm3zKOpCoj_QwZeg@mail.gmail.com>
Elijah Newren <newren@gmail.com> writes:
>> $ # uses HEAD~1 instead of refs/heads/master
>> $ git fast-export HEAD~1
>>
>> blob
>> mark :1
>> data 0
>>
>> reset HEAD~1
>> commit HEAD~1
>
> Thanks for the report. It turns out this bug has been reported and is
> in the testsuite as t9350.19 -- currently marked as expected to fail.
> I looked at the problem a couple years ago for a little bit but never
> finished that particular patch and never got back around to it.
What _should_ be the right behaviour to begin with, I have to wonder.
Even though it is very clear that the set of objects that are exported are
defined by the "rev-list arguments" given to the command, I do not think
fast-export's semantics is not clearly defined as to what "refs" are to be
updated.
The easiest fix for this issue would be to forbid "git fast-export HEAD~1"
(or any range whose positive endpoints are _not_ refs), and I think that
would be in line with the original motivation of the command to export the
whole repository in a format fast-import would understand. The original
f2dc849 (Add 'git fast-export', the sister of 'git fast-import',
2007-12-02) says "This program dumps (parts of) a git repository...",
implying that partial export is within the scope of the command, but I do
not think it was designed carefully enough to deal with ranges more
complex than just "a set of branches".
I however have a feeling that people would want to say:
- I want to export up to that commit, and have that commit on this branch
on the importing side; or even better
- I want to export up to that commit, but what refs points at the commits
contained in the output stream will be decided when the output is
imported.
I do not think the latter meshes well with how "fast-import" works,
though. But fast-export should be fixable to allow the former without
breaking the semantics of fast-import.
You can think of "fast-export" an off-line "push" command [*1*]; instead
of giving a random commit object, e.g. "git fast-export HEAD~1", that can
not be used as a ref, you can use the refspec notation to tell where the
result should go, e.g. "git fast-export HEAD~1:refs/heads/a-bit-older",
from the command line of fast-export.
I suspect that also may clarify what Sverre was trying to do in his recent
series. The root cause of both this and the issue Sverre wanted to fix is
the design mistake of fast-export that tries to reuse the notation of
object range specification for a different purpose of telling which "ref"
to update, I think.
[Footnote]
*1* In a similar sense, unpacking "git bundle" output is an off-line
"fetch"; the bundle creator gave anchor points for tip objects, and allows
the unpacker to map them into its own namespace.
^ permalink raw reply
* [PATCH 2/3] revision.c: add show_object_with_name() helper function
From: Junio C Hamano @ 2011-08-17 21:30 UTC (permalink / raw)
To: git
In-Reply-To: <1313616635-25331-1-git-send-email-gitster@pobox.com>
There are two copies of traverse_commit_list callback that show the object
name followed by pathname the object was found, to produce output similar
to "rev-list --objects".
Unify them.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/rev-list.c | 15 +--------------
revision.c | 19 +++++++++++++++++++
revision.h | 2 ++
upload-pack.c | 15 +--------------
4 files changed, 23 insertions(+), 28 deletions(-)
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index d789279..f5ce487 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -176,21 +176,8 @@ static void finish_object(struct object *obj, const struct name_path *path, cons
static void show_object(struct object *obj, const struct name_path *path, const char *component)
{
- char *name = path_name(path, component);
- /* An object with name "foo\n0000000..." can be used to
- * confuse downstream "git pack-objects" very badly.
- */
- const char *ep = strchr(name, '\n');
-
finish_object(obj, path, component);
- if (ep) {
- printf("%s %.*s\n", sha1_to_hex(obj->sha1),
- (int) (ep - name),
- name);
- }
- else
- printf("%s %s\n", sha1_to_hex(obj->sha1), name);
- free(name);
+ show_object_with_name(stdout, obj, path, component);
}
static void show_edge(struct commit *commit)
diff --git a/revision.c b/revision.c
index c46cfaa..c5b38cc 100644
--- a/revision.c
+++ b/revision.c
@@ -40,6 +40,25 @@ char *path_name(const struct name_path *path, const char *name)
return n;
}
+void show_object_with_name(FILE *out, struct object *obj, const struct name_path *path, const char *component)
+{
+ char *name = path_name(path, component);
+ const char *ep = strchr(name, '\n');
+
+ /*
+ * An object with name "foo\n0000000..." can be used to
+ * confuse downstream "git pack-objects" very badly.
+ */
+ if (ep) {
+ fprintf(out, "%s %.*s\n", sha1_to_hex(obj->sha1),
+ (int) (ep - name),
+ name);
+ }
+ else
+ fprintf(out, "%s %s\n", sha1_to_hex(obj->sha1), name);
+ free(name);
+}
+
void add_object(struct object *obj,
struct object_array *p,
struct name_path *path,
diff --git a/revision.h b/revision.h
index 3d64ada..da00a58 100644
--- a/revision.h
+++ b/revision.h
@@ -185,6 +185,8 @@ struct name_path {
char *path_name(const struct name_path *path, const char *name);
+extern void show_object_with_name(FILE *, struct object *, const struct name_path *, const char *);
+
extern void add_object(struct object *obj,
struct object_array *p,
struct name_path *path,
diff --git a/upload-pack.c b/upload-pack.c
index ce5cbbe..970a1eb 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -85,20 +85,7 @@ static void show_commit(struct commit *commit, void *data)
static void show_object(struct object *obj, const struct name_path *path, const char *component)
{
- /* An object with name "foo\n0000000..." can be used to
- * confuse downstream git-pack-objects very badly.
- */
- const char *name = path_name(path, component);
- const char *ep = strchr(name, '\n');
- if (ep) {
- fprintf(pack_pipe, "%s %.*s\n", sha1_to_hex(obj->sha1),
- (int) (ep - name),
- name);
- }
- else
- fprintf(pack_pipe, "%s %s\n",
- sha1_to_hex(obj->sha1), name);
- free((char *)name);
+ show_object_with_name(pack_pipe, obj, path, component);
}
static void show_edge(struct commit *commit)
--
1.7.6.472.g4bfe7c
^ permalink raw reply related
* [PATCH 3/3] revision.c: update show_object_with_name() without using malloc()
From: Junio C Hamano @ 2011-08-17 21:30 UTC (permalink / raw)
To: git
In-Reply-To: <1313616635-25331-1-git-send-email-gitster@pobox.com>
Allocating and then immediately freeing temporary memory a million times
when listing a million objects is distasteful.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* It may also be an option to keep a single static strbuf in
show_object_with_name() and reuse it.
revision.c | 50 ++++++++++++++++++++++++++++++++++++--------------
1 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/revision.c b/revision.c
index c5b38cc..072ddac 100644
--- a/revision.c
+++ b/revision.c
@@ -40,23 +40,45 @@ char *path_name(const struct name_path *path, const char *name)
return n;
}
+static int show_path_component_truncated(FILE *out, const char *name, int len)
+{
+ int cnt;
+ for (cnt = 0; cnt < len; cnt++) {
+ int ch = name[cnt];
+ if (!ch || ch == '\n')
+ return -1;
+ fputc(ch, out);
+ }
+ return len;
+}
+
+static int show_path_truncated(FILE *out, const struct name_path *path)
+{
+ int emitted, ours;
+
+ if (!path)
+ return 0;
+ emitted = show_path_truncated(out, path->up);
+ if (emitted < 0)
+ return emitted;
+ if (emitted)
+ fputc('/', out);
+ ours = show_path_component_truncated(out, path->elem, path->elem_len);
+ if (ours < 0)
+ return ours;
+ return ours || emitted;
+}
+
void show_object_with_name(FILE *out, struct object *obj, const struct name_path *path, const char *component)
{
- char *name = path_name(path, component);
- const char *ep = strchr(name, '\n');
+ struct name_path leaf;
+ leaf.up = (struct name_path *)path;
+ leaf.elem = component;
+ leaf.elem_len = strlen(component);
- /*
- * An object with name "foo\n0000000..." can be used to
- * confuse downstream "git pack-objects" very badly.
- */
- if (ep) {
- fprintf(out, "%s %.*s\n", sha1_to_hex(obj->sha1),
- (int) (ep - name),
- name);
- }
- else
- fprintf(out, "%s %s\n", sha1_to_hex(obj->sha1), name);
- free(name);
+ fprintf(out, "%s ", sha1_to_hex(obj->sha1));
+ show_path_truncated(out, &leaf);
+ fputc('\n', out);
}
void add_object(struct object *obj,
--
1.7.6.472.g4bfe7c
^ permalink raw reply related
* [PATCH 1/3] rev-list: fix finish_object() call
From: Junio C Hamano @ 2011-08-17 21:30 UTC (permalink / raw)
To: git
In-Reply-To: <1313616635-25331-1-git-send-email-gitster@pobox.com>
The callback to traverse_commit_list() are to take linked name_path and
a string for the last path component.
If the callee used its parameters, it would have seen duplicated leading
paths. In this particular case, the callee does not use this argument but
that is not a reason to leave the call broken.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/rev-list.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 56727e8..d789279 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -182,7 +182,7 @@ static void show_object(struct object *obj, const struct name_path *path, const
*/
const char *ep = strchr(name, '\n');
- finish_object(obj, path, name);
+ finish_object(obj, path, component);
if (ep) {
printf("%s %.*s\n", sha1_to_hex(obj->sha1),
(int) (ep - name),
--
1.7.6.472.g4bfe7c
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox