[PATCH 20/83] builtin/apply: move 'threeway' global into 'struct apply_state'
Signed-off-by: Christian Couder
---
builtin/apply.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index e488879..33a1f8f 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -41,6 +41,8 @@ struct apply_state {
int summary;
+ int threeway;
+
/*
* --check turns on checking that the working tree matches the
*files that are being modified, but doesn't apply the patch
@@ -64,7 +66,6 @@ static int state_p_value = 1;
static int p_value_known;
static int apply = 1;
static int no_add;
-static int threeway;
static int unsafe_paths;
static const char *fake_ancestor;
static int line_termination = '\n';
@@ -3504,7 +3505,7 @@ static int apply_data(struct apply_state *state, struct
patch *patch,
if (patch->direct_to_threeway ||
apply_fragments(state, &image, patch) < 0) {
/* Note: with --reject, apply_fragments() returns 0 */
- if (!threeway || try_threeway(state, &image, patch, st, ce) < 0)
+ if (!state->threeway || try_threeway(state, &image, patch, st,
ce) < 0)
return -1;
}
patch->result = image.buf;
@@ -3799,7 +3800,7 @@ static int check_patch(struct apply_state *state, struct
patch *patch)
((0 < patch->is_new) || patch->is_rename || patch->is_copy)) {
int err = check_to_create(state, new_name, ok_if_exists);
- if (err && threeway) {
+ if (err && state->threeway) {
patch->direct_to_threeway = 1;
} else switch (err) {
case 0:
@@ -4614,7 +4615,7 @@ int cmd_apply(int argc, const char **argv, const char
*prefix_)
N_("accept a patch that touches outside the working
area")),
OPT_BOOL(0, "apply", &force_apply,
N_("also apply the patch (use with
--stat/--summary/--check)")),
- OPT_BOOL('3', "3way", &threeway,
+ OPT_BOOL('3', "3way", &state.threeway,
N_( "attempt three-way merge if a patch does not
apply")),
OPT_FILENAME(0, "build-fake-ancestor", &fake_ancestor,
N_("build a temporary index based on embedded index
information")),
@@ -4666,11 +4667,11 @@ int cmd_apply(int argc, const char **argv, const char
*prefix_)
argc = parse_options(argc, argv, state.prefix, builtin_apply_options,
apply_usage, 0);
- if (state.apply_with_reject && threeway)
+ if (state.apply_with_reject && state.threeway)
die("--reject and --3way cannot be used together.");
- if (state.cached && threeway)
+ if (state.cached && state.threeway)
die("--cached and --3way cannot be used together.");
- if (threeway) {
+ if (state.threeway) {
if (is_not_gitdir)
die(_("--3way outside a repository"));
state.check_index = 1;
--
2.8.1.300.g5fed0c0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/83] builtin/apply: move 'check' global into 'struct apply_state'
Signed-off-by: Christian Couder
---
builtin/apply.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index ad81210..6c628f6 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -25,12 +25,15 @@ struct apply_state {
const char *prefix;
int prefix_length;
+ /*
+* --check turns on checking that the working tree matches the
+*files that are being modified, but doesn't apply the patch
+*/
+ int check;
int unidiff_zero;
};
/*
- * --check turns on checking that the working tree matches the
- *files that are being modified, but doesn't apply the patch
* --stat does just a diffstat, and doesn't actually apply
* --numstat does numeric diffstat, and doesn't actually apply
* --index-info shows the old and new index info for paths if available.
@@ -47,7 +50,6 @@ static int cached;
static int diffstat;
static int numstat;
static int summary;
-static int check;
static int apply = 1;
static int apply_in_reverse;
static int apply_with_reject;
@@ -2053,7 +2055,7 @@ static int parse_chunk(struct apply_state *state, char
*buffer, unsigned long si
* without metadata change. A binary patch appears
* empty to us here.
*/
- if ((apply || check) &&
+ if ((apply || state->check) &&
(!patch->is_binary && !metadata_changes(patch)))
die(_("patch with only garbage at line %d"), linenr);
}
@@ -4441,7 +4443,7 @@ static int apply_patch(struct apply_state *state,
die(_("unable to read index file"));
}
- if ((check || apply) &&
+ if ((state->check || apply) &&
check_patch_list(state, list) < 0 &&
!apply_with_reject)
exit(1);
@@ -4561,7 +4563,7 @@ int cmd_apply(int argc, const char **argv, const char
*prefix_)
N_("show number of added and deleted lines in decimal
notation")),
OPT_BOOL(0, "summary", &summary,
N_("instead of applying the patch, output a summary for
the input")),
- OPT_BOOL(0, "check", &check,
+ OPT_BOOL(0, "check", &state.check,
N_("instead of applying the patch, see if the patch is
applicable")),
OPT_BOOL(0, "index", &check_index,
N_("make sure the patch is applicable to the current
index")),
@@ -4634,7 +4636,7 @@ int cmd_apply(int argc, const char **argv, const char
*prefix_)
}
if (apply_with_reject)
apply = apply_verbosely = 1;
- if (!force_apply && (diffstat || numstat || summary || check ||
fake_ancestor))
+ if (!force_apply && (diffstat || numstat || summary || state.check ||
fake_ancestor))
apply = 0;
if (check_index && is_not_gitdir)
die(_("--index outside a repository"));
--
2.8.1.300.g5fed0c0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 79/83] apply: make some parsing functions static again
Signed-off-by: Christian Couder
---
apply.c | 6 +++---
apply.h | 5 -
2 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/apply.c b/apply.c
index 99b7a2d..86e0d20 100644
--- a/apply.c
+++ b/apply.c
@@ -27,7 +27,7 @@ static void git_apply_config(void)
git_config(git_default_config, NULL);
}
-int parse_whitespace_option(struct apply_state *state, const char *option)
+static int parse_whitespace_option(struct apply_state *state, const char
*option)
{
if (!option) {
state->ws_error_action = warn_on_ws_error;
@@ -57,8 +57,8 @@ int parse_whitespace_option(struct apply_state *state, const
char *option)
return error(_("unrecognized whitespace option '%s'"), option);
}
-int parse_ignorewhitespace_option(struct apply_state *state,
- const char *option)
+static int parse_ignorewhitespace_option(struct apply_state *state,
+const char *option)
{
if (!option || !strcmp(option, "no") ||
!strcmp(option, "false") || !strcmp(option, "never") ||
diff --git a/apply.h b/apply.h
index 6803259..ba138d4 100644
--- a/apply.h
+++ b/apply.h
@@ -121,11 +121,6 @@ struct apply_state {
enum ws_ignore ws_ignore_action;
};
-extern int parse_whitespace_option(struct apply_state *state,
- const char *option);
-extern int parse_ignorewhitespace_option(struct apply_state *state,
-const char *option);
-
extern int apply_option_parse_exclude(const struct option *opt,
const char *arg, int unset);
extern int apply_option_parse_include(const struct option *opt,
--
2.8.1.300.g5fed0c0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 81/83] apply: roll back index in case of error
Signed-off-by: Christian Couder
---
apply.c | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/apply.c b/apply.c
index 86e0d20..7cee834 100644
--- a/apply.c
+++ b/apply.c
@@ -4718,8 +4718,11 @@ int apply_all_patches(struct apply_state *state,
if (!strcmp(arg, "-")) {
res = apply_patch(state, 0, "", options);
- if (res < 0)
+ if (res < 0) {
+ if (state->lock_file)
+ rollback_lock_file(state->lock_file);
return -1;
+ }
errs |= res;
read_stdin = 0;
continue;
@@ -4734,16 +4737,22 @@ int apply_all_patches(struct apply_state *state,
read_stdin = 0;
set_default_whitespace_mode(state);
res = apply_patch(state, fd, arg, options);
- if (res < 0)
+ if (res < 0) {
+ if (state->lock_file)
+ rollback_lock_file(state->lock_file);
return -1;
+ }
errs |= res;
close(fd);
}
set_default_whitespace_mode(state);
if (read_stdin) {
res = apply_patch(state, 0, "", options);
- if (res < 0)
+ if (res < 0) {
+ if (state->lock_file)
+ rollback_lock_file(state->lock_file);
return -1;
+ }
errs |= res;
}
@@ -4757,11 +4766,14 @@ int apply_all_patches(struct apply_state *state,
squelched),
squelched);
}
- if (state->ws_error_action == die_on_ws_error)
+ if (state->ws_error_action == die_on_ws_error) {
+ if (state->lock_file)
+ rollback_lock_file(state->lock_file);
return error(Q_("%d line adds whitespace errors.",
"%d lines add whitespace errors.",
state->whitespace_error),
state->whitespace_error);
+ }
if (state->applied_after_fixing_ws && state->apply)
warning("%d line%s applied after"
" fixing whitespace errors.",
--
2.8.1.300.g5fed0c0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 30/83] builtin/apply: move 'has_include' global into 'struct apply_state'
Signed-off-by: Christian Couder
---
builtin/apply.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index c8b9bf0..0717cd2 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -70,6 +70,7 @@ struct apply_state {
const char *patch_input_file;
struct string_list limit_by_name;
+ int has_include;
};
static int newfd = -1;
@@ -1976,7 +1977,6 @@ static void prefix_patch(struct apply_state *state,
struct patch *p)
* include/exclude
*/
-static int has_include;
static void add_name_limit(struct apply_state *state,
const char *name,
int exclude)
@@ -2012,7 +2012,7 @@ static int use_patch(struct apply_state *state, struct
patch *p)
* not used. Otherwise, we saw bunch of exclude rules (or none)
* and such a path is used.
*/
- return !has_include;
+ return !state->has_include;
}
@@ -4550,7 +4550,7 @@ static int option_parse_include(const struct option *opt,
{
struct apply_state *state = opt->value;
add_name_limit(state, arg, 0);
- has_include = 1;
+ state->has_include = 1;
return 0;
}
--
2.8.1.300.g5fed0c0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/83] builtin/apply: introduce 'struct apply_state' to start libifying
Currently commands that want to use the apply functionality have to launch
a "git apply" process which can be bad for performance.
Let's start libifying the apply functionality and to do that we first need
to get rid of the global variables in "builtin/apply.c".
This patch introduces "struct apply_state" into which all the previously
global variables will be moved. A new parameter called "state" that is a
pointer to the "apply_state" structure will come at the beginning of the
helper functions that need it and will be passed around the call chain.
To start let's move the "prefix" and "prefix_length" global variables into
"struct apply_state".
Signed-off-by: Christian Couder
---
builtin/apply.c | 94 ++---
1 file changed, 56 insertions(+), 38 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 8fd8dbc..51e6af4 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -21,6 +21,11 @@
#include "ll-merge.h"
#include "rerere.h"
+struct apply_state {
+ const char *prefix;
+ int prefix_length;
+};
+
/*
* --check turns on checking that the working tree matches the
*files that are being modified, but doesn't apply the patch
@@ -30,8 +35,6 @@
* --index updates the cache as well.
* --cached updates only the cache without ever touching the working tree.
*/
-static const char *prefix;
-static int prefix_length = -1;
static int newfd = -1;
static int unidiff_zero;
@@ -749,7 +752,7 @@ static int count_slashes(const char *cp)
* Given the string after "--- " or "+++ ", guess the appropriate
* p_value for the given patch.
*/
-static int guess_p_value(const char *nameline)
+static int guess_p_value(struct apply_state *state, const char *nameline)
{
char *name, *cp;
int val = -1;
@@ -762,17 +765,17 @@ static int guess_p_value(const char *nameline)
cp = strchr(name, '/');
if (!cp)
val = 0;
- else if (prefix) {
+ else if (state->prefix) {
/*
* Does it begin with "a/$our-prefix" and such? Then this is
* very likely to apply to our directory.
*/
- if (!strncmp(name, prefix, prefix_length))
- val = count_slashes(prefix);
+ if (!strncmp(name, state->prefix, state->prefix_length))
+ val = count_slashes(state->prefix);
else {
cp++;
- if (!strncmp(cp, prefix, prefix_length))
- val = count_slashes(prefix) + 1;
+ if (!strncmp(cp, state->prefix, state->prefix_length))
+ val = count_slashes(state->prefix) + 1;
}
}
free(name);
@@ -859,7 +862,10 @@ static int has_epoch_timestamp(const char *nameline)
* files, we can happily check the index for a match, but for creating a
* new file we should try to match whatever "patch" does. I have no idea.
*/
-static void parse_traditional_patch(const char *first, const char *second,
struct patch *patch)
+static void parse_traditional_patch(struct apply_state *state,
+ const char *first,
+ const char *second,
+ struct patch *patch)
{
char *name;
@@ -867,8 +873,8 @@ static void parse_traditional_patch(const char *first,
const char *second, struc
second += 4;/* skip "+++ " */
if (!p_value_known) {
int p, q;
- p = guess_p_value(first);
- q = guess_p_value(second);
+ p = guess_p_value(state, first);
+ q = guess_p_value(state, second);
if (p < 0) p = q;
if (0 <= p && p == q) {
state_p_value = p;
@@ -1430,7 +1436,11 @@ static int parse_fragment_header(const char *line, int
len, struct fragment *fra
return offset;
}
-static int find_header(const char *line, unsigned long size, int *hdrsize,
struct patch *patch)
+static int find_header(struct apply_state *state,
+ const char *line,
+ unsigned long size,
+ int *hdrsize,
+ struct patch *patch)
{
unsigned long offset, len;
@@ -1507,7 +1517,7 @@ static int find_header(const char *line, unsigned long
size, int *hdrsize, struc
continue;
/* Ok, we'll consider it a patch */
- parse_traditional_patch(line, line+len, patch);
+ parse_traditional_patch(state, line, line+len, patch);
*hdrsize = len + nextlen;
[PATCH 22/83] builtin/apply: move 'unsafe_paths' global into 'struct apply_state'
Signed-off-by: Christian Couder
---
builtin/apply.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 506357c..c45e481 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -57,6 +57,8 @@ struct apply_state {
int unidiff_zero;
int update_index;
+
+ int unsafe_paths;
};
/*
@@ -67,7 +69,6 @@ static int newfd = -1;
static int state_p_value = 1;
static int p_value_known;
static int apply = 1;
-static int unsafe_paths;
static const char *fake_ancestor;
static int line_termination = '\n';
static unsigned int p_context = UINT_MAX;
@@ -3842,7 +3843,7 @@ static int check_patch(struct apply_state *state, struct
patch *patch)
}
}
- if (!unsafe_paths)
+ if (!state->unsafe_paths)
die_on_unsafe_path(patch);
/*
@@ -4612,7 +4613,7 @@ int cmd_apply(int argc, const char **argv, const char
*prefix_)
N_("make sure the patch is applicable to the current
index")),
OPT_BOOL(0, "cached", &state.cached,
N_("apply a patch without touching the working tree")),
- OPT_BOOL(0, "unsafe-paths", &unsafe_paths,
+ OPT_BOOL(0, "unsafe-paths", &state.unsafe_paths,
N_("accept a patch that touches outside the working
area")),
OPT_BOOL(0, "apply", &force_apply,
N_("also apply the patch (use with
--stat/--summary/--check)")),
@@ -4689,7 +4690,7 @@ int cmd_apply(int argc, const char **argv, const char
*prefix_)
state.check_index = 1;
}
if (state.check_index)
- unsafe_paths = 0;
+ state.unsafe_paths = 0;
for (i = 0; i < argc; i++) {
const char *arg = argv[i];
--
2.8.1.300.g5fed0c0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 82/83] environment: add set_index_file()
Signed-off-by: Christian Couder
---
cache.h | 1 +
environment.c | 5 +
2 files changed, 6 insertions(+)
diff --git a/cache.h b/cache.h
index 2711048..7f36aa3 100644
--- a/cache.h
+++ b/cache.h
@@ -461,6 +461,7 @@ extern int is_inside_work_tree(void);
extern const char *get_git_dir(void);
extern const char *get_git_common_dir(void);
extern char *get_object_directory(void);
+extern void set_index_file(char *index_file);
extern char *get_index_file(void);
extern char *get_graft_file(void);
extern int set_git_dir(const char *path);
diff --git a/environment.c b/environment.c
index 57acb2f..5a57822 100644
--- a/environment.c
+++ b/environment.c
@@ -290,6 +290,11 @@ int odb_pack_keep(char *name, size_t namesz, const
unsigned char *sha1)
return open(name, O_RDWR|O_CREAT|O_EXCL, 0600);
}
+void set_index_file(char *index_file)
+{
+ git_index_file = index_file;
+}
+
char *get_index_file(void)
{
if (!git_index_file)
--
2.8.1.300.g5fed0c0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 80/83] run-command: make dup_devnull() non static
Signed-off-by: Christian Couder
---
run-command.c | 2 +-
run-command.h | 6 ++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/run-command.c b/run-command.c
index 8c7115a..29d2bda 100644
--- a/run-command.c
+++ b/run-command.c
@@ -85,7 +85,7 @@ static inline void close_pair(int fd[2])
}
#ifndef GIT_WINDOWS_NATIVE
-static inline void dup_devnull(int to)
+void dup_devnull(int to)
{
int fd = open("/dev/null", O_RDWR);
if (fd < 0)
diff --git a/run-command.h b/run-command.h
index de1727e..3aa244a 100644
--- a/run-command.h
+++ b/run-command.h
@@ -200,4 +200,10 @@ int run_processes_parallel(int n,
task_finished_fn,
void *pp_cb);
+/**
+ * Misc helper functions
+ */
+
+void dup_devnull(int to);
+
#endif
--
2.8.1.300.g5fed0c0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 26/83] builtin/apply: move 'apply' global into 'struct apply_state'
Signed-off-by: Christian Couder
---
builtin/apply.c | 31 +--
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index b6d2343..699cabf 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -25,6 +25,7 @@ struct apply_state {
const char *prefix;
int prefix_length;
+ int apply;
int allow_overlap;
int apply_in_reverse;
int apply_with_reject;
@@ -71,7 +72,7 @@ static int newfd = -1;
static int state_p_value = 1;
static int p_value_known;
-static int apply = 1;
+
static const char * const apply_usage[] = {
N_("git apply [] [...]"),
NULL
@@ -142,10 +143,11 @@ static void parse_ignorewhitespace_option(const char
*option)
die(_("unrecognized whitespace ignore option '%s'"), option);
}
-static void set_default_whitespace_mode(const char *whitespace_option)
+static void set_default_whitespace_mode(struct apply_state *state,
+ const char *whitespace_option)
{
if (!whitespace_option && !apply_default_whitespace)
- ws_error_action = (apply ? warn_on_ws_error : nowarn_ws_error);
+ ws_error_action = (state->apply ? warn_on_ws_error :
nowarn_ws_error);
}
/*
@@ -2074,7 +2076,7 @@ static int parse_chunk(struct apply_state *state, char
*buffer, unsigned long si
* without metadata change. A binary patch appears
* empty to us here.
*/
- if ((apply || state->check) &&
+ if ((state->apply || state->check) &&
(!patch->is_binary && !metadata_changes(patch)))
die(_("patch with only garbage at line %d"), linenr);
}
@@ -2933,7 +2935,7 @@ static int apply_one_fragment(struct apply_state *state,
* apply_data->apply_fragments->apply_one_fragment
*/
if (ws_error_action == die_on_ws_error)
- apply = 0;
+ state->apply = 0;
}
if (state->apply_verbosely && applied_pos != pos) {
@@ -4477,9 +4479,9 @@ static int apply_patch(struct apply_state *state,
die(_("unrecognized input"));
if (whitespace_error && (ws_error_action == die_on_ws_error))
- apply = 0;
+ state->apply = 0;
- state->update_index = state->check_index && apply;
+ state->update_index = state->check_index && state->apply;
if (state->update_index && newfd < 0)
newfd = hold_locked_index(&lock_file, 1);
@@ -4488,12 +4490,12 @@ static int apply_patch(struct apply_state *state,
die(_("unable to read index file"));
}
- if ((state->check || apply) &&
+ if ((state->check || state->apply) &&
check_patch_list(state, list) < 0 &&
!state->apply_with_reject)
exit(1);
- if (apply && write_out_results(state, list)) {
+ if (state->apply && write_out_results(state, list)) {
if (state->apply_with_reject)
exit(1);
/* with --3way, we still need to write the index out */
@@ -4660,6 +4662,7 @@ int cmd_apply(int argc, const char **argv, const char
*prefix_)
memset(&state, 0, sizeof(state));
state.prefix = prefix_;
state.prefix_length = state.prefix ? strlen(state.prefix) : 0;
+ state.apply = 1;
state.line_termination = '\n';
state.p_context = UINT_MAX;
@@ -4682,9 +4685,9 @@ int cmd_apply(int argc, const char **argv, const char
*prefix_)
state.check_index = 1;
}
if (state.apply_with_reject)
- apply = state.apply_verbosely = 1;
+ state.apply = state.apply_verbosely = 1;
if (!force_apply && (state.diffstat || state.numstat || state.summary
|| state.check || state.fake_ancestor))
- apply = 0;
+ state.apply = 0;
if (state.check_index && is_not_gitdir)
die(_("--index outside a repository"));
if (state.cached) {
@@ -4712,11 +4715,11 @@ int cmd_apply(int argc, const char **argv, const char
*prefix_)
if (fd < 0)
die_errno(_("can't open patch '%s'"), arg);
read_stdin = 0;
- set_default_whitespace_mode(whitespace_option);
+ set_default_whitespace_mode(&state, whitespace_option);
errs |= apply_patch(&state, fd, arg, options);
clos
[PATCH 15/83] builtin/apply: move 'allow_overlap' global into 'struct apply_state'
Signed-off-by: Christian Couder
---
builtin/apply.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index b57be2c..a5dff99 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -25,6 +25,7 @@ struct apply_state {
const char *prefix;
int prefix_length;
+ int allow_overlap;
int apply_in_reverse;
int apply_with_reject;
int apply_verbosely;
@@ -58,7 +59,6 @@ static int diffstat;
static int numstat;
static int summary;
static int apply = 1;
-static int allow_overlap;
static int no_add;
static int threeway;
static int unsafe_paths;
@@ -2635,7 +2635,8 @@ static void remove_last_line(struct image *img)
* apply at applied_pos (counts in line numbers) in "img".
* Update "img" to remove "preimage" and replace it with "postimage".
*/
-static void update_image(struct image *img,
+static void update_image(struct apply_state *state,
+struct image *img,
int applied_pos,
struct image *preimage,
struct image *postimage)
@@ -2700,7 +2701,7 @@ static void update_image(struct image *img,
memcpy(img->line + applied_pos,
postimage->line,
postimage->nr * sizeof(*img->line));
- if (!allow_overlap)
+ if (!state->allow_overlap)
for (i = 0; i < postimage->nr; i++)
img->line[applied_pos + i].flag |= LINE_PATCHED;
img->nr = nr;
@@ -2948,7 +2949,7 @@ static int apply_one_fragment(struct apply_state *state,
fprintf_ln(stderr, _("Context reduced to (%ld/%ld)"
" to apply fragment at %d"),
leading, trailing, applied_pos+1);
- update_image(img, applied_pos, &preimage, &postimage);
+ update_image(state, img, applied_pos, &preimage, &postimage);
} else {
if (state->apply_verbosely)
error(_("while searching for:\n%.*s"),
@@ -4629,7 +4630,7 @@ int cmd_apply(int argc, const char **argv, const char
*prefix_)
N_("don't expect at least one line of context")),
OPT_BOOL(0, "reject", &state.apply_with_reject,
N_("leave the rejected hunks in corresponding *.rej
files")),
- OPT_BOOL(0, "allow-overlap", &allow_overlap,
+ OPT_BOOL(0, "allow-overlap", &state.allow_overlap,
N_("allow overlapping hunks")),
OPT__VERBOSE(&state.apply_verbosely, N_("be verbose")),
OPT_BIT(0, "inaccurate-eof", &options,
--
2.8.1.300.g5fed0c0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 18/83] builtin/apply: move 'numstat' global into 'struct apply_state'
Signed-off-by: Christian Couder
---
builtin/apply.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index d90948a..16d78f9 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -36,6 +36,9 @@ struct apply_state {
/* --stat does just a diffstat, and doesn't actually apply */
int diffstat;
+ /* --numstat does numeric diffstat, and doesn't actually apply */
+ int numstat;
+
/*
* --check turns on checking that the working tree matches the
*files that are being modified, but doesn't apply the patch
@@ -51,14 +54,12 @@ struct apply_state {
};
/*
- * --numstat does numeric diffstat, and doesn't actually apply
* --index-info shows the old and new index info for paths if available.
*/
static int newfd = -1;
static int state_p_value = 1;
static int p_value_known;
-static int numstat;
static int summary;
static int apply = 1;
static int no_add;
@@ -4500,7 +4501,7 @@ static int apply_patch(struct apply_state *state,
if (state->diffstat)
stat_patch_list(list);
- if (numstat)
+ if (state->numstat)
numstat_patch_list(list);
if (summary)
@@ -4598,7 +4599,7 @@ int cmd_apply(int argc, const char **argv, const char
*prefix_)
N_("instead of applying the patch, output diffstat for
the input")),
OPT_NOOP_NOARG(0, "allow-binary-replacement"),
OPT_NOOP_NOARG(0, "binary"),
- OPT_BOOL(0, "numstat", &numstat,
+ OPT_BOOL(0, "numstat", &state.numstat,
N_("show number of added and deleted lines in decimal
notation")),
OPT_BOOL(0, "summary", &summary,
N_("instead of applying the patch, output a summary for
the input")),
@@ -4675,7 +4676,7 @@ int cmd_apply(int argc, const char **argv, const char
*prefix_)
}
if (state.apply_with_reject)
apply = state.apply_verbosely = 1;
- if (!force_apply && (state.diffstat || numstat || summary ||
state.check || fake_ancestor))
+ if (!force_apply && (state.diffstat || state.numstat || summary ||
state.check || fake_ancestor))
apply = 0;
if (state.check_index && is_not_gitdir)
die(_("--index outside a repository"));
--
2.8.1.300.g5fed0c0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 11/83] builtin/apply: move 'apply_in_reverse' global into 'struct apply_state'
Signed-off-by: Christian Couder
---
builtin/apply.c | 51 ---
1 file changed, 32 insertions(+), 19 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 3f8671c..755e0e3 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -25,6 +25,8 @@ struct apply_state {
const char *prefix;
int prefix_length;
+ int apply_in_reverse;
+
/*
* --check turns on checking that the working tree matches the
*files that are being modified, but doesn't apply the patch
@@ -53,7 +55,6 @@ static int diffstat;
static int numstat;
static int summary;
static int apply = 1;
-static int apply_in_reverse;
static int apply_with_reject;
static int apply_verbosely;
static int allow_overlap;
@@ -1561,8 +1562,11 @@ static void check_whitespace(const char *line, int len,
unsigned ws_rule)
* between a "---" that is part of a patch, and a "---" that starts
* the next patch is to look at the line counts..
*/
-static int parse_fragment(const char *line, unsigned long size,
- struct patch *patch, struct fragment *fragment)
+static int parse_fragment(struct apply_state *state,
+ const char *line,
+ unsigned long size,
+ struct patch *patch,
+ struct fragment *fragment)
{
int added, deleted;
int len = linelen(line, size), offset;
@@ -1602,12 +1606,12 @@ static int parse_fragment(const char *line, unsigned
long size,
if (!deleted && !added)
leading++;
trailing++;
- if (!apply_in_reverse &&
+ if (!state->apply_in_reverse &&
ws_error_action == correct_ws_error)
check_whitespace(line, len, patch->ws_rule);
break;
case '-':
- if (apply_in_reverse &&
+ if (state->apply_in_reverse &&
ws_error_action != nowarn_ws_error)
check_whitespace(line, len, patch->ws_rule);
deleted++;
@@ -1615,7 +1619,7 @@ static int parse_fragment(const char *line, unsigned long
size,
trailing = 0;
break;
case '+':
- if (!apply_in_reverse &&
+ if (!state->apply_in_reverse &&
ws_error_action != nowarn_ws_error)
check_whitespace(line, len, patch->ws_rule);
added++;
@@ -1671,7 +1675,10 @@ static int parse_fragment(const char *line, unsigned
long size,
* The (fragment->patch, fragment->size) pair points into the memory given
* by the caller, not a copy, when we return.
*/
-static int parse_single_patch(const char *line, unsigned long size, struct
patch *patch)
+static int parse_single_patch(struct apply_state *state,
+ const char *line,
+ unsigned long size,
+ struct patch *patch)
{
unsigned long offset = 0;
unsigned long oldlines = 0, newlines = 0, context = 0;
@@ -1683,7 +1690,7 @@ static int parse_single_patch(const char *line, unsigned
long size, struct patch
fragment = xcalloc(1, sizeof(*fragment));
fragment->linenr = linenr;
- len = parse_fragment(line, size, patch, fragment);
+ len = parse_fragment(state, line, size, patch, fragment);
if (len <= 0)
die(_("corrupt patch at line %d"), linenr);
fragment->patch = line;
@@ -2013,8 +2020,10 @@ static int parse_chunk(struct apply_state *state, char
*buffer, unsigned long si
? patch->new_name
: patch->old_name);
- patchsize = parse_single_patch(buffer + offset + hdrsize,
- size - offset - hdrsize, patch);
+ patchsize = parse_single_patch(state,
+ buffer + offset + hdrsize,
+ size - offset - hdrsize,
+ patch);
if (!patchsize) {
static const char git_binary[] = "GIT binary patch\n";
@@ -2747,7 +2756,7 @@ static int apply_one_fragment(struct apply_state *state,
if (len < size && patch[len] == '\\')
plen--;
first = *patch;
- if (apply_in_reverse) {
+ if (state->ap
[PATCH 16/83] builtin/apply: move 'cached' global into 'struct apply_state'
Signed-off-by: Christian Couder
---
builtin/apply.c | 37 +
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index a5dff99..ba828df 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -30,6 +30,9 @@ struct apply_state {
int apply_with_reject;
int apply_verbosely;
+ /* --cached updates only the cache without ever touching the working
tree. */
+ int cached;
+
/*
* --check turns on checking that the working tree matches the
*files that are being modified, but doesn't apply the patch
@@ -48,13 +51,11 @@ struct apply_state {
* --stat does just a diffstat, and doesn't actually apply
* --numstat does numeric diffstat, and doesn't actually apply
* --index-info shows the old and new index info for paths if available.
- * --cached updates only the cache without ever touching the working tree.
*/
static int newfd = -1;
static int state_p_value = 1;
static int p_value_known;
-static int cached;
static int diffstat;
static int numstat;
static int summary;
@@ -3272,7 +3273,7 @@ static int load_patch_target(struct apply_state *state,
const char *name,
unsigned expected_mode)
{
- if (cached || state->check_index) {
+ if (state->cached || state->check_index) {
if (read_file_or_gitlink(ce, buf))
return error(_("read of %s failed"), name);
} else if (name) {
@@ -3545,7 +3546,7 @@ static int check_preimage(struct apply_state *state,
return error(_("path %s has been renamed/deleted"), old_name);
if (previous) {
st_mode = previous->new_mode;
- } else if (!cached) {
+ } else if (!state->cached) {
stat_ret = lstat(old_name, st);
if (stat_ret && errno != ENOENT)
return error(_("%s: %s"), old_name, strerror(errno));
@@ -3563,9 +3564,9 @@ static int check_preimage(struct apply_state *state,
if (checkout_target(&the_index, *ce, st))
return -1;
}
- if (!cached && verify_index_match(*ce, st))
+ if (!state->cached && verify_index_match(*ce, st))
return error(_("%s: does not match index"), old_name);
- if (cached)
+ if (state->cached)
st_mode = (*ce)->ce_mode;
} else if (stat_ret < 0) {
if (patch->is_new < 0)
@@ -3573,7 +3574,7 @@ static int check_preimage(struct apply_state *state,
return error(_("%s: %s"), old_name, strerror(errno));
}
- if (!cached && !previous)
+ if (!state->cached && !previous)
st_mode = ce_mode_from_stat(*ce, st->st_mode);
if (patch->is_new < 0)
@@ -3611,7 +3612,7 @@ static int check_to_create(struct apply_state *state,
cache_name_pos(new_name, strlen(new_name)) >= 0 &&
!ok_if_exists)
return EXISTS_IN_INDEX;
- if (cached)
+ if (state->cached)
return 0;
if (!lstat(new_name, &nst)) {
@@ -4105,7 +4106,7 @@ static void remove_file(struct apply_state *state, struct
patch *patch, int rmdi
if (remove_file_from_cache(patch->old_name) < 0)
die(_("unable to remove %s from index"),
patch->old_name);
}
- if (!cached) {
+ if (!state->cached) {
if (!remove_or_warn(patch->old_mode, patch->old_name) &&
rmdir_empty) {
remove_path(patch->old_name);
}
@@ -4138,7 +4139,7 @@ static void add_index_file(struct apply_state *state,
get_sha1_hex(s, ce->sha1))
die(_("corrupt patch for submodule %s"), path);
} else {
- if (!cached) {
+ if (!state->cached) {
if (lstat(path, &st) < 0)
die_errno(_("unable to stat newly created file
'%s'"),
path);
@@ -4190,9 +4191,13 @@ static int try_create_file(const char *path, unsigned
int mode, const char *buf,
* which is true 99% of the time anyway. If they don't,
* we create them and try again.
*/
-static void create_one_file(char *path, unsigned mode, const char *buf,
unsigned long size)
+static void create_one_file(struct apply_state *state,
+ char *path,
+ unsigned mode,
+ const char *buf,
+ unsigned long size)
{
-
[PATCH 04/83] builtin/apply: avoid local variable shadowing 'len' parameter
Signed-off-by: Christian Couder
---
builtin/apply.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 7115dc2..78849e4 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2194,17 +2194,17 @@ static void update_pre_post_images(struct image
*preimage,
fixed = preimage->buf;
for (i = reduced = ctx = 0; i < postimage->nr; i++) {
- size_t len = postimage->line[i].len;
+ size_t l_len = postimage->line[i].len;
if (!(postimage->line[i].flag & LINE_COMMON)) {
/* an added line -- no counterparts in preimage */
- memmove(new, old, len);
- old += len;
- new += len;
+ memmove(new, old, l_len);
+ old += l_len;
+ new += l_len;
continue;
}
/* a common context -- skip it in the original postimage */
- old += len;
+ old += l_len;
/* and find the corresponding one in the fixed preimage */
while (ctx < preimage->nr &&
@@ -2223,11 +2223,11 @@ static void update_pre_post_images(struct image
*preimage,
}
/* and copy it in, while fixing the line length */
- len = preimage->line[ctx].len;
- memcpy(new, fixed, len);
- new += len;
- fixed += len;
- postimage->line[i].len = len;
+ l_len = preimage->line[ctx].len;
+ memcpy(new, fixed, l_len);
+ new += l_len;
+ fixed += l_len;
+ postimage->line[i].len = l_len;
ctx++;
}
--
2.8.1.300.g5fed0c0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 14/83] builtin/apply: move 'update_index' global into 'struct apply_state'
Signed-off-by: Christian Couder
---
builtin/apply.c | 46 +++---
1 file changed, 27 insertions(+), 19 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 154679e..b57be2c 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -39,6 +39,8 @@ struct apply_state {
int check_index;
int unidiff_zero;
+
+ int update_index;
};
/*
@@ -51,7 +53,6 @@ static int newfd = -1;
static int state_p_value = 1;
static int p_value_known;
-static int update_index;
static int cached;
static int diffstat;
static int numstat;
@@ -4097,9 +4098,9 @@ static void patch_stats(struct patch *patch)
}
}
-static void remove_file(struct patch *patch, int rmdir_empty)
+static void remove_file(struct apply_state *state, struct patch *patch, int
rmdir_empty)
{
- if (update_index) {
+ if (state->update_index) {
if (remove_file_from_cache(patch->old_name) < 0)
die(_("unable to remove %s from index"),
patch->old_name);
}
@@ -4110,14 +4111,18 @@ static void remove_file(struct patch *patch, int
rmdir_empty)
}
}
-static void add_index_file(const char *path, unsigned mode, void *buf,
unsigned long size)
+static void add_index_file(struct apply_state *state,
+ const char *path,
+ unsigned mode,
+ void *buf,
+ unsigned long size)
{
struct stat st;
struct cache_entry *ce;
int namelen = strlen(path);
unsigned ce_size = cache_entry_size(namelen);
- if (!update_index)
+ if (!state->update_index)
return;
ce = xcalloc(1, ce_size);
@@ -4227,13 +4232,14 @@ static void create_one_file(char *path, unsigned mode,
const char *buf, unsigned
die_errno(_("unable to write file '%s' mode %o"), path, mode);
}
-static void add_conflicted_stages_file(struct patch *patch)
+static void add_conflicted_stages_file(struct apply_state *state,
+ struct patch *patch)
{
int stage, namelen;
unsigned ce_size, mode;
struct cache_entry *ce;
- if (!update_index)
+ if (!state->update_index)
return;
namelen = strlen(patch->new_name);
ce_size = cache_entry_size(namelen);
@@ -4254,7 +4260,7 @@ static void add_conflicted_stages_file(struct patch
*patch)
}
}
-static void create_file(struct patch *patch)
+static void create_file(struct apply_state *state, struct patch *patch)
{
char *path = patch->new_name;
unsigned mode = patch->new_mode;
@@ -4266,22 +4272,24 @@ static void create_file(struct patch *patch)
create_one_file(path, mode, buf, size);
if (patch->conflicted_threeway)
- add_conflicted_stages_file(patch);
+ add_conflicted_stages_file(state, patch);
else
- add_index_file(path, mode, buf, size);
+ add_index_file(state, path, mode, buf, size);
}
/* phase zero is to remove, phase one is to create */
-static void write_out_one_result(struct patch *patch, int phase)
+static void write_out_one_result(struct apply_state *state,
+struct patch *patch,
+int phase)
{
if (patch->is_delete > 0) {
if (phase == 0)
- remove_file(patch, 1);
+ remove_file(state, patch, 1);
return;
}
if (patch->is_new > 0 || patch->is_copy) {
if (phase == 1)
- create_file(patch);
+ create_file(state, patch);
return;
}
/*
@@ -4289,9 +4297,9 @@ static void write_out_one_result(struct patch *patch, int
phase)
* thing: remove the old, write the new
*/
if (phase == 0)
- remove_file(patch, patch->is_rename);
+ remove_file(state, patch, patch->is_rename);
if (phase == 1)
- create_file(patch);
+ create_file(state, patch);
}
static int write_out_one_reject(struct apply_state *state, struct patch *patch)
@@ -4378,7 +4386,7 @@ static int write_out_results(struct apply_state *state,
struct patch *list)
if (l->rejected)
errs = 1;
else {
- write_out_one_result(l, phase);
+ write_out_one_result(state, l, phase);
if (phase == 1) {
if (write_out_one_reject(state, l))
errs = 1;
@@ -4458,8 +4466,8 @@ static int apply_patch(struct apply_state *state,
if
[PATCH 05/83] builtin/apply: extract line_by_line_fuzzy_match() from match_fragment()
The match_fragment() function is very big and contains a big special case
algorithm that does line by line fuzzy matching. So let's extract this
algorithm in a separate line_by_line_fuzzy_match() function.
Signed-off-by: Christian Couder
---
builtin/apply.c | 129
1 file changed, 73 insertions(+), 56 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 78849e4..02239d9 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2242,6 +2242,74 @@ static void update_pre_post_images(struct image
*preimage,
postimage->nr -= reduced;
}
+static int line_by_line_fuzzy_match(struct image *img,
+ struct image *preimage,
+ struct image *postimage,
+ unsigned long try,
+ int try_lno,
+ int preimage_limit)
+{
+ int i;
+ size_t imgoff = 0;
+ size_t preoff = 0;
+ size_t postlen = postimage->len;
+ size_t extra_chars;
+ char *buf;
+ char *preimage_eof;
+ char *preimage_end;
+ struct strbuf fixed;
+ char *fixed_buf;
+ size_t fixed_len;
+
+ for (i = 0; i < preimage_limit; i++) {
+ size_t prelen = preimage->line[i].len;
+ size_t imglen = img->line[try_lno+i].len;
+
+ if (!fuzzy_matchlines(img->buf + try + imgoff, imglen,
+ preimage->buf + preoff, prelen))
+ return 0;
+ if (preimage->line[i].flag & LINE_COMMON)
+ postlen += imglen - prelen;
+ imgoff += imglen;
+ preoff += prelen;
+ }
+
+ /*
+* Ok, the preimage matches with whitespace fuzz.
+*
+* imgoff now holds the true length of the target that
+* matches the preimage before the end of the file.
+*
+* Count the number of characters in the preimage that fall
+* beyond the end of the file and make sure that all of them
+* are whitespace characters. (This can only happen if
+* we are removing blank lines at the end of the file.)
+*/
+ buf = preimage_eof = preimage->buf + preoff;
+ for ( ; i < preimage->nr; i++)
+ preoff += preimage->line[i].len;
+ preimage_end = preimage->buf + preoff;
+ for ( ; buf < preimage_end; buf++)
+ if (!isspace(*buf))
+ return 0;
+
+ /*
+* Update the preimage and the common postimage context
+* lines to use the same whitespace as the target.
+* If whitespace is missing in the target (i.e.
+* if the preimage extends beyond the end of the file),
+* use the whitespace from the preimage.
+*/
+ extra_chars = preimage_end - preimage_eof;
+ strbuf_init(&fixed, imgoff + extra_chars);
+ strbuf_add(&fixed, img->buf + try, imgoff);
+ strbuf_add(&fixed, preimage_eof, extra_chars);
+ fixed_buf = strbuf_detach(&fixed, &fixed_len);
+ update_pre_post_images(preimage, postimage,
+ fixed_buf, fixed_len, postlen);
+ return 1;
+}
+
static int match_fragment(struct image *img,
struct image *preimage,
struct image *postimage,
@@ -2251,7 +2319,7 @@ static int match_fragment(struct image *img,
int match_beginning, int match_end)
{
int i;
- char *fixed_buf, *buf, *orig, *target;
+ char *fixed_buf, *orig, *target;
struct strbuf fixed;
size_t fixed_len, postlen;
int preimage_limit;
@@ -2312,6 +2380,7 @@ static int match_fragment(struct image *img,
* There must be one non-blank context line that match
* a line before the end of img.
*/
+ char *buf;
char *buf_end;
buf = preimage->buf;
@@ -2331,61 +2400,9 @@ static int match_fragment(struct image *img,
* fuzzy matching. We collect all the line length information because
* we need it to adjust whitespace if we match.
*/
- if (ws_ignore_action == ignore_ws_change) {
- size_t imgoff = 0;
- size_t preoff = 0;
- size_t postlen = postimage->len;
- size_t extra_chars;
- char *preimage_eof;
- char *preimage_end;
- for (i = 0; i < preimage_limit; i++) {
- size_t prelen = preimage->line[i].len;
- size_t imglen = img->line[try_lno+i].len;
-
- if (!fuzzy_matchlines(img->buf + try + imgoff, imglen,
- preimage->buf + preoff,
Re: [PATCH 00/83] libify apply and use lib in am
On Sun, Apr 24, 2016 at 5:23 PM, Ramsay Jones wrote: > > > On 24/04/16 14:33, Christian Couder wrote: >> This is a patch series about libifying `git apply` functionality, and >> using this libified functionality in `git am`, so that no 'git apply' >> process is spawn anymore. This makes `git am` significantly faster, so >> `git rebase`, when it uses the am backend, is also significantly >> faster. > > I just tried to git-am these patches, but patch #78 did not make it > to the list. That's strange because gmail tells me it has been sent, and it made it to chrisc...@tuxfamily.org. I use send-email and smtp.gmail.com. Just after sending patch #78 the connection to smtp.gmail.com was closed with: 4.7.0 Try again later, closing connection. (MAIL) j6sm6717101wjb.29 - gsmtp Then I had to wait a few minutes before I could send the last patches. > (Also, patches #59 and #62 both issue a 'new blank line at EOF' warning). Ok, I will take a look at that. Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/83] libify apply and use lib in am
On Sun, Apr 24, 2016 at 6:27 PM, Christian Couder wrote: > On Sun, Apr 24, 2016 at 5:23 PM, Ramsay Jones > wrote: >> >> >> On 24/04/16 14:33, Christian Couder wrote: >>> This is a patch series about libifying `git apply` functionality, and >>> using this libified functionality in `git am`, so that no 'git apply' >>> process is spawn anymore. This makes `git am` significantly faster, so >>> `git rebase`, when it uses the am backend, is also significantly >>> faster. >> >> I just tried to git-am these patches, but patch #78 did not make it >> to the list. > > That's strange because gmail tells me it has been sent, and it made it > to chrisc...@tuxfamily.org. Instead of waiting for the patch to appear on the list, you might want to use branch libify-apply-use-in-am25 in my GitHub repo: https://github.com/chriscool/git/commits/libify-apply-use-in-am25 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/83] libify apply and use lib in am
On Mon, Apr 25, 2016 at 2:14 AM, Duy Nguyen wrote: > On Mon, Apr 25, 2016 at 12:42 AM, Ramsay Jones > wrote: >> >> >> On 24/04/16 17:56, Christian Couder wrote: >>> On Sun, Apr 24, 2016 at 6:27 PM, Christian Couder >>> wrote: >>>> On Sun, Apr 24, 2016 at 5:23 PM, Ramsay Jones >>>> wrote: >>>>> >>>>> I just tried to git-am these patches, but patch #78 did not make it >>>>> to the list. >>>> >>>> That's strange because gmail tells me it has been sent, and it made it >>>> to chrisc...@tuxfamily.org. > > #78 moves 4000k lines around and ends up ~260k in size, I think it may > have hit vger limit. Yeah probably. Thanks for looking at this. I think I will have to split it into smaller patches in a v2. >>> Instead of waiting for the patch to appear on the list, you might want >>> to use branch libify-apply-use-in-am25 in my GitHub repo: >>> >>> https://github.com/chriscool/git/commits/libify-apply-use-in-am25 >> >> Hmm, that branch doesn't correspond directly to the patches you sent >> out (there are 86 commits, some marked with draft. I think commit d13d2ac >> corresponds kinda to patch #83, but ). >> >> I think I'll wait to see the patches as you intend them to be seen. ;-) > > I git-am'd the series then compared with the rebased version of > libify-apply-use-in-am25 on master. 33198a1 (i.e. > libify-apply-use-in-am25^) matches what was sent in content (didn't > compare commit messages). Thanks for checking. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/83] libify apply and use lib in am
On Mon, Apr 25, 2016 at 11:02 AM, Duy Nguyen wrote: > On Sun, Apr 24, 2016 at 8:33 PM, Christian Couder > wrote: >> Sorry if this patch series is a bit long. I can split it into two or >> more series if it is prefered. > > I suspect you deliberately made the series long just to show off how > good new git-rebase is ;-) Yeah, and `git am` too :-) >> Performance numbers: >> >> - A few days ago Ævar did a huge many-hundred commit rebase on the >> kernel with untracked cache. >> >> command: git rebase --onto 1993b17 52bef0c 29dde7c >> >> Vanilla "next" without split index:1m54.953s >> Vanilla "next" with split index: 1m22.476s >> This series on top of "next" without split index: 1m12.034s >> This series on top of "next" with split index: 0m15.678s > > I was a bit puzzled why split-index helped so much. It shouldn't have > in my opinion unless you paired it with index-helper, to cut down both > read and write time. So I ran some tests. Long story short, I think we > can achieve this gain (and a little more) even without split-index. Yeah, perhaps. For now though Ævar and myself would be happy to just have a config option for split-index :-) The other performance numbers I mentioned show that now the `git am` part of a 13 commit long rebase is reduced from 58% to 19% of the whole rebase time. So further improvements in speeding up `git am` could only make such a rebase at most 19% faster. > I ran my measurement patch [1] with and without your series used this > series as rebase material. Without the series, the picture is not so > surprising. We run git-apply 80+ times, each consists of this sequence > > read index > write index (cache tree updates only) > read index again > optionally initialize name hash (when new entries are added, I guess) > read packed-refs > write index > > With this series, we run a single git-apply which does > > read index (and sharedindex too if in split-index mode) > initialize name hash > write index 80+ times > > This explains why I guessed it wrong: when you write only, not read > back, of course index-helper is not required. And with split-index you > only write as many entries as you touch (which is usually a small > number compared to worktree's size), so writing 80+ times with > split-index is a lot faster than write 80+ times with whole index. Yeah, I tried to explain in the cover letter and in the last patch why this series enables split-index to give great results, but I think you are much better at explaining it. > But why write so many times when nobody reads it? We only need to > write before git-apply exits, You mean `git am` here I think. > either after successfully applying the > whole series, or after it stops at conflicts, and maybe even at die() > and SIGINT. Yes if git-apply segfaults, Here too. > then the index update is lost, > but in such a case, it's usually a good idea to restart fresh anyway. > When you only write index once (or twice) it won't matter if > split-index is used. Yeah I agree, but it would need further work, that can be done after this series is merged. And I am not sure if the potential gains on a typical rebase would be worth it. Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/83] builtin/apply: extract line_by_line_fuzzy_match() from match_fragment()
On Mon, Apr 25, 2016 at 8:50 PM, Stefan Beller wrote:
>> @@ -2251,7 +2319,7 @@ static int match_fragment(struct image *img,
>> int match_beginning, int match_end)
>> {
>> int i;
>> - char *fixed_buf, *buf, *orig, *target;
>> + char *fixed_buf, *orig, *target;
>> struct strbuf fixed;
>> size_t fixed_len, postlen;
>> int preimage_limit;
>> @@ -2312,6 +2380,7 @@ static int match_fragment(struct image *img,
>> * There must be one non-blank context line that match
>> * a line before the end of img.
>> */
>> + char *buf;
>
> patches 1-4 looking good, with no comment from me. Here is the first spot to
> comment on.
>
> It's not clear why we need to declare buf here? Oh wait it is. It's just
> moved from the start of the function. But why do it in this patch?
> It seems unrelated to the general intent of the patch. No need to reroll
> for this nit alone, it just took me a while to figure out it was an unrelated
> thing.
Yeah, I agree it's a bit unrelated. But rather than add another patch
to an already long series just for this, I added the following to the
commit message:
While at it, let's reduce the scope of "char *buf" in match_fragment().
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/83] builtin/apply: move 'check' global into 'struct apply_state'
On Mon, Apr 25, 2016 at 8:57 PM, Stefan Beller wrote:
> On Sun, Apr 24, 2016 at 6:33 AM, Christian Couder
> wrote:
>> Signed-off-by: Christian Couder
>> ---
>> builtin/apply.c | 16 +---
>> 1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/builtin/apply.c b/builtin/apply.c
>> index ad81210..6c628f6 100644
>> --- a/builtin/apply.c
>> +++ b/builtin/apply.c
>> @@ -25,12 +25,15 @@ struct apply_state {
>> const char *prefix;
>> int prefix_length;
>>
>> + /*
>> +* --check turns on checking that the working tree matches the
>> +*files that are being modified, but doesn't apply the patch
>
> This is true, but at this part of the file/code we rather want to know what
> `check` does, instead of what the command line option --check does.
> (They are 2 different things, though one leading to the other one?) How about:
>
> /*
> * Only check the files to be modified, but do not modify the files.
> */
>
>
>> /*
>> - * --check turns on checking that the working tree matches the
>> - *files that are being modified, but doesn't apply the patch
>
> Oh I see it was moved from here. Not sure if we want to rename
> comments along the way or just keep it in this series.
I kept the existing comments when they were still relevant.
It could be a cleanup to change them to something like what you
suggest, but as it is not important for this series which is already
long, I prefer to leave it for now.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 18/83] builtin/apply: move 'numstat' global into 'struct apply_state'
On Mon, Apr 25, 2016 at 11:40 PM, Stefan Beller wrote:
> On Sun, Apr 24, 2016 at 6:33 AM, Christian Couder
> wrote:
>> Signed-off-by: Christian Couder
>> ---
>> builtin/apply.c | 11 ++-
>> 1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/builtin/apply.c b/builtin/apply.c
>> index d90948a..16d78f9 100644
>> --- a/builtin/apply.c
>> +++ b/builtin/apply.c
>> @@ -36,6 +36,9 @@ struct apply_state {
>> /* --stat does just a diffstat, and doesn't actually apply */
>> int diffstat;
>>
>> + /* --numstat does numeric diffstat, and doesn't actually apply */
>> + int numstat;
>> +
>> /*
>> * --check turns on checking that the working tree matches the
>> *files that are being modified, but doesn't apply the patch
>> @@ -51,14 +54,12 @@ struct apply_state {
>> };
>>
>> /*
>> - * --numstat does numeric diffstat, and doesn't actually apply
>> * --index-info shows the old and new index info for paths if available.
>> */
>> static int newfd = -1;
>>
>> static int state_p_value = 1;
>> static int p_value_known;
>> -static int numstat;
>> static int summary;
>> static int apply = 1;
>> static int no_add;
>> @@ -4500,7 +4501,7 @@ static int apply_patch(struct apply_state *state,
>> if (state->diffstat)
>> stat_patch_list(list);
>>
>> - if (numstat)
>> + if (state->numstat)
>> numstat_patch_list(list);
>>
>> if (summary)
>> @@ -4598,7 +4599,7 @@ int cmd_apply(int argc, const char **argv, const char
>> *prefix_)
>> N_("instead of applying the patch, output diffstat
>> for the input")),
>> OPT_NOOP_NOARG(0, "allow-binary-replacement"),
>> OPT_NOOP_NOARG(0, "binary"),
>> - OPT_BOOL(0, "numstat", &numstat,
>> + OPT_BOOL(0, "numstat", &state.numstat,
>> N_("show number of added and deleted lines in
>> decimal notation")),
>> OPT_BOOL(0, "summary", &summary,
>> N_("instead of applying the patch, output a summary
>> for the input")),
>> @@ -4675,7 +4676,7 @@ int cmd_apply(int argc, const char **argv, const char
>> *prefix_)
>> }
>> if (state.apply_with_reject)
>> apply = state.apply_verbosely = 1;
>> - if (!force_apply && (state.diffstat || numstat || summary ||
>> state.check || fake_ancestor))
>> + if (!force_apply && (state.diffstat || state.numstat || summary ||
>> state.check || fake_ancestor))
>
> Mental note: This patch is just doing a mechanical conversion, so it
> is fine to check for many "state.FOOs" here.
>
> However later we may want to move this out to a static oneliner like:
>
> static int really_apply(state *s) {
> return s->diffstat || s->numstat || ...;
> }
>
> (with a better name obviously)
Yeah, this is another cleanup that could be done.
I added it to a list and will try to take care of it later.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RFC: Supporting .git/hooks/$NAME.d/* && /etc/git/hooks/$NAME.d/*
On Tue, Apr 26, 2016 at 6:09 PM, Ævar Arnfjörð Bjarmason wrote: > On Tue, Apr 26, 2016 at 3:40 PM, Marc Branchaud wrote: >> On 2016-04-26 06:58 AM, Ævar Arnfjörð Bjarmason wrote: >>> >>> Makes sense to have an experimental.* config tree for git for stuff like >>> this. >> >> I disagree. >> >> * If the point is to express some kind of warning to users, I think the >> community has been much better served by leaving experimental settings >> undocumented (or documented only in unmerged topic branches). There are features considered experimental that are documented, like --split-index in `git update-index` and `git interpret-trailers`. As far as I know the possible reasons they are considered experimental are: - in the release notes they were described as "experimental", - they are not considered complete, because of missing features, - they might not be useful as is, - they might be buggy in the real world. >> It feels like >> an experimental.* tree is a doorway to putting experimental features in >> official releases, which seems odd considering that (IMHO) git has so far >> done very well with the carefully-planned-out integration of all sorts of >> features. Some people have been happily experimenting with or even using some of the experimental features, like the ones I am talking about above. >> * Part of the experiment is coming up with appropriate configuration knobs, >> including where those knobs should live. I agree that it can be difficult to find the right knobs for any new feature. >> Often such considerations lead to a >> better implementation for the feature. Dumping things into an experimental.* >> tree would merely postpone that part of the feature's design. I am not so sure about this. For example `git update-index --untracked-cache` used to be considered experimental, but it definitely had knobs that were not right, so its UI has been reworked recently. Maybe if it had first been created with an UI in an experimental.* config tree, rather than only options to `git update-index` it would have been an easier transition. >> * Such a tree creates a flag day when the experimental feature eventually >> becomes a "real" feature. That'll annoy any early adopters. Sure, they >> *should* be prepared to deal with config tree bike-shedding, but still that >> extra churn seems unnecessary. Not sure about that. New config options outside the experimental.* config tree could always take over config options in the experimental.* config tree, as if they appear, it means that the user is aware that they are the new way to configure the feature. Then the cost of supporting both the old options in the experimental.* config tree and the new ones outside should be very small, especially if there are not many changes between the two set of options. If there are a lot of changes between these two sets, it means that we were probably right to have the old ones in the experimental.* config tree. And in the worst case, which should hardly ever happen, we can probably afford to just emit warnings saying "old 'experimental.' config option is not supported anymore, please use config option instead" and just ignore the 'experimental.' config option. > By "stuff like this", yeah I did mean, and I assume Junio means, > putting "experimental" features in official releases. > > E.g. perl does this, if you type "perldoc experimental" on a Linux box > you'll get the documentation. > > Basically you can look at patches to a project on a spectrum of: > > 1. You hacked something up locally > 2. It's in someone's *.git repo as a POC > 3. It's a third-party patch series used by a bunch of people > 4. In an official release but documented as experimental > 5. In an official release as a first-rate feature Yeah, and it seems to me that Git also has: 4.5. In an official release, documented, but scarcely documented as experimental and in fact more stuff under 4.5. than under 4. And in Git there is also the notion of porcelain vs plumbing, where porcelain output can more easily be changed a bit than plumbing output. > Something like an experimental.WHATEVER=bool flag provides #4. > > I think aside from this feature just leaving these things undocumented > really provides the worst of both worlds. Yeah I agree. Undocumented stuff in Git is already for stuff that we don't want people to use. > Now you have some feature that's undocumented *because* you're not > sure about it, but you can't ever be sure about it unless people > actually use it, and if it's not documented at all effectively it's as > visible as some third-party patch series. I.e. only people really > involved in the project will ever use it. > > Which is why perl has the "experimental" subsystem, it allows for > playing around with features the maintainers aren't quite sure about > in official releases, and the users know they're opting in to trying > something unstable that may go away or have its semantics changed from > under the
Re: [PATCH 33/83] builtin/apply: move 'root' global into 'struct apply_state'
On Mon, Apr 25, 2016 at 11:50 PM, Stefan Beller wrote:
> On Sun, Apr 24, 2016 at 6:33 AM, Christian Couder
> wrote:
>
>> @@ -4630,9 +4644,10 @@ static int option_parse_whitespace(const struct
>> option *opt,
>> static int option_parse_directory(const struct option *opt,
>> const char *arg, int unset)
>> {
>> - strbuf_reset(&root);
>> - strbuf_addstr(&root, arg);
>> - strbuf_complete(&root, '/');
>> + struct apply_state *state = opt->value;
>
> Or even
>
> struct strbuf root = ((state*)opt->value)->root;
>
> and then keep the next lines as is?
I found it more coherent to have all the option_parse_*() functions do:
struct apply_state *state = opt->value;
as their first instruction.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 45/83] builtin/apply: move 'state' init into init_apply_state()
On Mon, Apr 25, 2016 at 9:32 AM, Eric Sunshine wrote:
> On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder
> wrote:
>> Signed-off-by: Christian Couder
>> ---
>> diff --git a/builtin/apply.c b/builtin/apply.c
>> @@ -4670,6 +4670,28 @@ static int option_parse_directory(const struct option
>> *opt,
>> +static void init_apply_state(struct apply_state *state, const char *prefix_)
>> +{
>> + memset(state, 0, sizeof(*state));
>> + state->prefix = prefix_;
>> + state->prefix_length = state->prefix ? strlen(state->prefix) : 0;
>> + state->apply = 1;
>> + state->line_termination = '\n';
>> + state->p_value = 1;
>> + state->p_context = UINT_MAX;
>> + state->squelch_whitespace_errors = 5;
>> + state->ws_error_action = warn_on_ws_error;
>> + state->ws_ignore_action = ignore_ws_none;
>> + state->linenr = 1;
>> + strbuf_init(&state->root, 0);
>> +
>> + git_apply_config();
>> + if (apply_default_whitespace)
>> + parse_whitespace_option(state, apply_default_whitespace);
>> + if (apply_default_ignorewhitespace)
>> + parse_ignorewhitespace_option(state,
>> apply_default_ignorewhitespace);
>> +}
>
> Minor:
>
> If factoring out this code from cmd_apply() into init_apply_state()
> was done as a preparatory patch before introduction of 'apply_state',
> then each new 'state->foo=...' line would already be at its final
> location when added by its respective patch.
Yeah I agree. So I did something like that.
And by the way while doing it I saw that Junio also suggested a similar change.
The little difference with what you and Junio suggest is that the
patch that creates init_apply_state() is just after the patch that
introduces 'struct apply_state'. I think it is a bit cleaner this way,
as if I create init_apply_state() first, then I would have to change
its arguments in the next patch that introduces 'struct apply_state'.
> Doing so would also provide an obvious opportunity to name the
> 'prefix' argument to init_apply_state() "prefix" rather than the odd
> "prefix_".
Yeah, I did that too.
Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 22/83] builtin/apply: move 'unsafe_paths' global into 'struct apply_state'
On Tue, Apr 26, 2016 at 10:27 PM, Junio C Hamano wrote:
> Christian Couder writes:
>
>> Signed-off-by: Christian Couder
>> ---
>> builtin/apply.c | 9 +
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/builtin/apply.c b/builtin/apply.c
>> index 506357c..c45e481 100644
>> --- a/builtin/apply.c
>> +++ b/builtin/apply.c
>> @@ -57,6 +57,8 @@ struct apply_state {
>> int unidiff_zero;
>>
>> int update_index;
>> +
>> + int unsafe_paths;
>> };
>
> Having said
>
> I like the way this series moves only a few variables at a time to
> limit the scope of each step.
>
> it gets irritating to see all these unnecessary blank lines in the
> structure definition added by each step, which would mean all of
> these patches need to fix them in the next reroll.
The reason I added some blank lines is that I moved comments that were
all in one block at the top.
I moved each comment near the related variables and sometimes a
comment is related to 2 variables, like in the extract below the
comment that starts with "For "diff-stat" like behaviour,...":
--
/* --index updates the cache as well. */
int check_index;
int unidiff_zero;
int update_index;
int unsafe_paths;
int line_termination;
/*
* For "diff-stat" like behaviour, we keep track of the biggest change
* we've seen, and the longest filename. That allows us to do simple
* scaling.
*/
int max_change;
int max_len;
/*
* Various "current state", notably line numbers and what
* file (and how) we're patching right now.. The "is_"
* things are flags, where -1 means "don't know yet".
*/
int linenr;
--
If I remove all the blank lines, I think it will make it harder to
understand which comment belong to which variable(s).
Maybe a compromise would be to just remove blank lines between the
variables that don't have any related comment like "unidiff_zero",
"update_index", "unsafe_paths" and "line_termination" above.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 27/83] builtin/apply: move 'read_stdin' global into cmd_apply()
On Tue, Apr 26, 2016 at 10:28 PM, Junio C Hamano wrote: > Christian Couder writes: > >> Signed-off-by: Christian Couder >> --- >> builtin/apply.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > Makes sense but do so immediately next to 06/83, "options" thing? Ok, in the next reroll this patch will be immediately next to 06/83. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 39/83] builtin/apply: move 'ws_error_action' into 'struct apply_state'
On Tue, Apr 26, 2016 at 10:36 PM, Junio C Hamano wrote:
> Christian Couder writes:
>
>> +enum ws_error_action {
>> + nowarn_ws_error,
>> + warn_on_ws_error,
>> + die_on_ws_error,
>> + correct_ws_error
>> +};
>> +
>> struct apply_state {
>> const char *prefix;
>> int prefix_length;
>> @@ -80,6 +87,8 @@ struct apply_state {
>> int whitespace_error;
>> int squelch_whitespace_errors;
>> int applied_after_fixing_ws;
>> +
>> + enum ws_error_action ws_error_action;
>> };
>>
>> static int newfd = -1;
>> @@ -89,12 +98,6 @@ static const char * const apply_usage[] = {
>> NULL
>> };
>>
>> -static enum ws_error_action {
>> - nowarn_ws_error,
>> - warn_on_ws_error,
>> - die_on_ws_error,
>> - correct_ws_error
>> -} ws_error_action = warn_on_ws_error;
>
> This is a good example of a variable that needs initialization,
> which is turned into a field in apply_state that needs
> initialization. It is done here:
>
>> @@ -4738,6 +4743,7 @@ int cmd_apply(int argc, const char **argv, const char
>> *prefix_)
>> state.p_value = 1;
>> state.p_context = UINT_MAX;
>> state.squelch_whitespace_errors = 5;
>> + state.ws_error_action = warn_on_ws_error;
>> strbuf_init(&state.root, 0);
>
> and we already have these random initial values described here.
>
> As I do not expect that cmd_apply() which is a moral equivalent of
> main() will stay to be the only one who wants to see a reasonably
> initialized apply_state(), I think the patch that introduced the
> very first version of "struct apply_state" should also introduce a
> helper function to initialize it, i.e.
>
> static void init_apply_state(struct apply_state *s,
> const char *prefix)
> {
> memset(s, '\0', sizeof(*s));
> s->prefix = prefix;
> s->prefix_length = s->prefix ? strlen(s->prefix) : 0;
> }
>
> in [PATCH 7/xx].
Yeah, Eric also made nearly the same comment, so in the next reroll
the init_apply_state() is introduced in the patch immediately after
the patch that creates 'struct apply_state' (rather than much later in
the series). I prefer to not introduce it in the same patch as the
patch that creates 'struct apply_state' is already quite big. (But if
you insist, yeah I will squash both patches together.)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/83] builtin/apply: extract line_by_line_fuzzy_match() from match_fragment()
On Tue, Apr 26, 2016 at 10:20 PM, Junio C Hamano wrote: > Christian Couder writes: > >>> It's not clear why we need to declare buf here? Oh wait it is. It's just >>> moved from the start of the function. But why do it in this patch? >>> It seems unrelated to the general intent of the patch. No need to reroll >>> for this nit alone, it just took me a while to figure out it was an >>> unrelated >>> thing. >> >> Yeah, I agree it's a bit unrelated. But rather than add another patch >> to an already long series just for this,... > > Isn't not doing the unrelated move at all a more sensible > alternative, if that change does not deserve its own place in the > series after all? Ok, I removed the unrelated move. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 47/83] builtin/apply: move applying patches into apply_all_patches()
On Tue, Apr 26, 2016 at 12:00 AM, Stefan Beller wrote: > On Sun, Apr 24, 2016 at 6:33 AM, Christian Couder > wrote: >> Signed-off-by: Christian Couder > > Up to this patch, have a > Reviewed-by: Stefan Beller > in case you want to split the series in here (as indicated in the > cover letter, this was the last > patch rerolled, the next patches are new and may need more discussion). > > I had some nits, but they cleared up in later patches. Thanks for your review! I will add your Reviewed-by in the next reroll. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/83] builtin/apply: avoid parameter shadowing 'linenr' global
On Wed, Apr 27, 2016 at 6:27 PM, Junio C Hamano wrote: > > I think 02/83 that renamed the global-to-be-moved-to-state to > state_p_value was brilliant, and this should follow suit; you would > be moving linenr into the state eventually in later steps, right? Yeah, ok, I will do the same thing to this patch as I did to 02/83. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] trailer: load config to handle core.commentChar
On Wed, Apr 27, 2016 at 9:24 PM, Rafal Klys wrote: > Add call to git_config(git_default_config, NULL) to update the > comment_char_line from default '#' to possible different value set in > core.commentChar. It is "comment_line_char" not "comment_char_line", but otherwise you can add "Reviewed-by: Christian Couder ". Thanks! > Signed-off-by: Rafal Klys > --- > trailer.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/trailer.c b/trailer.c > index 8e48a5c..a3700b4 100644 > --- a/trailer.c > +++ b/trailer.c > @@ -888,6 +888,9 @@ void process_trailers(const char *file, int in_place, int > trim_empty, struct str > git_config(git_trailer_default_config, NULL); > git_config(git_trailer_config, NULL); > > + /* for core.commentChar */ > + git_config(git_default_config, NULL); > + > lines = read_input_file(file); > > if (in_place) > -- > 2.8.1.68.g625efa9.dirty > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 49/83] builtin/apply: move 'lock_file' global into 'struct apply_state'
On Mon, Apr 25, 2016 at 7:55 PM, Junio C Hamano wrote: > Eric Sunshine writes: > >>> + /* >>> +* Since lockfile.c keeps a linked list of all created >>> +* lock_file structures, it isn't safe to free(lock_file). >>> +*/ >>> + struct lock_file *lock_file; >> >> Is there ever a time when lock_file is removed from the list (such as >> upon successful write of the index), in which case it would be safe to >> free() this? > > I do not think you need to think about "free"ing. Yeah, lockfile.h says: * * Automatic cruft removal. If the program exits after we lock a * file but before the changes have been committed, we want to make * sure that we remove the lockfile. This is done by remembering the * lockfiles we have created in a linked list and setting up an * `atexit(3)` handler and a signal handler that clean up the * lockfiles. This mechanism ensures that outstanding lockfiles are * cleaned up if the program exits (including when `die()` is * called) or if the program is terminated by a signal. and: * The caller: * * * Allocates a `struct lock_file` either as a static variable or on * the heap, initialized to zeros. Once you use the structure to * call the `hold_lock_file_for_*()` family of functions, it belongs * to the lockfile subsystem and its storage must remain valid * throughout the life of the program (i.e. you cannot use an * on-stack variable to hold this structure). > Even if the libified version of the apply internal can be called > multiple times to process multiple patch inputs, there is no need to > run multiple instances of it yet. And a lockfile, after the caller > finishes interacting with one file using it by calling commit or > rollback, can be reused to interact with another file. > > So the cleanest would be to: > > * make the caller of apply API responsible for preparing a static >or (allocating and leaking) lockfile instance, Ok. > * make it pass a pointer to the lockfile to the apply API so that >it can store it in apply_state, and Ok, I will add a new "struct lock_file *" parameter to init_apply_state(), for the caller of the apply API to do that. Though I think it should be ok for the caller to pass a NULL pointer and in this case have init_apply_state() allocate the lockfile file instance. > * have the caller use apply API feeding one patch at a time in a >loop, allowing the same lockfile instance used for each >"invocation". Ok, the same lockfile instance will be used for each invocation. > I sounded as if I were advocating non-reentrant implementation in > the introductory paragraph, but that is not the intention. If you > want to do N threads, you would prepare N lockfile instances, give > them to N apply API instances to be stored in N apply_state > instances, and you would have N parallel applications, if you wanted > to. Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] trailer: load config to handle core.commentChar
On Thu, Apr 28, 2016 at 10:00 PM, Rafal Klys wrote: > Fall throught git_default_config when reading config to update the > comment_line_char from default '#' to possible different value set in > core.commentChar. > > Signed-off-by: Rafal Klys > --- > > Added fallthru instead of reading config third time. > > Added test, updated commit message. > > I even tried to change that to only one pass, but looks like it would require > a > bit more coding, so maybe next time. > > Thanks for feedback, I'm impressed that contributing to Git is so easy for > newbies (never sent a patch via email before!) and that the response is so > quick. Thanks for contributing great patches and for the kind words! I am also happy that you find it easy for newbies to contribute. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 49/83] builtin/apply: move 'lock_file' global into 'struct apply_state'
On Mon, Apr 25, 2016 at 9:50 AM, Eric Sunshine wrote:
>> @@ -4515,8 +4521,6 @@ static int write_out_results(struct apply_state
>> *state, struct patch *list)
>> return errs;
>> }
>>
>> -static struct lock_file lock_file;
>
> Does the static lock_file in build_fake_ancestor() deserve the same
> sort of treatment? (I haven't traced the code enough to answer this.)
Maybe yes we could do the same thing for this static lock_file, but
this can be done later, and it could be a bit involved, so I prefer to
not touch that for now.
We are using the lock_file like this in build_fake_ancestor():
hold_lock_file_for_update(&lock, filename, LOCK_DIE_ON_ERROR);
if (write_locked_index(&result, &lock, COMMIT_LOCK))
return error("Could not write temporary index to %s", filename);
so it looks like it is safe to call build_fake_ancestor() many times
as long as it is not called by different threads.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 51/83] builtin/apply: make apply_patch() return -1 instead of die()ing
On Tue, Apr 26, 2016 at 3:20 AM, Eric Sunshine wrote:
> On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder
> wrote:
>> To libify `git apply` functionality we have to signal errors
>> to the caller instead of die()ing.
>>
>> As a first step in this direction, let's make apply_patch() return
>> -1 in case of errors instead of dying. For now its only caller
>> apply_all_patches() will exit(1) when apply_patch() return -1.
>>
>> In a later patch, apply_all_patches() will return -1 too instead of
>> exiting.
>>
>> Signed-off-by: Christian Couder
>> ---
>> diff --git a/builtin/apply.c b/builtin/apply.c
>> @@ -4522,6 +4522,14 @@ static int write_out_results(struct apply_state
>> *state, struct patch *list)
>> static int apply_patch(struct apply_state *state,
>>int fd,
>>const char *filename,
>> @@ -4564,7 +4572,7 @@ static int apply_patch(struct apply_state *state,
>> }
>>
>> if (!list && !skipped_patch)
>> - die(_("unrecognized input"));
>> + return error(_("unrecognized input"));
>>
>> if (state->whitespace_error && (state->ws_error_action ==
>> die_on_ws_error))
>> state->apply = 0;
>> @@ -4575,19 +4583,17 @@ static int apply_patch(struct apply_state *state,
>> hold_locked_index(state->lock_file, 1);
>> }
>>
>> - if (state->check_index) {
>> - if (read_cache() < 0)
>> - die(_("unable to read index file"));
>> - }
>> + if (state->check_index && read_cache() < 0)
>> + return error(_("unable to read index file"));
>>
>> if ((state->check || state->apply) &&
>> check_patch_list(state, list) < 0 &&
>> !state->apply_with_reject)
>> - exit(1);
>> + return -1;
>>
>> if (state->apply && write_out_results(state, list)) {
>> if (state->apply_with_reject)
>> - exit(1);
>> + return -1;
>> /* with --3way, we still need to write the index out */
>> return 1;
>> }
>
> Are these new 'returns' leaking 'list', 'buf', and 'fn_table' which
> otherwise get released at the end of the function?
Yeah, you are right, I will fix that. Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 53/83] builtin/apply: make find_header() return -1 instead of die()ing
On Wed, Apr 27, 2016 at 8:08 PM, Eric Sunshine wrote:
> On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder
> wrote:
>> To be compatible with the rest of the error handling in builtin/apply.c,
>> find_header() should return -1 instead of calling die().
>>
>> Unfortunately find_header() already returns -1 when no header is found,
>> so let's make it return -2 instead in this case.
>>
>> Signed-off-by: Christian Couder
>> ---
>> diff --git a/builtin/apply.c b/builtin/apply.c
>> @@ -1588,18 +1596,18 @@ static int find_header(struct apply_state *state,
>> continue;
>> if (!patch->old_name && !patch->new_name) {
>> if (!patch->def_name)
>> - die(Q_("git diff header lacks
>> filename information when removing "
>> - "%d leading pathname
>> component (line %d)",
>> - "git diff header lacks
>> filename information when removing "
>> - "%d leading pathname
>> components (line %d)",
>> - state->p_value),
>> - state->p_value, state->linenr);
>> + return error(Q_("git diff header
>> lacks filename information when removing "
>> + "%d leading pathname
>> component (line %d)",
>> + "git diff header
>> lacks filename information when removing "
>> + "%d leading pathname
>> components (line %d)",
>> + state->p_value),
>> +state->p_value,
>> state->linenr);
>> patch->old_name = xstrdup(patch->def_name);
>> patch->new_name = xstrdup(patch->def_name);
>> }
>> if (!patch->is_delete && !patch->new_name)
>> - die("git diff header lacks filename
>> information "
>> - "(line %d)", state->linenr);
>> + return error("git diff header lacks filename
>> information "
>> +"(line %d)", state->linenr);
>
> I realize that the caller in this patch currently just die()'s, and I
> haven't looked at subsequent patches yet, but is this new 'return'
> going to cause the caller to start leaking patch->old_name and
> patch->new_name which are xstrdup()'d just above?
I think it is ok because find_header() is called only from
parse_chunk() which is only called by apply_patch() and apply_patch()
calls "free_patch(patch)" when parse_chunk() returns a negative
integer.
And free_patch() frees old_name and new_name.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 53/83] builtin/apply: make find_header() return -1 instead of die()ing
On Wed, Apr 27, 2016 at 8:10 PM, Eric Sunshine wrote: > On Mon, Apr 25, 2016 at 9:18 AM, Duy Nguyen wrote: >> On Sun, Apr 24, 2016 at 8:33 PM, Christian Couder >> wrote: >>> To be compatible with the rest of the error handling in builtin/apply.c, >>> find_header() should return -1 instead of calling die(). >>> >>> Unfortunately find_header() already returns -1 when no header is found, >>> so let's make it return -2 instead in this case. >> >> I don't think this is a good way to go. Too many magic numbers. I >> don't have a better option though. Maybe returning names instead of >> numbers would help a bit. > > I suppose 'hdrsize' could signal this extra condition. For instance, > always return -1 on error, and set hdrsize to 0 for header not found > (unless 0 is a valid size?), and -1 for any other error. > > But perhaps that's getting too clever... Yeah, I don't think it would make reading the patch simpler. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 61/83] builtin/apply: libify check_apply_state()
On Mon, Apr 25, 2016 at 3:26 PM, Duy Nguyen wrote:
> On Sun, Apr 24, 2016 at 8:34 PM, Christian Couder
> wrote:
>> Signed-off-by: Christian Couder
>> ---
>> builtin/apply.c | 16 +---
>> 1 file changed, 9 insertions(+), 7 deletions(-)
>>
>> diff --git a/builtin/apply.c b/builtin/apply.c
>> index e3ee199..7576ec5 100644
>> --- a/builtin/apply.c
>> +++ b/builtin/apply.c
>> @@ -4534,17 +4534,17 @@ static int option_parse_directory(const struct
>> option *opt,
>> return 0;
>> }
>>
>> -static void check_apply_state(struct apply_state *state, int force_apply)
>> +static int check_apply_state(struct apply_state *state, int force_apply)
>> {
>> int is_not_gitdir = !startup_info->have_repository;
>>
>> if (state->apply_with_reject && state->threeway)
>> - die("--reject and --3way cannot be used together.");
>> + return error("--reject and --3way cannot be used together.");
>> if (state->cached && state->threeway)
>> - die("--cached and --3way cannot be used together.");
>> + return error("--cached and --3way cannot be used together.");
>
> Sweet opportunity to _() these messages since it clear shows in this
> patch that other messages of the same category are _()'d in this
> function. Could be done as a separate patch. But I'm also ok if you
> say "no".
I have added it to my todo list of things that should be done after this series.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 63/83] builtin/apply: make apply_all_patches() return -1 on error
On Mon, Apr 25, 2016 at 3:30 PM, Duy Nguyen wrote:
> On Sun, Apr 24, 2016 at 8:34 PM, Christian Couder
> wrote:
>> if (state->update_index) {
>> if (write_locked_index(&the_index, state->lock_file,
>> COMMIT_LOCK))
>> - die(_("Unable to write new index file"));
>> + return error(_("Unable to write new index file"));
>> }
>>
>> return !!errs;
>> @@ -4698,5 +4698,8 @@ int cmd_apply(int argc, const char **argv, const char
>> *prefix)
>> if (check_apply_state(&state, force_apply))
>> exit(1);
>>
>> - return apply_all_patches(&state, argc, argv, options);
>> + if (apply_all_patches(&state, argc, argv, options))
>> + exit(1);
>
> Abusing exit() too much? This is cmd_apply(). A return should be
> enough and you can do the !! trick that is used in
> apply_all_patches(), shown just a little bit above.
Ok, I will use:
return !!apply_all_patches(&state, argc, argv, options);
Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 65/83] builtin/apply: make gitdiff_verify_name() return -1 on error
On Mon, Apr 25, 2016 at 3:36 PM, Duy Nguyen wrote:
> On Sun, Apr 24, 2016 at 8:34 PM, Christian Couder
> wrote:
>> Signed-off-by: Christian Couder
>> ---
>> builtin/apply.c | 52 ++--
>> 1 file changed, 30 insertions(+), 22 deletions(-)
>>
>> diff --git a/builtin/apply.c b/builtin/apply.c
>> index 6b8ba2a..268356b 100644
>> --- a/builtin/apply.c
>> +++ b/builtin/apply.c
>> @@ -812,7 +812,7 @@ static int gitdiff_hdrend(struct apply_state *state,
>> const char *line,
>> struct patch *patch)
>> {
>> - return -1;
>> + return 1;
>> }
>
> These are in a function group that will be called as p->fn() in
> parse_git_header(). Is it possible to isolate them in a separate
> patch? This patch can follow after, which covers only
> gitdiff_verify_name() returning -1 and its call sites.
Yeah, I had planned to do something like that when developing the
series but I forgot.
Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 54/83] builtin/apply: make parse_chunk() return a negative integer on error
On Sun, May 1, 2016 at 9:04 PM, Eric Sunshine wrote:
> On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder
> wrote:
>> This negative number can be -2 if no patch header has been found,
>> otherwise it is -1.
>>
>> As parse_chunk() is called only by apply_patch() which already
>> returns -1 when an error happened, let's make it return -1 when
>> parse_chunk() returns -1.
>>
>> Signed-off-by: Christian Couder
>> ---
>> diff --git a/builtin/apply.c b/builtin/apply.c
>> @@ -4566,6 +4567,8 @@ static int apply_patch(struct apply_state *state,
>> nr = parse_chunk(state, buf.buf + offset, buf.len - offset,
>> patch);
>> if (nr < 0) {
>> free_patch(patch);
>> + if (nr == -1)
>> + return -1;
>
> Same comment as 51/83 about this leaking 'list', 'buf', and 'fn_table'.
Ok, thanks!
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 59/83] builtin/apply: move init_apply_state() to apply.c
On Sun, May 1, 2016 at 9:37 PM, Eric Sunshine wrote: > On Sun, Apr 24, 2016 at 9:33 AM, Christian Couder > wrote: >> Signed-off-by: Christian Couder >> --- >> diff --git a/apply.c b/apply.c >> @@ -0,0 +1,80 @@ >> +#include "cache.h" >> +#include "apply.h" >> + >> + >> + > > Too many blank lines? Ok, I will remove 2 of them. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] quote: fix broken sq_quote_buf() related comment
Since 77d604c (Enhanced sq_quote(), 10 Oct 2005), the comment at the beginning of quote.c is broken. Let's fix it. Signed-off-by: Christian Couder --- The only change in this v2 is the added Signed-off-by ;-) Sorry for the spam. quote.c | 1 + 1 file changed, 1 insertion(+) diff --git a/quote.c b/quote.c index 7920e18..890885a 100644 --- a/quote.c +++ b/quote.c @@ -7,6 +7,7 @@ int quote_path_fully = 1; /* Help to copy the thing properly quoted for the shell safety. * any single quote is replaced with '\'', any exclamation point * is replaced with '\!', and the whole thing is enclosed in a + * single quote pair. * * E.g. * original sq_quote result -- 2.6.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] quote: move comment before sq_quote_buf()
A big comment at the beginning of quote.c is really
related to sq_quote_buf(), so let's move it in front
of this function.
Signed-off-by: Christian Couder
---
quote.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/quote.c b/quote.c
index 890885a..fe884d2 100644
--- a/quote.c
+++ b/quote.c
@@ -4,6 +4,11 @@
int quote_path_fully = 1;
+static inline int need_bs_quote(char c)
+{
+ return (c == '\'' || c == '!');
+}
+
/* Help to copy the thing properly quoted for the shell safety.
* any single quote is replaced with '\'', any exclamation point
* is replaced with '\!', and the whole thing is enclosed in a
@@ -16,11 +21,6 @@ int quote_path_fully = 1;
* a'b ==> a'\''b==> 'a'\''b'
* a!b ==> a'\!'b==> 'a'\!'b'
*/
-static inline int need_bs_quote(char c)
-{
- return (c == '\'' || c == '!');
-}
-
void sq_quote_buf(struct strbuf *dst, const char *src)
{
char *to_free = NULL;
--
2.6.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Draft of Git Rev News edition 8
Hi, A draft of Git Rev News edition 8 is available here: https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-8.md Everyone is welcome to contribute in any section either by editing the above page on GitHub and sending a pull request, or by commenting on this GitHub issue: https://github.com/git/git.github.io/issues/100 You can also reply to this email. I tried to cc everyone who appears in this edition but maybe I missed some people, sorry about that. Thomas, Nicola and myself plan to publish this edition on Wednesday the 14th of October. Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Draft of Git Rev News edition 8
On Sun, Oct 11, 2015 at 1:45 AM, Eric Sunshine wrote: > On Sat, Oct 10, 2015 at 7:36 PM, Christian Couder > wrote: >> A draft of Git Rev News edition 8 is available here: >> https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-8.md > > Does Karsten's comprehensive post[1] about nanosecond-related racy > detection problems merit mention? Yeah, probably, would you like to write something which can be very short, about it. We would also gladly accept something about Git version 2.6 (hint, hint :-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] Git Rev News edition 8
Hi everyone, I'm happy announce that the 8th edition of Git Rev News is now published: https://git.github.io/rev_news/2015/10/14/edition-8/ Thanks a lot to all the contributors! Enjoy, Christian, Thomas and Nicola. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/4] stripspace: Implement --count-lines option
On Mon, Oct 19, 2015 at 3:46 PM, Tobias Klauser wrote: > On 2015-10-18 at 19:18:53 +0200, Junio C Hamano wrote: >> Eric Sunshine writes: >> >> > Is there any application beyond git-rebase--interactive where a >> > --count-lines options is expected to be useful? It's not obvious from >> > the commit message that this change is necessarily a win for later >> > porting of git-rebase--interactive to C since the amount of extra code >> > and support material added by this patch probably outweighs the amount >> > of code a C version of git-rebase--interactive would need to count the >> > lines itself. >> > >> > Stated differently, are the two or three instances of piping through >> > 'wc' in git-rebase--interactive sufficient justification for >> > introducing extra complexity into git-stripspace and its documentation >> > and tests? >> >> Interesting thought. When somebody rewrites "rebase -i" in C, >> nobody needs to count lines in "stripspace" output. The rewritten >> "rebase -i" would internally run strbuf_stripspace() and the question >> becomes what is the best way to let that code find out how many lines >> the result contains. >> >> When viewed from that angle, I agree that "stripspace --count" does >> not add anything to further the goal of helping "rebase -i" to move >> to C. Adding strbuf_count_lines() that counts the number of lines >> in the given strbuf (if there is no such helper yet; I didn't check), >> though. > > I check before implementing this series and didn't find any helper. I > also didn't find any other uses of line counting in the code. This shows that implementing "git stripspace --count-lines" could indirectly help porting "git rebase -i" to C as you could implement strbuf_count_lines() for the former and it could then be reused in the latter. > After considering your and Eric's reply, I'll drop these patches for > now and only resubmit patches 1/4 and 2/4 for v3 (also see my reply to > Eric). It would be sad in my opinion. >> >> +test_expect_success '--count-lines with newline only' ' >> >> + printf "0\n" >expect && >> >> + printf "\n" | git stripspace --count-lines >actual && >> >> + test_cmp expect actual >> >> +' >> > >> > What is the expected behavior when the input is an empty file, a file >> > with content but no newline, a file with one or more lines but lacking >> > a newline on the final line? Should these cases be tested, as well? >> >> Good point here, too. If we were to add strbuf_count_lines() >> helper, whoever adds that function needs to take a possible >> incomplete line at the end into account. > > Yes, makes more sense like this (even though it doesn't correspond to > what 'wc -l' does). Tests for "git stripspace --count-lines" would test strbuf_count_lines() which would also help when porting git rebase -i to C. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Watchman/inotify support and other ways to speed up git status
Hi everyone, I am starting to investigate ways to speed up git status and other git commands for Booking.com (thanks to AEvar) and I'd be happy to discuss the current status or be pointed to relevant documentation or mailing list threads. >From the threads below ([0], [1], [2], [3], [4], [5], [6], [7], [8]) I understand that the status is roughly the following: - instead of working on inotify support it's better to work on using a cross platform tool like Watchman - instead of working on Watchman support it is better to work first on caching information in the index - git update-index --untracked-cache has been developed by Duy and others and merged to master in May 2015 to cache untracked status in the index; it is still considered experimental - git index-helper has been worked on by Duy but its status is not clear (at least to me) Is that correct? What are the possible/planned next steps in this area? improving --untracked-cache? git index-helper? watchman support? Thanks, Christian. [0] March 8 2015: [PATCH 00/24] nd/untracked-cache updates http://thread.gmane.org/gmane.comp.version-control.git/265053/ [1] November 11 2014: [RFC] On watchman support http://thread.gmane.org/gmane.comp.version-control.git/259399/ [2] October 27 2014:[PATCH 00/19] Untracked cache to speed up "git status" http://thread.gmane.org/gmane.comp.version-control.git/258766 [3] July 28 2014: [PATCH v3 0/9] Speed up cache loading time http://thread.gmane.org/gmane.comp.version-control.git/254314/ [4] May 7 2014: [PATCH 00/20] Untracked cache to speed up "git status" http://thread.gmane.org/gmane.comp.version-control.git/248306 [5] May 2 2014: Watchman support for git http://thread.gmane.org/gmane.comp.version-control.git/248004/ [6] March 10 2014: http://git.661346.n2.nabble.com/question-about-Facebook-makes-Mercurial-faster-than-Git-tt7605273.html#a7605280 [7] January 29 2014: http://git.661346.n2.nabble.com/inotify-support-nearly-there-tt7602739.html [8] January 12 2014: http://git.661346.n2.nabble.com/PATCH-0-6-inotify-support-tt7601877.html#a7603955 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Make "git checkout" automatically update submodules?
On Fri, Oct 23, 2015 at 5:46 AM, Kannan Goundan wrote:
> Junio C Hamano pobox.com> writes:
>
>> We are unfortunately not set up to handle money well. For a
>> background explanation, please go read [*1*], which I wrote my take
>> on "money" some time ago. Note that it is an explanation and not a
>> justification. It explains why we are not set up to handle money
>> well and what the issues around money that are troublesome for the
>> project are. It does not mean to say that it is a good thing that
>> it is hard to buy feature with money from our project [*2*].
>
> I think the way I described it ("sponsoring a feature") doesn't really
> reflect how I was imagining it. In my head, it looked like this:
>
> 1. Figure out whether the Git community and maintainers seem ok with the
> overall feature idea. If not, give up.
> 2. Come up with a plan for the UI/UX; see if the Git community and
> maintainers seem ok with it. If not, iterate or give up.
> 3. Implement it, then go through the regular process of getting it merged
> upstream. If it doesn't go well, might have to iterate or give up.
>
> I could try doing that myself, but someone familiar with the Git
> codebase/community/history would be better at it (and probably be easier for
> you guys to work with :-)
>
> I guess I'm just wondering if there are people who meet those qualifications
> and are interested in going through those steps for pay. Or maybe there's a
> company that does this, like the old Cygnus Solutions?
Well I am starting to do that for Booking.com. Not sure if it will
also be possible for me to work for you as I also work on IPFS
(http://ipfs.io) for the company that develops it, but we can discuss
it privately.
There was David Kastrup (dak at gnu.org) who previously said he could
be interested in such jobs. We wrote a very short article about it in
the first edition of Git Rev News last March:
http://git.github.io/rev_news/2015/03/25/edition-1/
We also wrote a very short article "Job Offer" article about
Booking.com looking for Git developers in Git Rev News in the third
edition last May:
http://git.github.io/rev_news/2015/05/13/edition-3/
so if you want we can write a similar "Job Offer" article in the next
Git Rev News edition.
You can even propose such an article yourself by editing the draft of
the next edition here:
https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-9.md
and then creating a pull request.
> In particular, I don't expect anything to change about the project's
> development process.
>
> (This part is not relevant to the Git project, but I understand that it's
> hard for anyone to guarantee a feature will make it into an open source
> project. I imagine these kinds of contracts are set up so that you're
> primarily paying for the effort, not the outcome. If it ends up not working
> out, you don't get your money back.)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Watchman/inotify support and other ways to speed up git status
On Wed, Oct 28, 2015 at 12:54 AM, David Turner wrote: > > On Thu, 2015-10-22 at 07:59 +0200, Christian Couder wrote: >> Hi everyone, >> >> I am starting to investigate ways to speed up git status and other git >> commands for Booking.com (thanks to AEvar) and I'd be happy to discuss >> the current status or be pointed to relevant documentation or mailing >> list threads. >> >> From the threads below ([0], [1], [2], [3], [4], [5], [6], [7], [8]) I >> understand that the status is roughly the following: >> >> - instead of working on inotify support it's better to work on using a >> cross platform tool like Watchman >> >> - instead of working on Watchman support it is better to work first on >> caching information in the index >> >> - git update-index --untracked-cache has been developed by Duy and >> others and merged to master in May 2015 to cache untracked status in >> the index; it is still considered experimental >> >> - git index-helper has been worked on by Duy but its status is not >> clear (at least to me) >> >> Is that correct? >> What are the possible/planned next steps in this area? improving > > We're using Watchman at Twitter. A week or two ago posted a dump of our > code to github, but I would advise waiting a day or two to use it, as > I'm about to pull a large number of bugfixes into it (I'll update this > thread and provide a link once I do so). Great, I will have a look at it then! > It's good, but it's not great. One major problem is a bug on OS X[1] > that causes missed updates. Another is that wide changes end up being > quite inefficient when querying watchman. This means that we do some > hackery to manually update the fs_cache during various large git > operations. > > I agree that in general it would be better to store or all some of this > information in the index, and the untracked-cache is a good step on > that. But with it enabled and watchman disabled, there still appears to > be 1 lstat per file (plus one stat per dir). The stats per-directory > alone are a large issue for Twitter because we have a relatively deep > and bushy directory structure (an average dir has about 3 or 4 entries > in it). As a result, git status with watchman is almost twice as fast > as with the untracked cache (on my particular machine). Thanks for this detailled description. > [1] https://github.com/facebook/watchman/issues/172 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Watchman/inotify support and other ways to speed up git status
On Tue, Nov 3, 2015 at 6:45 AM, Duy Nguyen wrote: > On Mon, Nov 2, 2015 at 9:56 PM, David Turner wrote: >> On Thu, 2015-10-29 at 09:10 +0100, Christian Couder wrote: >>> > We're using Watchman at Twitter. A week or two ago posted a dump of our >>> > code to github, but I would advise waiting a day or two to use it, as >>> > I'm about to pull a large number of bugfixes into it (I'll update this >>> > thread and provide a link once I do so). >>> >>> Great, I will have a look at it then! >> >> Here's the most recent version: >> >> https://github.com/dturner-tw/git/tree/dturner/watchman > > Christian, the index-helper/watchman series are posted because you > showed interest in this area. I'm not rerolling to address David's > comments on the series for now. Ok no problem. Thanks a lot to you and David for posting your rebased series! > Take your time evaluate the two > approaches, then you can pick one (and let me know if you want me to > hand my series over, very glad to do so). Yeah, I will do that, thanks again! -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Draft of Git Rev News edition 9
Hi, A draft of Git Rev News edition 9 is available here: https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-9.md Everyone is welcome to contribute in any section either by editing the above page on GitHub and sending a pull request, or by commenting on this GitHub issue: https://github.com/git/git.github.io/issues/108 You can also reply to this email. I tried to cc everyone who appears in this edition but maybe I missed some people, sorry about that. Thomas, Nicola and myself plan to publish this edition on Wednesday the 11th of November. Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] Use watchman to reduce index refresh time
On Tue, Nov 3, 2015 at 10:21 AM, Duy Nguyen wrote: > On Mon, Nov 2, 2015 at 8:23 PM, Duy Nguyen wrote: >> On Mon, Nov 2, 2015 at 3:54 PM, Paolo Ciarrocchi >> wrote: >>> On Sun, Nov 1, 2015 at 2:55 PM, Nguyễn Thái Ngọc Duy >>> wrote: >>> >>> Hi Duy, >>> This series builds on top of the index-helper series I just sent and uses watchman to keep track of file changes in order to avoid lstat() at refresh time. The series can also be found at [1] When I started this work, watchman did not support Windows yet. It does now, even if still experimental [2]. So Windows people, please try it out if you have time. To put all pieces so far together, we have split-index to reduce index write time, untracked cache to reduce I/O as well as computation for .gitignore, index-helper for index read time and this series for lstat() at refresh time. The remaining piece is killing lstat() from untracked cache, but right now it's just some idea and incomplete code. >>> >>> Did you manage to measure the speedup introduced by this series? >> >> It was from last year. I may have measured it but because I didn't >> save it in the commit message, it was lost anyway. Installing watchman >> and measuring with webkit.git soon.. > > Test repo: webkit.git with 104665 tracked files, 5615 untracked, > 3517 dirs. Best numbers out of a few tries. This is best case > scenario. Normal usage could have worse numbers. Thank you for the tests! I tried to replicate your results on webkit.git with 184672 tracked files, 5631 untracked, 9330 dirs. Also best numbers out of a few tries. > There is something strange about the "-uno" measurements. I don't > think watchman+untracked cache can beat -uno.. Maybe I did something > wrong. > > 0m0.383s index v2 > 0m0.351s index v4 > 0m0.352s v2 split-index > 0m0.309s v2 split index-helper > 0m0.159s v2 split helper untracked-cache > 0m0.123s v2 split helper "status -uno" > 0m0.098s v2 split helper untracked watchman > 0m0.071s v2 split helper watchman "status -uno" I got the following results from "time git status ...": 0m0.774s 0m0.799s split 0m0.766s split helper 0m0.335s split helper untracked 0m0.232s split helper untracked -uno 0m0.284s split helper untracked watchman 0m0.188s split helper untracked watchman -uno Using David's series I get worse results than all of the above but I guess it's because his series is based on an ancient git version (v2.0.0-rc0). > Note, the watchman series needs > s/free_watchman_shm/release_watchman_shm/ (I didn't do a good job > of testing after rebase). And there's a small bug in index-helper > --detach code writing incorrect PID.. I did the s/free_watchman_shm/release_watchman_shm/ change, but I didn't notice the index-helper bug. Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] Git Rev News edition 9
Hi everyone, I'm happy announce that the 9th edition of Git Rev News is now published: http://git.github.io/rev_news/2015/11/11/edition-9/ Thanks a lot to all the contributors, especially Matthieu! Enjoy, Christian, Thomas and Nicola. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] Use watchman to reduce index refresh time
On Tue, Nov 10, 2015 at 10:04 PM, David Turner wrote: > On Mon, 2015-11-09 at 21:06 +0100, Christian Couder wrote: >> Using David's series I get worse results than all of the above but I >> guess it's because his series is based on an ancient git version >> (v2.0.0-rc0). > > My more-recent series is on top of 2.4, but (for webkit): > mine: 0m0.206s > duy's: 0m0.107s I tried using your more recent series and I get basically no change compared to the current git (without using untracked cache) on Webkit with around 5600 untracked files. Maybe I am doing something wrong. I compiled watchman and installed it. It is in /usr/local/bin/watchman but I am not sure it is actually being used though I have configured "core.usewatchman" to "true". > However, I'm getting occasional index-helper segfaults due to > istate->last_update being NULL. (I'm using Duy's index-helper branch > from his github + this patchset + a function signature fix due to a > newer version of libwatchman). I haven't looked into why this is -- > maybe accidentally mixing git versions while testing? > > Also, after messing around for a while (on Duy's branch), I ended up in > a state where git status would take ~2.5s every time. The index helper > was alive but evidently not working right. Killing and restarting it > worked. Actually, I think I can repro this more easily: "git rm > Changelog" seems to put the index-helper into this state. Thanks for this. I have also seen strange things happening with the index-helper. When it is working I found that it provides around 10% improvement on git rebase time. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Documentation/git-update-index: add missing opts to synopsys
Untracked cache related options should appear in the synopsis. Signed-off-by: Christian Couder --- Documentation/git-update-index.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt index 1a296bc..3df9c26 100644 --- a/Documentation/git-update-index.txt +++ b/Documentation/git-update-index.txt @@ -17,6 +17,7 @@ SYNOPSIS [--[no-]assume-unchanged] [--[no-]skip-worktree] [--ignore-submodules] +[--[no-|force-]untracked-cache] [--really-refresh] [--unresolve] [--again | -g] [--info-only] [--index-info] [-z] [--stdin] [--index-version ] -- 2.6.2.412.gf783589.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH] config: add core.trustmtime
When we know that mtime is fully supported by the environment, we
don't want any slow down because we used --untracked-cache rather
than --force-untracked-cache, and we don't want untracked cache to
stop working because we updated a kernel.
Also when we know that mtime is not supported by the environment,
for example because the repo is shared over a network file system,
then we might want 'git update-index --untracked-cache' to fail
immediately instead of it testing if it works (because it might
work on some systems using the repo over the network file system
but not others).
Signed-off-by: Christian Couder
---
At Booking.com we know that mtime works everywhere and we don't
want the untracked cache to stop working when a kernel is upgraded
or when the repo is copied to a machine with a different kernel.
I will add tests later if people are ok with this.
Documentation/config.txt | 8
Documentation/git-update-index.txt | 6 +-
builtin/update-index.c | 6 +-
cache.h| 1 +
config.c | 7 +++
contrib/completion/git-completion.bash | 1 +
dir.c | 2 +-
environment.c | 1 +
8 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index b4b0194..186ad17 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -308,6 +308,14 @@ core.trustctime::
crawlers and some backup systems).
See linkgit:git-update-index[1]. True by default.
+core.trustmtime::
+ If unset or set to 'default' or 'check', Git will test if
+ mtime is working properly before using features that need a
+ working mtime. If false, Git will refuse to use such
+ features. If set to true, Git will blindly use such features
+ without testing if they work.
+ See linkgit:git-update-index[1].
+
core.checkStat::
Determines which stat fields to match between the index
and work tree. The user can set this to 'default' or
diff --git a/Documentation/git-update-index.txt
b/Documentation/git-update-index.txt
index 1a296bc..8b4c5af 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -182,7 +182,9 @@ may not support it yet.
For safety, `--untracked-cache` performs tests on the working
directory to make sure untracked cache can be used. These
tests can take a few seconds. `--force-untracked-cache` can be
- used to skip the tests.
+ used to skip the tests. If you always want to skip those
+ tests, you can set the `core.trustmtime` configuration
+ variable to 'true', (see linkgit:git-config[1]).
\--::
Do not interpret any more arguments as options.
@@ -398,6 +400,8 @@ It can be useful when the inode change time is regularly
modified by
something outside Git (file system crawlers and backup systems use
ctime for marking files processed) (see linkgit:git-config[1]).
+Untracked cache needs a fully working mtime, so it will look at
+`core.trustmtime` configuration variable (see linkgit:git-config[1]).
SEE ALSO
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 7431938..c64439f 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1107,9 +1107,13 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
if (untracked_cache > 0) {
struct untracked_cache *uc;
+ if (trust_mtime == 0) {
+ fprintf_ln(stderr,_("core.trustmtime is set to false"));
+ return 1;
+ }
if (untracked_cache < 2) {
setup_work_tree();
- if (!test_if_untracked_cache_is_supported())
+ if (trust_mtime != 1 &&
!test_if_untracked_cache_is_supported())
return 1;
}
if (!the_index.untracked) {
diff --git a/cache.h b/cache.h
index 736abc0..69a6197 100644
--- a/cache.h
+++ b/cache.h
@@ -601,6 +601,7 @@ extern void set_alternate_index_output(const char *);
/* Environment bits from configuration mechanism */
extern int trust_executable_bit;
extern int trust_ctime;
+extern int trust_mtime;
extern int check_stat;
extern int quote_path_fully;
extern int has_symlinks;
diff --git a/config.c b/config.c
index 248a21a..d720b1f 100644
--- a/config.c
+++ b/config.c
@@ -691,6 +691,13 @@ static int git_default_core_config(const char *var, const
char *value)
trust_ctime = git_config_bool(var, value);
return 0;
}
+ if (!strcmp(var, "core.trustmtime")) {
+ if (!strcasecmp(value, "default") || !strcasecmp(value,
"check"))
+
[PATCH v2] Documentation/git-update-index: add missing opts to synopsys
Untracked cache and split index related options should appear in the 'SYNOPSIS' section. These options are already documented in the 'OPTIONS' section. Signed-off-by: Christian Couder --- Soon after sending the first version I realized that the split index options were not in the synopsys either... Documentation/git-update-index.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt index 1a296bc..f4e5a85 100644 --- a/Documentation/git-update-index.txt +++ b/Documentation/git-update-index.txt @@ -17,6 +17,8 @@ SYNOPSIS [--[no-]assume-unchanged] [--[no-]skip-worktree] [--ignore-submodules] +[--[no-]split-index] +[--[no-|force-]untracked-cache] [--really-refresh] [--unresolve] [--again | -g] [--info-only] [--index-info] [-z] [--stdin] [--index-version ] -- 2.6.3.380.g494b52d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation/git-update-index: add missing opts to synopsys
On Tue, Nov 24, 2015 at 9:46 PM, Jeff King wrote: > On Tue, Nov 24, 2015 at 12:55:07PM +0100, Christian Couder wrote: > >> Untracked cache related options should appear in the synopsis. >> >> Signed-off-by: Christian Couder >> --- >> Documentation/git-update-index.txt | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/Documentation/git-update-index.txt >> b/Documentation/git-update-index.txt >> index 1a296bc..3df9c26 100644 >> --- a/Documentation/git-update-index.txt >> +++ b/Documentation/git-update-index.txt >> @@ -17,6 +17,7 @@ SYNOPSIS >>[--[no-]assume-unchanged] >>[--[no-]skip-worktree] >>[--ignore-submodules] >> + [--[no-|force-]untracked-cache] > > Thanks, and it looks like they are already documented in the OPTIONS > section. Yeah, I just added this information into the commit message and sent a v2 that also add split index related options to the synopsis. Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] Documentation/git-update-index: add missing opts to synopsis
Untracked cache and split index related options should appear in the 'SYNOPSIS' section. These options are already documented in the 'OPTIONS' section. Signed-off-by: Christian Couder --- The only change compared to v2 is s/synopsys/synopsis/ thanks to Eric. Documentation/git-update-index.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt index 1a296bc..f4e5a85 100644 --- a/Documentation/git-update-index.txt +++ b/Documentation/git-update-index.txt @@ -17,6 +17,8 @@ SYNOPSIS [--[no-]assume-unchanged] [--[no-]skip-worktree] [--ignore-submodules] +[--[no-]split-index] +[--[no-|force-]untracked-cache] [--really-refresh] [--unresolve] [--again | -g] [--info-only] [--index-info] [-z] [--stdin] [--index-version ] -- 2.6.3.380.g494b52d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] Documentation/git-update-index: add missing opts to synopsys
On Wed, Nov 25, 2015 at 10:03 AM, Jeff King wrote: > On Wed, Nov 25, 2015 at 07:53:12AM +0100, Christian Couder wrote: > >> Untracked cache and split index related options should appear >> in the 'SYNOPSIS' section. >> >> These options are already documented in the 'OPTIONS' section. >> >> Signed-off-by: Christian Couder >> --- >> Soon after sending the first version I realized that the split index >> options were not in the synopsys either... > > Sorry, too late. I merged v1 as part of yesterday's cycle, as it seemed > obviously correct (that's what I get... :) ). > > Can you please send the change as a patch on top? Ok will do. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4] Documentation/git-update-index: add missing opts to synopsis
Split index related options should appear in the 'SYNOPSIS' section. These options are already documented in the 'OPTIONS' section. Signed-off-by: Christian Couder --- This v4 contains only the split-index options and applies on top of the new master that already contains the untracked-cache options. Documentation/git-update-index.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt index 3df9c26..f4e5a85 100644 --- a/Documentation/git-update-index.txt +++ b/Documentation/git-update-index.txt @@ -17,6 +17,7 @@ SYNOPSIS [--[no-]assume-unchanged] [--[no-]skip-worktree] [--ignore-submodules] +[--[no-]split-index] [--[no-|force-]untracked-cache] [--really-refresh] [--unresolve] [--again | -g] [--info-only] [--index-info] -- 2.6.3.380.g494b52d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] config: add core.trustmtime
Hi Johannes,
On Wed, Nov 25, 2015 at 11:25 AM, Johannes Schindelin
wrote:
> Hi Christian,
>
> On Wed, 25 Nov 2015, Christian Couder wrote:
>
>> diff --git a/config.c b/config.c
>> index 248a21a..d720b1f 100644
>> --- a/config.c
>> +++ b/config.c
>> @@ -691,6 +691,13 @@ static int git_default_core_config(const char *var,
>> const char *value)
>> trust_ctime = git_config_bool(var, value);
>> return 0;
>> }
>> + if (!strcmp(var, "core.trustmtime")) {
>> + if (!strcasecmp(value, "default") || !strcasecmp(value,
>> "check"))
>> + trust_mtime = -1;
>> + else
>> + trust_mtime = git_config_maybe_bool(var, value);
>
> If `core.trustmtime` is set to `OMG, never!`, `git_config_maybe_bool()`
> will set trust_mtime to -1, i.e. the exact same value as if you had set
> the variable to `default` or `check`...
Yeah, I think returning -1 is the safe thing to do in this case.
> Maybe you want to be a bit stricter here and either test the return value
> of `git_config_maybe_bool()` for -1 (and warn in that case), or just
> delete the `maybe_`?
I thought that if we ever add more options in the future then people
might be annoyed by older git versions warning about the new options.
But I am ok to add a warning.
Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] config: add core.trustmtime
Hi Duy, On Wed, Nov 25, 2015 at 8:51 PM, Duy Nguyen wrote: > On Wed, Nov 25, 2015 at 10:00 AM, Ævar Arnfjörð Bjarmason > wrote: >> On Wed, Nov 25, 2015 at 7:35 AM, Christian Couder >> wrote: >>> At Booking.com we know that mtime works everywhere and we don't >>> want the untracked cache to stop working when a kernel is upgraded >>> or when the repo is copied to a machine with a different kernel. >>> I will add tests later if people are ok with this. >> >> I bit more info: I rolled Git out internally with this patch: >> https://github.com/avar/git/commit/c63f7c12c2664631961add7cf3da901b0b6aa2f2 >> >> The --untracked-cache feature hardcodes the equivalent of: >> >> pwd; uname --kernel-name --kernel-release --kernel-version >> >> Into the index. If any of those change it prints out the "cache is >> disabled" warning. >> >> This patch will make it stop being so afraid of itself to the point of >> disabling itself on minor kernel upgrades :) > > The problem is, there's no way to teach git to know it's a "minor" > upgrade.. but if there is a config key to say "don't be paranoid, I > know what I'm doing", then we can skip that check, or just warn > instead of disabling the cache. Yeah, in my patch if core.trustmtime is set to true or false the check is skipped. I am wondering why you didn't make it by default run the mtime checks when a kernel change is detected. Maybe that would be better than disabling itself. >> A few other issues with this feature I've noticed: >> >> * There's no way to just enable it globally via the config. Makes it >> a bit of a hassle to use it. I wanted to have a config option to >> enable it via the config, how about "index.untracked_cache = true" for >> the config variable name? > > If you haven't noticed, all these experimental features have no real > UI (update-index is plumbing). I have been waiting for someone like > you to start using it and figure out the best UI (then implement it) > ;) Ok, we are happy to do that (including implementing it) :-) I will take a look at something like index.untracked_cache. It will probably also be a tristate like this: - true: always enable it; die if core.trustmtime is false otherwise warn if it is not true - default/unset: same as current behavior - false: die if it is enabled or when trying to enable to it >> * Doing "cd /tmp: git --git-dir=/git/somewhere/else/.git update-index >> --untracked-cache" doesn't work how I'd expect. It hardcodes "/tmp" as >> the directory that "works" into the index, so if you use the working >> tree you'll never use the untracked cache. I spotted this because I >> carry out a bunch of git maintenance commands with --git-dir instead >> of cd-ing to the relevant directories. This works for most other >> things in git, is it a bug that it doesn't work here? > > It needs the current directory at --untrack-cache time to test if the > directory satisfies the requirements. So either you cd to that > worktree, or you have to specify --worktree as well. Or am I missing > something? Maybe it could print out a message saying "Testing mtime in directory $(pwd)" and if that works then "Untracked cache is enabled for $(pwd)". That would make it clear that it will not work in other directories. Also maybe the mtime checks could be run when a directory change is detected. >> * If you "ctrl+c" git update-index --untracked-cache at an >> inopportune time you'll end up with a mtime-test-XX directory in >> your working tree. Perhaps this tempdir should be created in the .git >> directory instead? > > No because in theory .git could be on a separate file system with > different semantics. But we should probably clean those files at ^C. Ok, I will have a look at cleaning the files at ^C. >> * Maybe we should have a --test-untracked-cache option, so you can >> run the tests without enabling it. > > I'd say patches welcome. Ok, I wll have a look at that too. >> Aside from the slight hassle of enabling this and keeping it enabled >> this feature is great. It's sped up "git status" across the board by >> about 40%. Slightly less than that on faster spinning disks, slightly >> more than that on slower ones. > > I'm still waiting for the day when watchman support gets merged and > maybe poke Facebook guys to compare performance with Mercurial :) > Well, we are probably still behind Mercurial on that day. Yeah, it could be interesting to compare performance with Mercurial as we move forward :-) > Also, there's still work to be done. Right now it's optimized for > whole-tree "git status", Doing "git status -- abc" will not benefit > from untracked cache, similarly "git add" with pathspec.. Thanks for these details. Yeah, it might be interesting to look at "git add" too. Best, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 0/8] Untracked cache improvements
Following the discussions on the "config: add core.trustmtime" patch I previously sent, here is a patch series that tries to improve the untracked cache feature. This patch series implements core.untrackedCache instead of core.trustmtime. core.untrackedCache is more complex because basically when it's set to true git should always try to use the untracked cache, and when set to false git should never use it. Patchs 1/8 and 2/8 add some features that are missing. Patchs 3/8, 4/8 and 5/8 are some refactoring to prepare for patch 6/8 which implements core.untrackedCache. Up to patch 6/8 backward compatibility is preserved. Patchs 7/8 and 8/8 are trying to improve usability by making the untracked cache cli and config options more in line with other git cli and config options, but this sacrifies some backward compatibility. Christian Couder (8): update-index: add untracked cache notifications update-index: add --test-untracked-cache update-index: move 'uc' var declaration dir: add add_untracked_cache() dir: add remove_untracked_cache() config: add core.untrackedCache update-index: prevent --untracked-cache from performing tests update-index: make core.untrackedCache a bool Documentation/config.txt | 10 + Documentation/git-update-index.txt | 30 +++--- builtin/update-index.c | 39 -- cache.h| 1 + config.c | 4 contrib/completion/git-completion.bash | 1 + dir.c | 22 ++- dir.h | 2 ++ environment.c | 1 + t/t7063-status-untracked-cache.sh | 2 +- wt-status.c| 9 11 files changed, 90 insertions(+), 31 deletions(-) -- 2.6.3.391.g95a3a5c -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 5/8] dir: add remove_untracked_cache()
This new function will be used in a later patch.
Signed-off-by: Christian Couder
---
builtin/update-index.c | 3 +--
dir.c | 6 ++
dir.h | 1 +
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/builtin/update-index.c b/builtin/update-index.c
index ec67d14..2cbaee0 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1117,8 +1117,7 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
add_untracked_cache();
fprintf(stderr, _("Untracked cache enabled for '%s'\n"),
get_git_work_tree());
} else if (!untracked_cache && the_index.untracked) {
- the_index.untracked = NULL;
- the_index.cache_changed |= UNTRACKED_CHANGED;
+ remove_untracked_cache();
fprintf(stderr, _("Untracked disabled\n"));
}
diff --git a/dir.c b/dir.c
index 0f7e293..ffc0286 100644
--- a/dir.c
+++ b/dir.c
@@ -1952,6 +1952,12 @@ void add_untracked_cache(void)
the_index.cache_changed |= UNTRACKED_CHANGED;
}
+void remove_untracked_cache(void)
+{
+ the_index.untracked = NULL;
+ the_index.cache_changed |= UNTRACKED_CHANGED;
+}
+
static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct
*dir,
int base_len,
const struct pathspec
*pathspec)
diff --git a/dir.h b/dir.h
index ee94c76..3e5114d 100644
--- a/dir.h
+++ b/dir.h
@@ -309,4 +309,5 @@ struct untracked_cache *read_untracked_extension(const void
*data, unsigned long
void write_untracked_extension(struct strbuf *out, struct untracked_cache
*untracked);
void add_untracked_ident(struct untracked_cache *);
void add_untracked_cache(void);
+void remove_untracked_cache(void);
#endif
--
2.6.3.391.g95a3a5c
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 2/8] update-index: add --test-untracked-cache
It is nice to just be able to test if untracked cache is
supported without enabling it.
Signed-off-by: Christian Couder
---
Documentation/git-update-index.txt | 9 -
builtin/update-index.c | 8 ++--
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-update-index.txt
b/Documentation/git-update-index.txt
index 3df9c26..0ff7396 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -17,7 +17,7 @@ SYNOPSIS
[--[no-]assume-unchanged]
[--[no-]skip-worktree]
[--ignore-submodules]
-[--[no-|force-]untracked-cache]
+[--[no-|test-|force-]untracked-cache]
[--really-refresh] [--unresolve] [--again | -g]
[--info-only] [--index-info]
[-z] [--stdin] [--index-version ]
@@ -179,6 +179,13 @@ may not support it yet.
system must change `st_mtime` field of a directory if files
are added or deleted in that directory.
+--test-untracked-cache::
+ Only perform tests on the working directory to make sure
+ untracked cache can be used. You have to manually enable
+ untracked cache using `--force-untracked-cache` (or
+ `--untracked-cache` but this will run the tests again)
+ afterwards if you really want to use it.
+
--force-untracked-cache::
For safety, `--untracked-cache` performs tests on the working
directory to make sure untracked cache can be used. These
diff --git a/builtin/update-index.c b/builtin/update-index.c
index e568acc..b7b5108 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -996,8 +996,10 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
N_("enable or disable split index")),
OPT_BOOL(0, "untracked-cache", &untracked_cache,
N_("enable/disable untracked cache")),
+ OPT_SET_INT(0, "test-untracked-cache", &untracked_cache,
+ N_("test if the filesystem supports untracked
cache"), 2),
OPT_SET_INT(0, "force-untracked-cache", &untracked_cache,
- N_("enable untracked cache without testing the
filesystem"), 2),
+ N_("enable untracked cache without testing the
filesystem"), 3),
OPT_END()
};
@@ -1107,10 +1109,12 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
if (untracked_cache > 0) {
struct untracked_cache *uc;
- if (untracked_cache < 2) {
+ if (untracked_cache < 3) {
setup_work_tree();
if (!test_if_untracked_cache_is_supported())
return 1;
+ if (untracked_cache == 2)
+ return 0;
}
if (!the_index.untracked) {
uc = xcalloc(1, sizeof(*uc));
--
2.6.3.391.g95a3a5c
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 3/8] update-index: move 'uc' var declaration
Signed-off-by: Christian Couder
---
builtin/update-index.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/builtin/update-index.c b/builtin/update-index.c
index b7b5108..b54ddc3 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1107,8 +1107,6 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
the_index.cache_changed |= SOMETHING_CHANGED;
}
if (untracked_cache > 0) {
- struct untracked_cache *uc;
-
if (untracked_cache < 3) {
setup_work_tree();
if (!test_if_untracked_cache_is_supported())
@@ -1117,7 +1115,7 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
return 0;
}
if (!the_index.untracked) {
- uc = xcalloc(1, sizeof(*uc));
+ struct untracked_cache *uc = xcalloc(1, sizeof(*uc));
strbuf_init(&uc->ident, 100);
uc->exclude_per_dir = ".gitignore";
/* should be the same flags used by git-status */
--
2.6.3.391.g95a3a5c
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 8/8] update-index: make core.untrackedCache a bool
Most features in Git can be enabled or disabled using a simple
bool config variable and it would be nice if untracked cache
behaved the same way.
This makes --[no-|force-]untracked-cache change the value of
core.untrackedCache in the repo config file, to avoid making
those options useless and because this avoids the untracked
cache being disabled by a kernel change or a directory
change. Of course this breaks some backward compatibility,
but the simplification and increased useability is worth it.
Signed-off-by: Christian Couder
---
Documentation/git-update-index.txt | 13 ++---
builtin/update-index.c | 10 --
config.c | 8 +---
3 files changed, 11 insertions(+), 20 deletions(-)
diff --git a/Documentation/git-update-index.txt
b/Documentation/git-update-index.txt
index 5f74cc7..256b9c5 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -181,10 +181,11 @@ The underlying operating system and file system must
change `st_mtime`
field of a directory if files are added or deleted in that
directory. You can test that using
`--test-untracked-cache`. `--untracked-cache` used to test that too
-but it doesn't anymore. If you want to always enable, or always
-disable, untracked cache, you can set the `core.untrackedCache`
-configuration variable to 'true' or 'false' respectively, (see
-linkgit:git-config[1]).
+but it doesn't anymore.
++
+This sets the `core.untrackedCache` configuration variable to 'true'
+or 'false' in the repo config file, (see linkgit:git-config[1]), so
+that the untracked cache stays enabled or disabled.
--test-untracked-cache::
Only perform tests on the working directory to make sure
@@ -194,9 +195,7 @@ linkgit:git-config[1]).
it.
--force-untracked-cache::
- Same as `--untracked-cache`. Note that this option cannot
- enable untracked cache if `core.untrackedCache` configuration
- variable is set to false (see linkgit:git-config[1]).
+ Same as `--untracked-cache`.
\--::
Do not interpret any more arguments as options.
diff --git a/builtin/update-index.c b/builtin/update-index.c
index fb0ea3d..048c293 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -,15 +,13 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
return !test_if_untracked_cache_is_supported();
}
if (untracked_cache > 0) {
- if (!use_untracked_cache)
- die("core.untrackedCache is set to false; "
- "the untracked cache will not be enabled");
+ if (!use_untracked_cache &&
git_config_set("core.untrackedCache", "true"))
+ die("could not set core.untrackedCache to true");
add_untracked_cache();
fprintf(stderr, _("Untracked cache enabled for '%s'\n"),
get_git_work_tree());
} else if (!untracked_cache) {
- if (use_untracked_cache > 0)
- die("core.untrackedCache is set to true; "
- "the untracked cache will not be disabled");
+ if (use_untracked_cache > 0 &&
git_config_set("core.untrackedCache", "false"))
+ die("could not set core.untrackedCache to false");
if (the_index.untracked) {
remove_untracked_cache();
fprintf(stderr, _("Untracked disabled\n"));
diff --git a/config.c b/config.c
index 7d50f43..f023ee7 100644
--- a/config.c
+++ b/config.c
@@ -692,13 +692,7 @@ static int git_default_core_config(const char *var, const
char *value)
return 0;
}
if (!strcmp(var, "core.untrackedcache")) {
- if (!strcasecmp(value, "default") || !strcasecmp(value,
"check"))
- use_untracked_cache = -1;
- else {
- use_untracked_cache = git_config_maybe_bool(var, value);
- if (use_untracked_cache == -1)
- error("unknown core.untrackedCache value '%s';
using default", value);
- }
+ use_untracked_cache = git_config_bool(var, value);
return 0;
}
if (!strcmp(var, "core.checkstat")) {
--
2.6.3.391.g95a3a5c
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 4/8] dir: add add_untracked_cache()
This new function will be used in a later patch.
Signed-off-by: Christian Couder
---
builtin/update-index.c | 11 +--
dir.c | 14 ++
dir.h | 1 +
3 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/builtin/update-index.c b/builtin/update-index.c
index b54ddc3..ec67d14 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1114,16 +1114,7 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
if (untracked_cache == 2)
return 0;
}
- if (!the_index.untracked) {
- struct untracked_cache *uc = xcalloc(1, sizeof(*uc));
- strbuf_init(&uc->ident, 100);
- uc->exclude_per_dir = ".gitignore";
- /* should be the same flags used by git-status */
- uc->dir_flags = DIR_SHOW_OTHER_DIRECTORIES |
DIR_HIDE_EMPTY_DIRECTORIES;
- the_index.untracked = uc;
- }
- add_untracked_ident(the_index.untracked);
- the_index.cache_changed |= UNTRACKED_CHANGED;
+ add_untracked_cache();
fprintf(stderr, _("Untracked cache enabled for '%s'\n"),
get_git_work_tree());
} else if (!untracked_cache && the_index.untracked) {
the_index.untracked = NULL;
diff --git a/dir.c b/dir.c
index d2a8f06..0f7e293 100644
--- a/dir.c
+++ b/dir.c
@@ -1938,6 +1938,20 @@ void add_untracked_ident(struct untracked_cache *uc)
strbuf_addch(&uc->ident, 0);
}
+void add_untracked_cache(void)
+{
+ if (!the_index.untracked) {
+ struct untracked_cache *uc = xcalloc(1, sizeof(*uc));
+ strbuf_init(&uc->ident, 100);
+ uc->exclude_per_dir = ".gitignore";
+ /* should be the same flags used by git-status */
+ uc->dir_flags = DIR_SHOW_OTHER_DIRECTORIES |
DIR_HIDE_EMPTY_DIRECTORIES;
+ the_index.untracked = uc;
+ }
+ add_untracked_ident(the_index.untracked);
+ the_index.cache_changed |= UNTRACKED_CHANGED;
+}
+
static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct
*dir,
int base_len,
const struct pathspec
*pathspec)
diff --git a/dir.h b/dir.h
index 7b5855d..ee94c76 100644
--- a/dir.h
+++ b/dir.h
@@ -308,4 +308,5 @@ void free_untracked_cache(struct untracked_cache *);
struct untracked_cache *read_untracked_extension(const void *data, unsigned
long sz);
void write_untracked_extension(struct strbuf *out, struct untracked_cache
*untracked);
void add_untracked_ident(struct untracked_cache *);
+void add_untracked_cache(void);
#endif
--
2.6.3.391.g95a3a5c
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 1/8] update-index: add untracked cache notifications
Doing:
cd /tmp
git --git-dir=/git/somewhere/else/.git update-index --untracked-cache
doesn't work how one would expect. It hardcodes "/tmp" as the directory
that "works" into the index, so if you use the working tree, you'll
never use the untracked cache.
With this patch "git update-index --untracked-cache" tells the user in
which directory tests are performed and in which working directory the
untracked cache is allowed.
Signed-off-by: Christian Couder
---
builtin/update-index.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 7431938..e568acc 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -121,7 +121,7 @@ static int test_if_untracked_cache_is_supported(void)
if (!mkdtemp(mtime_dir.buf))
die_errno("Could not make temporary directory");
- fprintf(stderr, _("Testing "));
+ fprintf(stderr, _("Testing mtime in '%s' "), xgetcwd());
atexit(remove_test_directory);
xstat_mtime_dir(&st);
fill_stat_data(&base, &st);
@@ -1122,9 +1122,11 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
}
add_untracked_ident(the_index.untracked);
the_index.cache_changed |= UNTRACKED_CHANGED;
+ fprintf(stderr, _("Untracked cache enabled for '%s'\n"),
get_git_work_tree());
} else if (!untracked_cache && the_index.untracked) {
the_index.untracked = NULL;
the_index.cache_changed |= UNTRACKED_CHANGED;
+ fprintf(stderr, _("Untracked disabled\n"));
}
if (active_cache_changed) {
--
2.6.3.391.g95a3a5c
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 6/8] config: add core.untrackedCache
When we know that mtime is fully supported by the environment, we
might want the untracked cache to be always used by default without
any mtime test or kernel version check being performed.
Also when we know that mtime is not supported by the environment,
for example because the repo is shared over a network file system,
then we might want 'git update-index --untracked-cache' to fail
immediately instead of it testing if it works (because it might
work on some systems using the repo over the network file system
but not others).
Signed-off-by: Christian Couder
---
Documentation/config.txt | 10 ++
Documentation/git-update-index.txt | 11 +--
builtin/update-index.c | 28 ++--
cache.h| 1 +
config.c | 10 ++
contrib/completion/git-completion.bash | 1 +
dir.c | 2 +-
environment.c | 1 +
wt-status.c| 9 +
9 files changed, 60 insertions(+), 13 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index b4b0194..bf176ff 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -308,6 +308,16 @@ core.trustctime::
crawlers and some backup systems).
See linkgit:git-update-index[1]. True by default.
+core.untrackedCache::
+ If unset or set to 'default' or 'check', untracked cache will
+ not be enabled by default and when
+ 'update-index --untracked-cache' is called, Git will test if
+ mtime is working properly before enabling it. If set to false,
+ Git will refuse to enable untracked cache even if
+ '--force-untracked-cache' is used. If set to true, Git will
+ blindly enabled untracked cache by default without testing if
+ it works. See linkgit:git-update-index[1].
+
core.checkStat::
Determines which stat fields to match between the index
and work tree. The user can set this to 'default' or
diff --git a/Documentation/git-update-index.txt
b/Documentation/git-update-index.txt
index 0ff7396..4e6078b 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -177,7 +177,10 @@ may not support it yet.
up for commands that involve determining untracked files such
as `git status`. The underlying operating system and file
system must change `st_mtime` field of a directory if files
- are added or deleted in that directory.
+ are added or deleted in that directory. If you want to always
+ enable, or always disable, untracked cache, you can set the
+ `core.untrackedCache` configuration variable to 'true' or
+ 'false' respectively, (see linkgit:git-config[1]).
--test-untracked-cache::
Only perform tests on the working directory to make sure
@@ -190,7 +193,9 @@ may not support it yet.
For safety, `--untracked-cache` performs tests on the working
directory to make sure untracked cache can be used. These
tests can take a few seconds. `--force-untracked-cache` can be
- used to skip the tests.
+ used to skip the tests. It cannot enable untracked cache if
+ `core.untrackedCache` configuration variable is set to false
+ (see linkgit:git-config[1]).
\--::
Do not interpret any more arguments as options.
@@ -406,6 +411,8 @@ It can be useful when the inode change time is regularly
modified by
something outside Git (file system crawlers and backup systems use
ctime for marking files processed) (see linkgit:git-config[1]).
+Untracked cache look at `core.untrackedCache` configuration variable
+(see linkgit:git-config[1]).
SEE ALSO
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 2cbaee0..bf697a5 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1106,19 +1106,27 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
the_index.split_index = NULL;
the_index.cache_changed |= SOMETHING_CHANGED;
}
+ if (untracked_cache == 2 || (untracked_cache == 1 &&
use_untracked_cache == -1)) {
+ setup_work_tree();
+ if (!test_if_untracked_cache_is_supported())
+ return 1;
+ if (untracked_cache == 2)
+ return 0;
+ }
if (untracked_cache > 0) {
- if (untracked_cache < 3) {
- setup_work_tree();
- if (!test_if_untracked_cache_is_supported())
- return 1;
- if (untracked_cache == 2)
- return 0;
- }
+ if (!use_untracked_cache)
+ die("core.unt
[RFC/PATCH 7/8] update-index: prevent --untracked-cache from performing tests
`git update-index --untracked-cache` used to perform tests to
check that the underlying operating system and file system
change `st_mtime` field of a directory if files are added or
deleted in that directory. But those tests take a long time
and there is now `--test-untracked-cache` to perform them.
So to be more consistent with other git commands it's better
to make `--untracked-cache` not perform them, which is the
purpose of this patch.
Note that after this patch there is no difference any more
between `--untracked-cache` and `--force-untracked-cache`.
Signed-off-by: Christian Couder
---
Documentation/git-update-index.txt | 31 ---
builtin/update-index.c | 7 ++-
t/t7063-status-untracked-cache.sh | 2 +-
3 files changed, 19 insertions(+), 21 deletions(-)
diff --git a/Documentation/git-update-index.txt
b/Documentation/git-update-index.txt
index 4e6078b..5f74cc7 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -175,27 +175,28 @@ may not support it yet.
--no-untracked-cache::
Enable or disable untracked cache extension. This could speed
up for commands that involve determining untracked files such
- as `git status`. The underlying operating system and file
- system must change `st_mtime` field of a directory if files
- are added or deleted in that directory. If you want to always
- enable, or always disable, untracked cache, you can set the
- `core.untrackedCache` configuration variable to 'true' or
- 'false' respectively, (see linkgit:git-config[1]).
+ as `git status`.
++
+The underlying operating system and file system must change `st_mtime`
+field of a directory if files are added or deleted in that
+directory. You can test that using
+`--test-untracked-cache`. `--untracked-cache` used to test that too
+but it doesn't anymore. If you want to always enable, or always
+disable, untracked cache, you can set the `core.untrackedCache`
+configuration variable to 'true' or 'false' respectively, (see
+linkgit:git-config[1]).
--test-untracked-cache::
Only perform tests on the working directory to make sure
untracked cache can be used. You have to manually enable
- untracked cache using `--force-untracked-cache` (or
- `--untracked-cache` but this will run the tests again)
- afterwards if you really want to use it.
+ untracked cache using `--untracked-cache` or
+ `--force-untracked-cache` afterwards if you really want to use
+ it.
--force-untracked-cache::
- For safety, `--untracked-cache` performs tests on the working
- directory to make sure untracked cache can be used. These
- tests can take a few seconds. `--force-untracked-cache` can be
- used to skip the tests. It cannot enable untracked cache if
- `core.untrackedCache` configuration variable is set to false
- (see linkgit:git-config[1]).
+ Same as `--untracked-cache`. Note that this option cannot
+ enable untracked cache if `core.untrackedCache` configuration
+ variable is set to false (see linkgit:git-config[1]).
\--::
Do not interpret any more arguments as options.
diff --git a/builtin/update-index.c b/builtin/update-index.c
index bf697a5..fb0ea3d 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1106,12 +1106,9 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
the_index.split_index = NULL;
the_index.cache_changed |= SOMETHING_CHANGED;
}
- if (untracked_cache == 2 || (untracked_cache == 1 &&
use_untracked_cache == -1)) {
+ if (untracked_cache == 2) {
setup_work_tree();
- if (!test_if_untracked_cache_is_supported())
- return 1;
- if (untracked_cache == 2)
- return 0;
+ return !test_if_untracked_cache_is_supported();
}
if (untracked_cache > 0) {
if (!use_untracked_cache)
diff --git a/t/t7063-status-untracked-cache.sh
b/t/t7063-status-untracked-cache.sh
index 0e8d0d4..8c3e703 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -11,7 +11,7 @@ avoid_racy() {
# It's fine if git update-index returns an error code other than one,
# it'll be caught in the first test.
test_lazy_prereq UNTRACKED_CACHE '
- { git update-index --untracked-cache; ret=$?; } &&
+ { git update-index --test-untracked-cache; ret=$?; } &&
test $ret -ne 1
'
--
2.6.3.391.g95a3a5c
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 6/8] config: add core.untrackedCache
On Thu, Dec 3, 2015 at 5:10 PM, Torsten Bögershausen wrote: > [snip all good stuff] > > First of all: > Thanks for explaining it so well > > I now can see the point in having this patch. > (Do the commit messages reflect all this ? I need to re-read) Maybe not. I will have a look at improving them, but your re-reading is welcome. > The other question is: Do you have patch on a public repo ? Yes, here: https://github.com/chriscool/git/commits/uc-notifs8 > And, of course, can we add 1 or 2 test cases ? Yes I had planned to add tests for this. But it would be nice if I could know first if the last two patches are considered ok even though they are breaking compatibility a bit. In this case I could squash them with previous patches and only write tests for the resulting patches. > Thanks for pushing this forward. Thanks for your reviews, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 6/8] config: add core.untrackedCache
On Fri, Dec 4, 2015 at 6:54 PM, Torsten Bögershausen wrote: >> Current state of affairs: >> >> * Enable on a per-repo basis: git update-index --untracked-cache >> * Disable on a per-repo basis: git update-index --no-cache >> * Enable system-wide: N/A >> * Disable system-wide: N/A >> >> With this patch: >> >> * Enable on a per-repo basis: git update-index --untracked-cache OR >> "git config core.untrackedCache true" >> * Disable on a per-repo basis: git update-index --no-cache OR "git >> config core.untrackedCache false" >> * Enable system-wide: git config --global core.untrackedCache true >> * Disable system-wide: git config --global core.untrackedCache false >> * Caveat: The core.untrackedCache config has precidence over "git >> update-index" >> >> With the rest of the patches in this series: >> >> * Enable system-wide & per-repo the same, just tweak >> core.untrackedCache either for the local .git or --globally >> * If you want to test things either per-repo or globally just use >> "git update-index --test-untracked-cache" >> * If you want something exactly like the old --untracked-cache do: >> "git update-index --test-untracked-cache && git config >> core.untrackedCache true" >> >> I think applying this whole series makes sense. Enabling this feature >> doesn't work like anything else in Git, usually you just tweak a >> config option and thus can easily enable things either system-wide or >> per-repo (or any combination of the two), which makes both system >> administration and local configuration easy. >> >> A much saner UI for the CLI tools if we're going to ship git with >> tests for filesystem features is to separate the testing from the >> enabling of those features. > > My spontanous feeling: squash 6-8 together and add this nice explanation > to the commit message. Ok, I will do that then. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Draft of Git Rev News edition 10
Hi, A draft of Git Rev News edition 10 is available here: https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-10.md Everyone is welcome to contribute in any section either by editing the above page on GitHub and sending a pull request, or by commenting on this GitHub issue: https://github.com/git/git.github.io/issues/112 You can also reply to this email. I tried to cc everyone who appears in this edition but maybe I missed some people, sorry about that. Thomas, Nicola and myself plan to publish this edition on Wednesday the 9th of December. Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 7/8] update-index: prevent --untracked-cache from performing tests
On Wed, Dec 2, 2015 at 8:18 PM, Duy Nguyen wrote:
> On Tue, Dec 1, 2015 at 9:31 PM, Christian Couder
> wrote:
> diff --git a/t/t7063-status-untracked-cache.sh
> b/t/t7063-status-untracked-cache.sh
>> index 0e8d0d4..8c3e703 100755
>> --- a/t/t7063-status-untracked-cache.sh
>> +++ b/t/t7063-status-untracked-cache.sh
>> @@ -11,7 +11,7 @@ avoid_racy() {
>> # It's fine if git update-index returns an error code other than one,
>> # it'll be caught in the first test.
>
> Notice this comment. You probably have to chance --test-untr.. for the
> first test too if it's stilll true, or delete the comment.
Ok, I think I will remove the comment and still use "git update-index
--untracked-cache" in the first test.
>> test_lazy_prereq UNTRACKED_CACHE '
>> - { git update-index --untracked-cache; ret=$?; } &&
>> + { git update-index --test-untracked-cache; ret=$?; } &&
>> test $ret -ne 1
>> '
Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] config: add core.trustmtime
On Wed, Dec 2, 2015 at 8:28 PM, Duy Nguyen wrote: > On Wed, Nov 25, 2015 at 10:00 AM, Ævar Arnfjörð Bjarmason > wrote: >> Aside from the slight hassle of enabling this and keeping it enabled >> this feature is great. It's sped up "git status" across the board by >> about 40%. Slightly less than that on faster spinning disks, slightly >> more than that on slower ones. > > Before I forget again, you should also enable split-index. That > feature was added because index update time cut so much into the > saving from untracked cache (unless you have small indexes, unlikely). > And untracked cache can update the index often. Then maybe you can > also think about improving the usability for it to ;-) Yeah, we will probably have a look at split-index next. Thanks for developing it too, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/8] update-index: add --test-untracked-cache
On Wed, Dec 2, 2015 at 8:17 PM, Duy Nguyen wrote:
> On Tue, Dec 1, 2015 at 9:31 PM, Christian Couder
> wrote:
>> diff --git a/builtin/update-index.c b/builtin/update-index.c
>> index e568acc..b7b5108 100644
>> --- a/builtin/update-index.c
>> +++ b/builtin/update-index.c
>> @@ -996,8 +996,10 @@ int cmd_update_index(int argc, const char **argv, const
>> char *prefix)
>> N_("enable or disable split index")),
>> OPT_BOOL(0, "untracked-cache", &untracked_cache,
>> N_("enable/disable untracked cache")),
>> + OPT_SET_INT(0, "test-untracked-cache", &untracked_cache,
>> + N_("test if the filesystem supports untracked
>> cache"), 2),
>> OPT_SET_INT(0, "force-untracked-cache", &untracked_cache,
>> - N_("enable untracked cache without testing the
>> filesystem"), 2),
>> + N_("enable untracked cache without testing the
>> filesystem"), 3),
>> OPT_END()
>> };
>
> I think we got enough numbers to start using enum instead.
Ok, I will use an enum.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 1/8] update-index: add untracked cache notifications
On Wed, Dec 2, 2015 at 8:16 PM, Duy Nguyen wrote:
> On Tue, Dec 1, 2015 at 9:31 PM, Christian Couder
> wrote:
>> Doing:
>>
>> cd /tmp
>> git --git-dir=/git/somewhere/else/.git update-index --untracked-cache
>>
>> doesn't work how one would expect. It hardcodes "/tmp" as the directory
>> that "works" into the index, so if you use the working tree, you'll
>> never use the untracked cache.
>>
>> With this patch "git update-index --untracked-cache" tells the user in
>> which directory tests are performed and in which working directory the
>> untracked cache is allowed.
>>
>> Signed-off-by: Christian Couder
>> ---
>> builtin/update-index.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/builtin/update-index.c b/builtin/update-index.c
>> index 7431938..e568acc 100644
>> --- a/builtin/update-index.c
>> +++ b/builtin/update-index.c
>> @@ -121,7 +121,7 @@ static int test_if_untracked_cache_is_supported(void)
>> if (!mkdtemp(mtime_dir.buf))
>> die_errno("Could not make temporary directory");
>>
>> - fprintf(stderr, _("Testing "));
>> + fprintf(stderr, _("Testing mtime in '%s' "), xgetcwd());
>
> We probably should respect --verbose. I know I violated it in the first place.
The verbose help is:
--verbose report actions to standard output
so yeah, it is not respected first because the output is on by
default, and second because the output is on stderr instead of stdout.
Anyway it can be a separate patch or patch series to make it respect
one or both of these points.
I am not very much interested in doing it myself as I think it's
interesting to have the output by default especially if the above
patch is applied. But if people agree that it would be a good thing, I
will do it.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 8/8] update-index: make core.untrackedCache a bool
(Sorry I already sent a private reply to Tosten by mistake.) On Sat, Dec 5, 2015 at 1:44 PM, Torsten Bögershausen wrote: > On 01.12.15 21:31, Christian Couder wrote: >> Most features in Git can be enabled or disabled using a simple >> bool config variable and it would be nice if untracked cache >> behaved the same way. >> >> This makes --[no-|force-]untracked-cache change the value of >> core.untrackedCache in the repo config file, to avoid making >> those options useless and because this avoids the untracked >> cache being disabled by a kernel change or a directory >> change. Of course this breaks some backward compatibility, >> but the simplification and increased useability is worth it. > > Some loose thinking, how the core.untrackedcache and the > command line can work together: > > core.untrackedcache = unset > everything is as today > if --test-untracked-cache is used on command line, > the test is run, additionally the result is stored > in the config variable core.untrackedcache I don't like storing the result in core.untrackedcache as it means that --test-untracked-cache would not just test. And I agree with Aevar that it's a good thing to be able to completely separate testing and configuring. > core.untrackedcache = true > same as --force-untracked-cache > if --no-untracked-cache is given on command line, > this has higher priority I guess you mean that --no-untracked-cache has priority over "core.untrackedcache = true" without removing this config variable. This means that --no-untracked-cache would only remove the untracked cache (UC) from the index. But what git should then do the next time "git status" is run? Git sees that the index does not contain any UC, but it doesn't know that "git update-index --no-untracked-cache" has just been run. It might be "git config core.untrackedcache true" that has been run last. If "git config core.untrackedcache true" has been run last, it would be logical to add the UC to the index and use it. If "git update-index --no-untracked-cache" has been run last, it would be logical to not add the UC. > core.untrackedcache = false > same as --no-untracked-cache > if --force-untracked-cache is given on command line, > this has higher priority Then the same kind of problem happens as above. There is no clear solution about what "git status" should do when the state of the index conflicts with the value of core.untrackedcache. > Does this support the workflow described by Ævar ? I don't think so. I think that when we set "core.untrackedcache = true" we want the UC to be added to the index and used as much as possible, because we know that our OS/filesystem supports it. This means that using "git update-index --no-untracked-cache" when "core.untrackedcache = true" is set can only have two possible outcomes. It could either just die saying that "core.untrackedcache" is true, or it could just unset "core.untrackedcache" or set it to false (which might mean the same thing). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/8] Untracked cache improvements
Following the previous RFC version of this patch series and the related discussions, here is a new version that tries to improve the untracked cache feature. This patch series implements core.untrackedCache as a bool config variable. When it's true git should always try to use the untracked cache, and when false git should never use it. Patchs 1/8 and 3/8 add some features that are missing. Patch 2/8, which is new, adds an enum as suggested by Duy. Patchs 4/8, 5/8 and 6/8 are some refactoring to prepare for patch 7/8 which implements core.untrackedCache. Patch 7/8 is the result of squashing the last 3 patches of the RFC series. As discussed this sacrifies backward compatibility for a simpler interface. Patch 8/8, which is new, add some tests. So the changes compared to the RFC version are just small bug fixes and patchs 2/8 and 8/8. The patch series is also available there: https://github.com/chriscool/git/tree/uc-notifs14 Christian Couder (8): update-index: add untracked cache notifications update-index: use enum for untracked cache options update-index: add --test-untracked-cache update-index: move 'uc' var declaration dir: add add_untracked_cache() dir: add remove_untracked_cache() config: add core.untrackedCache t7063: add tests for core.untrackedCache Documentation/config.txt | 7 + Documentation/git-update-index.txt | 31 ++-- builtin/update-index.c | 52 +++--- cache.h| 1 + config.c | 4 +++ contrib/completion/git-completion.bash | 1 + dir.c | 22 +- dir.h | 2 ++ environment.c | 1 + t/t7063-status-untracked-cache.sh | 52 ++ wt-status.c| 9 ++ 11 files changed, 145 insertions(+), 37 deletions(-) -- 2.6.3.478.g9f95483.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/8] dir: add add_untracked_cache()
This new function will be used in a later patch.
Signed-off-by: Christian Couder
---
builtin/update-index.c | 11 +--
dir.c | 14 ++
dir.h | 1 +
3 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 21f74b2..40530b0 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1123,16 +1123,7 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
if (untracked_cache == TEST_UC)
return 0;
}
- if (!the_index.untracked) {
- struct untracked_cache *uc = xcalloc(1, sizeof(*uc));
- strbuf_init(&uc->ident, 100);
- uc->exclude_per_dir = ".gitignore";
- /* should be the same flags used by git-status */
- uc->dir_flags = DIR_SHOW_OTHER_DIRECTORIES |
DIR_HIDE_EMPTY_DIRECTORIES;
- the_index.untracked = uc;
- }
- add_untracked_ident(the_index.untracked);
- the_index.cache_changed |= UNTRACKED_CHANGED;
+ add_untracked_cache();
fprintf(stderr, _("Untracked cache enabled for '%s'\n"),
get_git_work_tree());
} else if (untracked_cache == NO_UC && the_index.untracked) {
the_index.untracked = NULL;
diff --git a/dir.c b/dir.c
index d2a8f06..0f7e293 100644
--- a/dir.c
+++ b/dir.c
@@ -1938,6 +1938,20 @@ void add_untracked_ident(struct untracked_cache *uc)
strbuf_addch(&uc->ident, 0);
}
+void add_untracked_cache(void)
+{
+ if (!the_index.untracked) {
+ struct untracked_cache *uc = xcalloc(1, sizeof(*uc));
+ strbuf_init(&uc->ident, 100);
+ uc->exclude_per_dir = ".gitignore";
+ /* should be the same flags used by git-status */
+ uc->dir_flags = DIR_SHOW_OTHER_DIRECTORIES |
DIR_HIDE_EMPTY_DIRECTORIES;
+ the_index.untracked = uc;
+ }
+ add_untracked_ident(the_index.untracked);
+ the_index.cache_changed |= UNTRACKED_CHANGED;
+}
+
static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct
*dir,
int base_len,
const struct pathspec
*pathspec)
diff --git a/dir.h b/dir.h
index 7b5855d..ee94c76 100644
--- a/dir.h
+++ b/dir.h
@@ -308,4 +308,5 @@ void free_untracked_cache(struct untracked_cache *);
struct untracked_cache *read_untracked_extension(const void *data, unsigned
long sz);
void write_untracked_extension(struct strbuf *out, struct untracked_cache
*untracked);
void add_untracked_ident(struct untracked_cache *);
+void add_untracked_cache(void);
#endif
--
2.6.3.478.g9f95483.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/8] update-index: add untracked cache notifications
Doing:
cd /tmp
git --git-dir=/git/somewhere/else/.git update-index --untracked-cache
doesn't work how one would expect. It hardcodes "/tmp" as the directory
that "works" into the index, so if you use the working tree, you'll
never use the untracked cache.
With this patch "git update-index --untracked-cache" tells the user in
which directory tests are performed and in which working directory the
untracked cache is allowed.
Signed-off-by: Christian Couder
---
builtin/update-index.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 7431938..6f6b289 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -121,7 +121,7 @@ static int test_if_untracked_cache_is_supported(void)
if (!mkdtemp(mtime_dir.buf))
die_errno("Could not make temporary directory");
- fprintf(stderr, _("Testing "));
+ fprintf(stderr, _("Testing mtime in '%s' "), xgetcwd());
atexit(remove_test_directory);
xstat_mtime_dir(&st);
fill_stat_data(&base, &st);
@@ -1122,9 +1122,11 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
}
add_untracked_ident(the_index.untracked);
the_index.cache_changed |= UNTRACKED_CHANGED;
+ fprintf(stderr, _("Untracked cache enabled for '%s'\n"),
get_git_work_tree());
} else if (!untracked_cache && the_index.untracked) {
the_index.untracked = NULL;
the_index.cache_changed |= UNTRACKED_CHANGED;
+ fprintf(stderr, _("Untracked cache disabled\n"));
}
if (active_cache_changed) {
--
2.6.3.478.g9f95483.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/8] config: add core.untrackedCache
When we know that mtime is fully supported by the environment, we might want the untracked cache to be always used by default without any mtime test or kernel version check being performed. Also when we know that mtime is not supported by the environment, for example because the repo is shared over a network file system, then we might want 'git update-index --untracked-cache' to fail immediately instead of preforming tests (because it might work on some systems using the repo over the network file system but not others). The normal way to achieve the above in Git is to use a config variable. That's why this patch introduces "core.untrackedCache". To keep things simple, this variable is a bool which default to false. When "git status" is run, it now adds or removes the untracked cache in the index to respect the value of this variable. This means that `git update-index --[no-|force-]untracked-cache`, to be compatible with the new config variable, have to set or unset it. This new behavior is backward incompatible change, but that is deliberate. Also `--untracked-cache` used to check that the underlying operating system and file system change `st_mtime` field of a directory if files are added or deleted in that directory. But those tests take a long time and there is now `--test-untracked-cache` to perform them. So to be more consistent with other git commands, this patch prevents `--untracked-cache` to perform tests. This means that after this patch there is no difference any more between `--untracked-cache` and `--force-untracked-cache`. Signed-off-by: Christian Couder --- Documentation/config.txt | 7 +++ Documentation/git-update-index.txt | 28 ++-- builtin/update-index.c | 23 +-- cache.h| 1 + config.c | 4 contrib/completion/git-completion.bash | 1 + dir.c | 2 +- environment.c | 1 + t/t7063-status-untracked-cache.sh | 4 +--- wt-status.c| 9 + 10 files changed, 56 insertions(+), 24 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 2d06b11..94820eb 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -308,6 +308,13 @@ core.trustctime:: crawlers and some backup systems). See linkgit:git-update-index[1]. True by default. +core.untrackedCache:: + Determines if untracked cache will be enabled. Using + 'git update-index --[no-|force-]untracked-cache' will set + this variable. Before setting it to true, you should check + that mtime is working properly on your system. + See linkgit:git-update-index[1]. False by default. + core.checkStat:: Determines which stat fields to match between the index and work tree. The user can set this to 'default' or diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt index 0ff7396..0fb39db 100644 --- a/Documentation/git-update-index.txt +++ b/Documentation/git-update-index.txt @@ -175,22 +175,28 @@ may not support it yet. --no-untracked-cache:: Enable or disable untracked cache extension. This could speed up for commands that involve determining untracked files such - as `git status`. The underlying operating system and file - system must change `st_mtime` field of a directory if files - are added or deleted in that directory. + as `git status`. ++ +The underlying operating system and file system must change `st_mtime` +field of a directory if files are added or deleted in that +directory. You can test that using +`--test-untracked-cache`. `--untracked-cache` used to test that too +but it doesn't anymore. ++ +This sets the `core.untrackedCache` configuration variable to 'true' +or 'false' in the repo config file, (see linkgit:git-config[1]), so +that the untracked cache stays enabled or disabled. --test-untracked-cache:: Only perform tests on the working directory to make sure untracked cache can be used. You have to manually enable - untracked cache using `--force-untracked-cache` (or - `--untracked-cache` but this will run the tests again) - afterwards if you really want to use it. + untracked cache using `--untracked-cache` or + `--force-untracked-cache` or the `core.untrackedCache` + configuration variable afterwards if you really want to use + it. --force-untracked-cache:: - For safety, `--untracked-cache` performs tests on the working - directory to make sure untracked cache can be used. These - tests can take a few seconds. `--force-untracked-cache` can be - used to skip the tests. + Same as `--untracked-cache`. \--:: Do not interp
[PATCH 3/8] update-index: add --test-untracked-cache
It is nice to just be able to test if untracked cache is
supported without enabling it.
Signed-off-by: Christian Couder
---
Documentation/git-update-index.txt | 9 -
builtin/update-index.c | 5 +
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-update-index.txt
b/Documentation/git-update-index.txt
index 3df9c26..0ff7396 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -17,7 +17,7 @@ SYNOPSIS
[--[no-]assume-unchanged]
[--[no-]skip-worktree]
[--ignore-submodules]
-[--[no-|force-]untracked-cache]
+[--[no-|test-|force-]untracked-cache]
[--really-refresh] [--unresolve] [--again | -g]
[--info-only] [--index-info]
[-z] [--stdin] [--index-version ]
@@ -179,6 +179,13 @@ may not support it yet.
system must change `st_mtime` field of a directory if files
are added or deleted in that directory.
+--test-untracked-cache::
+ Only perform tests on the working directory to make sure
+ untracked cache can be used. You have to manually enable
+ untracked cache using `--force-untracked-cache` (or
+ `--untracked-cache` but this will run the tests again)
+ afterwards if you really want to use it.
+
--force-untracked-cache::
For safety, `--untracked-cache` performs tests on the working
directory to make sure untracked cache can be used. These
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 246b3d3..ecb685d 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -40,6 +40,7 @@ enum uc_mode {
UNDEF_UC = -1,
NO_UC = 0,
UC,
+ TEST_UC,
FORCE_UC
};
@@ -1004,6 +1005,8 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
N_("enable or disable split index")),
OPT_BOOL(0, "untracked-cache", &untracked_cache,
N_("enable/disable untracked cache")),
+ OPT_SET_INT(0, "test-untracked-cache", &untracked_cache,
+ N_("test if the filesystem supports untracked
cache"), TEST_UC),
OPT_SET_INT(0, "force-untracked-cache", &untracked_cache,
N_("enable untracked cache without testing the
filesystem"), FORCE_UC),
OPT_END()
@@ -1119,6 +1122,8 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
setup_work_tree();
if (!test_if_untracked_cache_is_supported())
return 1;
+ if (untracked_cache == TEST_UC)
+ return 0;
}
if (!the_index.untracked) {
uc = xcalloc(1, sizeof(*uc));
--
2.6.3.478.g9f95483.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/8] update-index: use enum for untracked cache options
Signed-off-by: Christian Couder
---
builtin/update-index.c | 18 +-
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 6f6b289..246b3d3 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -35,6 +35,14 @@ static int mark_skip_worktree_only;
#define UNMARK_FLAG 2
static struct strbuf mtime_dir = STRBUF_INIT;
+/* Untracked cache mode */
+enum uc_mode {
+ UNDEF_UC = -1,
+ NO_UC = 0,
+ UC,
+ FORCE_UC
+};
+
__attribute__((format (printf, 1, 2)))
static void report(const char *fmt, ...)
{
@@ -902,7 +910,7 @@ static int reupdate_callback(struct parse_opt_ctx_t *ctx,
int cmd_update_index(int argc, const char **argv, const char *prefix)
{
int newfd, entries, has_errors = 0, line_termination = '\n';
- int untracked_cache = -1;
+ enum uc_mode untracked_cache = UNDEF_UC;
int read_from_stdin = 0;
int prefix_length = prefix ? strlen(prefix) : 0;
int preferred_index_format = 0;
@@ -997,7 +1005,7 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
OPT_BOOL(0, "untracked-cache", &untracked_cache,
N_("enable/disable untracked cache")),
OPT_SET_INT(0, "force-untracked-cache", &untracked_cache,
- N_("enable untracked cache without testing the
filesystem"), 2),
+ N_("enable untracked cache without testing the
filesystem"), FORCE_UC),
OPT_END()
};
@@ -1104,10 +1112,10 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
the_index.split_index = NULL;
the_index.cache_changed |= SOMETHING_CHANGED;
}
- if (untracked_cache > 0) {
+ if (untracked_cache > NO_UC) {
struct untracked_cache *uc;
- if (untracked_cache < 2) {
+ if (untracked_cache < FORCE_UC) {
setup_work_tree();
if (!test_if_untracked_cache_is_supported())
return 1;
@@ -1123,7 +1131,7 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
add_untracked_ident(the_index.untracked);
the_index.cache_changed |= UNTRACKED_CHANGED;
fprintf(stderr, _("Untracked cache enabled for '%s'\n"),
get_git_work_tree());
- } else if (!untracked_cache && the_index.untracked) {
+ } else if (untracked_cache == NO_UC && the_index.untracked) {
the_index.untracked = NULL;
the_index.cache_changed |= UNTRACKED_CHANGED;
fprintf(stderr, _("Untracked cache disabled\n"));
--
2.6.3.478.g9f95483.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/8] update-index: move 'uc' var declaration
Signed-off-by: Christian Couder
---
builtin/update-index.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/builtin/update-index.c b/builtin/update-index.c
index ecb685d..21f74b2 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1116,8 +1116,6 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
the_index.cache_changed |= SOMETHING_CHANGED;
}
if (untracked_cache > NO_UC) {
- struct untracked_cache *uc;
-
if (untracked_cache < FORCE_UC) {
setup_work_tree();
if (!test_if_untracked_cache_is_supported())
@@ -1126,7 +1124,7 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
return 0;
}
if (!the_index.untracked) {
- uc = xcalloc(1, sizeof(*uc));
+ struct untracked_cache *uc = xcalloc(1, sizeof(*uc));
strbuf_init(&uc->ident, 100);
uc->exclude_per_dir = ".gitignore";
/* should be the same flags used by git-status */
--
2.6.3.478.g9f95483.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/8] t7063: add tests for core.untrackedCache
Signed-off-by: Christian Couder --- t/t7063-status-untracked-cache.sh | 48 +-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh index 253160a..f0af41c 100755 --- a/t/t7063-status-untracked-cache.sh +++ b/t/t7063-status-untracked-cache.sh @@ -18,6 +18,10 @@ if ! test_have_prereq UNTRACKED_CACHE; then test_done fi +test_expect_success 'core.untrackedCache is unset' ' + test_must_fail git config --get core.untrackedCache +' + test_expect_success 'setup' ' git init worktree && cd worktree && @@ -28,6 +32,11 @@ test_expect_success 'setup' ' git update-index --untracked-cache ' +test_expect_success 'core.untrackedCache is true' ' + UC=$(git config core.untrackedCache) && + test "$UC" = "true" +' + test_expect_success 'untracked cache is empty' ' test-dump-untracked-cache >../actual && cat >../expect <../actual && - cat >../expect <../expect-from-test-dump <../actual && + echo "no untracked cache" >../expect && + test_cmp ../expect ../actual +' + +test_expect_success 'git status does not change anything' ' + git status && + test-dump-untracked-cache >../actual && + test_cmp ../expect ../actual && + UC=$(git config core.untrackedCache) && + test "$UC" = "false" +' + +test_expect_success 'setting core.untrackedCache and using git status creates the cache' ' + git config core.untrackedCache true && + test-dump-untracked-cache >../actual && + test_cmp ../expect ../actual && + git status && + test-dump-untracked-cache >../actual && + test_cmp ../expect-from-test-dump ../actual +' + +test_expect_success 'unsetting core.untrackedCache and using git status removes the cache' ' + git config --unset core.untrackedCache && + test-dump-untracked-cache >../actual && + test_cmp ../expect-from-test-dump ../actual && + git status && + test-dump-untracked-cache >../actual && + test_cmp ../expect ../actual +' + test_done -- 2.6.3.478.g9f95483.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/8] dir: add remove_untracked_cache()
This new function will be used in a later patch.
Signed-off-by: Christian Couder
---
builtin/update-index.c | 3 +--
dir.c | 6 ++
dir.h | 1 +
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 40530b0..e427657 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1126,8 +1126,7 @@ int cmd_update_index(int argc, const char **argv, const
char *prefix)
add_untracked_cache();
fprintf(stderr, _("Untracked cache enabled for '%s'\n"),
get_git_work_tree());
} else if (untracked_cache == NO_UC && the_index.untracked) {
- the_index.untracked = NULL;
- the_index.cache_changed |= UNTRACKED_CHANGED;
+ remove_untracked_cache();
fprintf(stderr, _("Untracked cache disabled\n"));
}
diff --git a/dir.c b/dir.c
index 0f7e293..ffc0286 100644
--- a/dir.c
+++ b/dir.c
@@ -1952,6 +1952,12 @@ void add_untracked_cache(void)
the_index.cache_changed |= UNTRACKED_CHANGED;
}
+void remove_untracked_cache(void)
+{
+ the_index.untracked = NULL;
+ the_index.cache_changed |= UNTRACKED_CHANGED;
+}
+
static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct
*dir,
int base_len,
const struct pathspec
*pathspec)
diff --git a/dir.h b/dir.h
index ee94c76..3e5114d 100644
--- a/dir.h
+++ b/dir.h
@@ -309,4 +309,5 @@ struct untracked_cache *read_untracked_extension(const void
*data, unsigned long
void write_untracked_extension(struct strbuf *out, struct untracked_cache
*untracked);
void add_untracked_ident(struct untracked_cache *);
void add_untracked_cache(void);
+void remove_untracked_cache(void);
#endif
--
2.6.3.478.g9f95483.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: best practices against long git rebase times?
On Mon, Dec 7, 2015 at 11:59 PM, Jeff King wrote: > On Mon, Dec 07, 2015 at 02:56:33PM -0800, Junio C Hamano wrote: > >> Jeff King writes: >> >> > You're computing the patch against the parent for each of those 3000 >> > commits (to get a hash of it to compare against the single hash on the >> > other side). Twelve minutes sounds long, but if you have a really >> > gigantic tree, it might not be unreasonable. >> > >> > You can also try compiling with "make XDL_FAST_HASH=" (i.e., setting >> > that option to the empty string). Last year I found there were some >> > pretty suboptimal corner cases, and you may be hitting one (we should >> > probably turn that option off by default; I got stuck on trying to find >> > a hash that would perform faster and never followed up[1]. >> > >> > I doubt that is your problem, but it's possible). >> > >> > -Peff >> > >> > [1] http://thread.gmane.org/gmane.comp.version-control.git/261638 >> >> I vaguely recall having discussed caching the patch-ids somewhere so >> that this does not have to be done every time. Would such an >> extension help here, I wonder? > > I think you missed John's earlier response which gave several pointers > to such caching schemes. :) Yeah, he also gave very interesting performance numbers. Thanks John! > I used to run with patch-id-caching in my personal fork (I frequently > use "git log --cherry-mark" to see what has made it upstream), but I > haven't for a while. It did make a big difference in speed, but I never > resolved the corner cases around cache invalidation. I will see if I can work on that after I am done with untracked cache... -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] Documentation/git-update-index: add missing opts to synopsis
On Wed, Nov 25, 2015 at 10:30 AM, Christian Couder wrote: > Split index related options should appear in the 'SYNOPSIS' > section. > > These options are already documented in the 'OPTIONS' section. > > Signed-off-by: Christian Couder > --- > This v4 contains only the split-index options and applies on top > of the new master that already contains the untracked-cache options. It looks like this patch has not been applied. Maybe I should have given it a different title to avoid confusion with a previous patch that added [--[no-|force-]untracked-cache] in the SYNOPSIS. > Documentation/git-update-index.txt | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/git-update-index.txt > b/Documentation/git-update-index.txt > index 3df9c26..f4e5a85 100644 > --- a/Documentation/git-update-index.txt > +++ b/Documentation/git-update-index.txt > @@ -17,6 +17,7 @@ SYNOPSIS > [--[no-]assume-unchanged] > [--[no-]skip-worktree] > [--ignore-submodules] > +[--[no-]split-index] > [--[no-|force-]untracked-cache] > [--really-refresh] [--unresolve] [--again | -g] > [--info-only] [--index-info] > -- > 2.6.3.380.g494b52d > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] Git Rev News edition 10
Hi everyone, I'm happy announce that the 10th edition of Git Rev News is now published: https://git.github.io/rev_news/2015/12/09/edition-10/ It was supposed to be published yesterday but I got busy with other things. Sorry about that. Thanks a lot to all the contributors, especially Stefan! Enjoy, Christian, Thomas and Nicola. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/8] update-index: use enum for untracked cache options
On Tue, Dec 8, 2015 at 8:11 PM, Junio C Hamano wrote:
> Christian Couder writes:
>
>> Signed-off-by: Christian Couder
>> ---
>> builtin/update-index.c | 18 +-
>> 1 file changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/builtin/update-index.c b/builtin/update-index.c
>> index 6f6b289..246b3d3 100644
>> --- a/builtin/update-index.c
>> +++ b/builtin/update-index.c
>> @@ -35,6 +35,14 @@ static int mark_skip_worktree_only;
>> #define UNMARK_FLAG 2
>> static struct strbuf mtime_dir = STRBUF_INIT;
>>
>> +/* Untracked cache mode */
>> +enum uc_mode {
>> + UNDEF_UC = -1,
>> + NO_UC = 0,
>> + UC,
>> + FORCE_UC
>> +};
>> +
>
> With these, the code is much easier to read than with the mystery
> constants, but did you consider making UC_ a common prefix for
> further ease-of-reading? E.g.
>
> UC_UNSPECIFIED
> UC_DONTUSE
> UC_USE
> UC_FORCE
>
> or something?
Ok, I will use what you suggest.
Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] update-index: add untracked cache notifications
On Tue, Dec 8, 2015 at 8:03 PM, Junio C Hamano wrote: > Christian Couder writes: > >> Doing: >> >> cd /tmp >> git --git-dir=/git/somewhere/else/.git update-index --untracked-cache >> >> doesn't work how one would expect. It hardcodes "/tmp" as the directory >> that "works" into the index, so if you use the working tree, you'll >> never use the untracked cache. > > I think your "expectation" needs to be more explicitly spelled out. > > "git -C /tmp --git-dir=/git/somewhere/else/.git" is a valid way to > use that repository you have in somewhere else to track things under > /tmp/ (as you are only passing GIT_DIR but not GIT_WORK_TREE, the > cwd, i.e. /tmp, is the root level of the working tree), and for such > a usage, the above command works as expected. Perhaps > > Attempting to flip the untracked-cache feature on for a random index > file with > >cd /random/unrelated/place >git --git-dir=/somewhere/else/.git update-index --untracked-cache > > would not work as you might expect. Because flipping the > feature on in the index also records the location of the > corresponding working tree (/random/unrelated/place in the above > example), when the index is subsequently used to keep track of > files in the working tree in /somewhere/else, the feature is > disabled. > > may be an improvement. Yeah, I agree that it is better. I have included your explanations in the next version I will send. Thanks. > The index already implicitly records where the working tree was and > that is not limited to untracked-cache option. For example, if you > have your repository and its working tree in /git/somewhere/else, > which does not have a path X, then doing: > > cd /tmp && >tmp/X > git --git-dir=/git/somewhere/else/.git update-index --add X > > would store X taken from /tmp in the index, so subsequent use of the > index "knows" about X that was taken outside /git/somewhere/else/ > after the above command finishes and the subsequent use is made > without the --git-dir parameter, e.g. > > cd /git/somewhere/else/ && git diff-index --cached HEAD' > > would say that you added X, even though /git/somewhere/else/ may not > have that X at all. And this is not limited to update-index, > either. You can temporarily use --git-dir with "git add X" and the > result would persist the same way in the index. > > I think the moral of the story is that you are not expected to > randomly use git-dir and git-work-tree to point at different places > without knowing what you are doing, and we may need a way to help > people understand what is going on when it is done by a mistake. Yeah, I agree, and I think displaying more information might be a good way. > The patch to show result from xgetcwd() and get_git_work_tree() may > be a step in the right direction, but if the user is not doing > anything fancy, "Testing mtime in /long/path/to/the/directory" would > be irritatingly verbose. Yeah, but after this series only "--test-untracked-cache" does any testing, and the 10 seconds time it takes are probably more irritating than its output. > I wonder if it is easy to tell when the user is doing such an > unnatural thing. Off the top of my head, when the working tree is > anywhere other than one level above $GIT_DIR, the user is doing > something unusual; I do not know if there are cases where the user > is doing something unnatural if $GIT_WORK_TREE is one level above > $GIT_DIR, though. Yeah, it could only print a warning in case something unusual is done. I am not sure it is worth it though. Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/8] dir: add add_untracked_cache()
On Wed, Dec 9, 2015 at 8:37 AM, Torsten Bögershausen wrote: > On 08.12.15 18:15, Christian Couder wrote: >> This new function will be used in a later patch. > May be > Factor out code into add_untracked_cache(), which will be used in the next > commit. Thanks for the suggestion. I think I will put something like this: Factor out code into add_untracked_cache(), which will be used in a later commit. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
