* [PATCH 4/8] dir.c: git-status --ignored: don't list empty directories as ignored
From: Karsten Blees @ 2013-03-18 20:28 UTC (permalink / raw)
To: Git List
Cc: Junio C Hamano, Erik Faye-Lund, Ramkumar Ramachandra, Robert Zeh,
Duy Nguyen, Antoine Pelisse, Adam Spiers
In-Reply-To: <514775FA.9080304@gmail.com>
'git-status --ignored' lists empty untracked directories as ignored, even
though they don't have any ignored files.
When checking if a directory is already listed as untracked (i.e. shouldn't
be listed as ignored as well), don't assume that the dirctory has only
ignored files if it doesn't have untracked files, as the directory may be
empty.
Signed-off-by: Karsten Blees <blees@dcon.de>
---
dir.c | 17 ++++++++---------
t/t7061-wtstatus-ignore.sh | 26 ++++++++++++++++++++++++--
2 files changed, 32 insertions(+), 11 deletions(-)
diff --git a/dir.c b/dir.c
index fd1f088..64457db 100644
--- a/dir.c
+++ b/dir.c
@@ -1071,17 +1071,16 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,
/*
* We are looking for ignored files and our directory is not ignored,
- * check if it contains only ignored files
+ * check if it contains untracked files (i.e. is listed as untracked)
*/
if ((dir->flags & DIR_SHOW_IGNORED) && !exclude) {
- int ignored;
- dir->flags &= ~DIR_SHOW_IGNORED;
- dir->flags |= DIR_HIDE_EMPTY_DIRECTORIES;
- ignored = read_directory_recursive(dir, dirname, len, 1, simplify);
- dir->flags &= ~DIR_HIDE_EMPTY_DIRECTORIES;
- dir->flags |= DIR_SHOW_IGNORED;
-
- return ignored ? ignore_directory : show_directory;
+ int untracked;
+ dir->flags &= ~(DIR_SHOW_IGNORED|DIR_SHOW_OTHER_DIRECTORIES);
+ untracked = read_directory_recursive(dir, dirname, len, 1, simplify);
+ dir->flags |= DIR_SHOW_IGNORED|DIR_SHOW_OTHER_DIRECTORIES;
+
+ if (untracked)
+ return ignore_directory;
}
if (!(dir->flags & DIR_SHOW_IGNORED) &&
!(dir->flags & DIR_HIDE_EMPTY_DIRECTORIES))
diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh
index 28b7d95..6171a49 100755
--- a/t/t7061-wtstatus-ignore.sh
+++ b/t/t7061-wtstatus-ignore.sh
@@ -64,13 +64,35 @@ cat >expected <<\EOF
?? .gitignore
?? actual
?? expected
-!! untracked-ignored/
EOF
-test_expect_success 'status untracked directory with ignored files with --ignore' '
+test_expect_success 'status empty untracked directory with --ignore' '
rm -rf ignored &&
mkdir untracked-ignored &&
mkdir untracked-ignored/test &&
+ git status --porcelain --ignored >actual &&
+ test_cmp expected actual
+'
+
+cat >expected <<\EOF
+?? .gitignore
+?? actual
+?? expected
+EOF
+
+test_expect_success 'status empty untracked directory with --ignore -u' '
+ git status --porcelain --ignored -u >actual &&
+ test_cmp expected actual
+'
+
+cat >expected <<\EOF
+?? .gitignore
+?? actual
+?? expected
+!! untracked-ignored/
+EOF
+
+test_expect_success 'status untracked directory with ignored files with --ignore' '
: >untracked-ignored/ignored &&
: >untracked-ignored/test/ignored &&
git status --porcelain --ignored >actual &&
--
1.8.1.2.8021.g7e51819
^ permalink raw reply related
* [PATCH 8/8] dir.c: git-status: avoid is_excluded checks for tracked files
From: Karsten Blees @ 2013-03-18 20:29 UTC (permalink / raw)
To: Git List
Cc: Junio C Hamano, Erik Faye-Lund, Ramkumar Ramachandra, Robert Zeh,
Duy Nguyen, Antoine Pelisse, Adam Spiers
In-Reply-To: <514775FA.9080304@gmail.com>
Checking if a file is in the index is much faster (hashtable lookup) than
checking if the file is excluded (linear search over exclude patterns).
Skip is_excluded checks for files: move the cache_name_exists check from
treat_file to treat_one_path and return early if the file is tracked.
This can safely be done as all other code paths also return path_ignored
for tracked files, and dir_add_ignored skips tracked files as well.
There's just one line left in treat_file, so move this to treat_one_path
as well.
Here's some performance data for git-status from the linux and WebKit
repos (best of 10 runs on a Debian Linux on SSD, core.preloadIndex=true):
| status | status --ignored
| linux | WebKit | linux | WebKit
-------+-------+--------+-------+---------
before | 0.218 | 1.583 | 0.321 | 2.579
after | 0.156 | 0.988 | 0.202 | 1.279
gain | 1.397 | 1.602 | 1.589 | 2.016
Signed-off-by: Karsten Blees <blees@dcon.de>
---
dir.c | 38 +++++++++++---------------------------
1 file changed, 11 insertions(+), 27 deletions(-)
diff --git a/dir.c b/dir.c
index 086a169..c159000 100644
--- a/dir.c
+++ b/dir.c
@@ -1026,28 +1026,6 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,
}
/*
- * Decide what to do when we find a file while traversing the
- * filesystem. Mostly two cases:
- *
- * 1. We are looking for ignored files
- * (a) File is ignored, include it
- * (b) File is in ignored path, include it
- * (c) File is not ignored, exclude it
- *
- * 2. Other scenarios, include the file if not excluded
- *
- * Return 1 for exclude, 0 for include.
- */
-static int treat_file(struct dir_struct *dir, struct strbuf *path, int exclude)
-{
- /* Always exclude indexed files */
- if (index_name_exists(&the_index, path->buf, path->len, ignore_case))
- return 1;
-
- return exclude == !(dir->flags & DIR_SHOW_IGNORED);
-}
-
-/*
* This is an inexact early pruning of any recursive directory
* reading - if the path cannot possibly be in the pathspec,
* return true, and we'll skip it early.
@@ -1170,7 +1148,16 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
const struct path_simplify *simplify,
int dtype, struct dirent *de)
{
- int exclude = is_excluded(dir, path->buf, &dtype);
+ int exclude;
+ if (dtype == DT_UNKNOWN)
+ dtype = get_dtype(de, path->buf, path->len);
+
+ /* Always exclude indexed files */
+ if (dtype != DT_DIR &&
+ cache_name_exists(path->buf, path->len, ignore_case))
+ return path_ignored;
+
+ exclude = is_excluded(dir, path->buf, &dtype);
if (exclude && (dir->flags & DIR_COLLECT_IGNORED)
&& exclude_matches_pathspec(path->buf, path->len, simplify))
dir_add_ignored(dir, path->buf, path->len);
@@ -1182,9 +1169,6 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
if (exclude && !(dir->flags & DIR_SHOW_IGNORED))
return path_ignored;
- if (dtype == DT_UNKNOWN)
- dtype = get_dtype(de, path->buf, path->len);
-
switch (dtype) {
default:
return path_ignored;
@@ -1201,7 +1185,7 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
break;
case DT_REG:
case DT_LNK:
- if (treat_file(dir, path, exclude))
+ if (exclude == !(dir->flags & DIR_SHOW_IGNORED))
return path_ignored;
break;
}
--
1.8.1.2.8021.g7e51819
^ permalink raw reply related
* [PATCH 7/8] dir.c: replace is_path_excluded with now equivalent is_excluded API
From: Karsten Blees @ 2013-03-18 20:29 UTC (permalink / raw)
To: Git List
Cc: Junio C Hamano, Erik Faye-Lund, Ramkumar Ramachandra, Robert Zeh,
Duy Nguyen, Antoine Pelisse, Adam Spiers
In-Reply-To: <514775FA.9080304@gmail.com>
Signed-off-by: Karsten Blees <blees@dcon.de>
---
builtin/add.c | 5 +---
builtin/check-ignore.c | 6 +---
builtin/ls-files.c | 15 +++-------
dir.c | 79 ++++----------------------------------------------
dir.h | 16 ++--------
unpack-trees.c | 10 +------
unpack-trees.h | 1 -
7 files changed, 16 insertions(+), 116 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index ab1c9e8..06f365d 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -444,9 +444,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
if (pathspec) {
int i;
- struct path_exclude_check check;
- path_exclude_check_init(&check, &dir);
if (!seen)
seen = find_pathspecs_matching_against_index(pathspec);
for (i = 0; pathspec[i]; i++) {
@@ -454,7 +452,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
&& !file_exists(pathspec[i])) {
if (ignore_missing) {
int dtype = DT_UNKNOWN;
- if (is_path_excluded(&check, pathspec[i], -1, &dtype))
+ if (is_excluded(&dir, pathspec[i], &dtype))
dir_add_ignored(&dir, pathspec[i], strlen(pathspec[i]));
} else
die(_("pathspec '%s' did not match any files"),
@@ -462,7 +460,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)
}
}
free(seen);
- path_exclude_check_clear(&check);
}
plug_bulk_checkin();
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 0240f99..7388346 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -59,7 +59,6 @@ static int check_ignore(const char *prefix, const char **pathspec)
const char *path, *full_path;
char *seen;
int num_ignored = 0, dtype = DT_UNKNOWN, i;
- struct path_exclude_check check;
struct exclude *exclude;
/* read_cache() is only necessary so we can watch out for submodules. */
@@ -76,7 +75,6 @@ static int check_ignore(const char *prefix, const char **pathspec)
return 0;
}
- path_exclude_check_init(&check, &dir);
/*
* look for pathspecs matching entries in the index, since these
* should not be ignored, in order to be consistent with
@@ -90,8 +88,7 @@ static int check_ignore(const char *prefix, const char **pathspec)
full_path = check_path_for_gitlink(full_path);
die_if_path_beyond_symlink(full_path, prefix);
if (!seen[i]) {
- exclude = last_exclude_matching_path(&check, full_path,
- -1, &dtype);
+ exclude = last_exclude_matching(&dir, full_path, &dtype);
if (exclude) {
if (!quiet)
output_exclude(path, exclude);
@@ -101,7 +98,6 @@ static int check_ignore(const char *prefix, const char **pathspec)
}
free(seen);
clear_directory(&dir);
- path_exclude_check_clear(&check);
return num_ignored;
}
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 175e6e3..2202072 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -201,19 +201,15 @@ static void show_ru_info(void)
}
}
-static int ce_excluded(struct path_exclude_check *check, struct cache_entry *ce)
+static int ce_excluded(struct dir_struct *dir, struct cache_entry *ce)
{
int dtype = ce_to_dtype(ce);
- return is_path_excluded(check, ce->name, ce_namelen(ce), &dtype);
+ return is_excluded(dir, ce->name, &dtype);
}
static void show_files(struct dir_struct *dir)
{
int i;
- struct path_exclude_check check;
-
- if ((dir->flags & DIR_SHOW_IGNORED))
- path_exclude_check_init(&check, dir);
/* For cached/deleted files we don't need to even do the readdir */
if (show_others || show_killed) {
@@ -227,7 +223,7 @@ static void show_files(struct dir_struct *dir)
for (i = 0; i < active_nr; i++) {
struct cache_entry *ce = active_cache[i];
if ((dir->flags & DIR_SHOW_IGNORED) &&
- !ce_excluded(&check, ce))
+ !ce_excluded(dir, ce))
continue;
if (show_unmerged && !ce_stage(ce))
continue;
@@ -243,7 +239,7 @@ static void show_files(struct dir_struct *dir)
struct stat st;
int err;
if ((dir->flags & DIR_SHOW_IGNORED) &&
- !ce_excluded(&check, ce))
+ !ce_excluded(dir, ce))
continue;
if (ce->ce_flags & CE_UPDATE)
continue;
@@ -256,9 +252,6 @@ static void show_files(struct dir_struct *dir)
show_ce_entry(tag_modified, ce);
}
}
-
- if ((dir->flags & DIR_SHOW_IGNORED))
- path_exclude_check_clear(&check);
}
/*
diff --git a/dir.c b/dir.c
index 16fee2c..086a169 100644
--- a/dir.c
+++ b/dir.c
@@ -799,7 +799,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
* Returns the exclude_list element which matched, or NULL for
* undecided.
*/
-static struct exclude *last_exclude_matching(struct dir_struct *dir,
+struct exclude *last_exclude_matching(struct dir_struct *dir,
const char *pathname,
int *dtype_p)
{
@@ -819,7 +819,7 @@ static struct exclude *last_exclude_matching(struct dir_struct *dir,
* scans all exclude lists to determine whether pathname is excluded.
* Returns 1 if true, otherwise 0.
*/
-static int is_excluded(struct dir_struct *dir, const char *pathname, int *dtype_p)
+int is_excluded(struct dir_struct *dir, const char *pathname, int *dtype_p)
{
struct exclude *exclude =
last_exclude_matching(dir, pathname, dtype_p);
@@ -828,47 +828,6 @@ static int is_excluded(struct dir_struct *dir, const char *pathname, int *dtype_
return 0;
}
-void path_exclude_check_init(struct path_exclude_check *check,
- struct dir_struct *dir)
-{
- check->dir = dir;
-}
-
-void path_exclude_check_clear(struct path_exclude_check *check)
-{
-}
-
-/*
- * For each subdirectory in name, starting with the top-most, checks
- * to see if that subdirectory is excluded, and if so, returns the
- * corresponding exclude structure. Otherwise, checks whether name
- * itself (which is presumably a file) is excluded.
- *
- * A path to a directory known to be excluded is left in check->path to
- * optimize for repeated checks for files in the same excluded directory.
- */
-struct exclude *last_exclude_matching_path(struct path_exclude_check *check,
- const char *name, int namelen,
- int *dtype)
-{
- return last_exclude_matching(check->dir, name, dtype);
-}
-
-/*
- * Is this name excluded? This is for a caller like show_files() that
- * do not honor directory hierarchy and iterate through paths that are
- * possibly in an ignored directory.
- */
-int is_path_excluded(struct path_exclude_check *check,
- const char *name, int namelen, int *dtype)
-{
- struct exclude *exclude =
- last_exclude_matching_path(check, name, namelen, dtype);
- if (exclude)
- return exclude->flags & EXC_FLAG_NEGATIVE ? 0 : 1;
- return 0;
-}
-
static struct dir_entry *dir_entry_new(const char *pathname, int len)
{
struct dir_entry *ent;
@@ -1045,15 +1004,6 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,
/* This is the "show_other_directories" case */
- /* might be a sub directory in an excluded directory */
- if (!exclude) {
- struct path_exclude_check check;
- int dt = DT_DIR;
- path_exclude_check_init(&check, dir);
- exclude = is_path_excluded(&check, dirname, len, &dt);
- path_exclude_check_clear(&check);
- }
-
/*
* We are looking for ignored files and our directory is not ignored,
* check if it contains untracked files (i.e. is listed as untracked)
@@ -1088,27 +1038,13 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,
*
* Return 1 for exclude, 0 for include.
*/
-static int treat_file(struct dir_struct *dir, struct strbuf *path, int exclude, int *dtype)
+static int treat_file(struct dir_struct *dir, struct strbuf *path, int exclude)
{
- struct path_exclude_check check;
- int exclude_file = 0;
-
/* Always exclude indexed files */
if (index_name_exists(&the_index, path->buf, path->len, ignore_case))
return 1;
- if (exclude)
- exclude_file = !(dir->flags & DIR_SHOW_IGNORED);
- else if (dir->flags & DIR_SHOW_IGNORED) {
- path_exclude_check_init(&check, dir);
-
- if (!is_path_excluded(&check, path->buf, path->len, dtype))
- exclude_file = 1;
-
- path_exclude_check_clear(&check);
- }
-
- return exclude_file;
+ return exclude == !(dir->flags & DIR_SHOW_IGNORED);
}
/*
@@ -1265,12 +1201,9 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
break;
case DT_REG:
case DT_LNK:
- switch (treat_file(dir, path, exclude, &dtype)) {
- case 1:
+ if (treat_file(dir, path, exclude))
return path_ignored;
- default:
- break;
- }
+ break;
}
return path_handled;
}
diff --git a/dir.h b/dir.h
index cd166d0..bfe726e 100644
--- a/dir.h
+++ b/dir.h
@@ -151,20 +151,10 @@ extern int match_pathname(const char *, int,
const char *, int,
const char *, int, int, int);
-/*
- * The is_excluded() API is meant for callers that check each level of leading
- * directory hierarchies with is_excluded() to avoid recursing into excluded
- * directories. Callers that do not do so should use this API instead.
- */
-struct path_exclude_check {
- struct dir_struct *dir;
-};
-extern void path_exclude_check_init(struct path_exclude_check *, struct dir_struct *);
-extern void path_exclude_check_clear(struct path_exclude_check *);
-extern struct exclude *last_exclude_matching_path(struct path_exclude_check *, const char *,
- int namelen, int *dtype);
-extern int is_path_excluded(struct path_exclude_check *, const char *, int namelen, int *dtype);
+extern struct exclude *last_exclude_matching(struct dir_struct *dir,
+ const char *name, int *dtype);
+extern int is_excluded(struct dir_struct *dir, const char *name, int *dtype);
extern struct exclude_list *add_exclude_list(struct dir_struct *dir,
int group_type, const char *src);
diff --git a/unpack-trees.c b/unpack-trees.c
index 09e53df..ede4299 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1026,10 +1026,6 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
o->el = ⪙
}
- if (o->dir) {
- o->path_exclude_check = xmalloc(sizeof(struct path_exclude_check));
- path_exclude_check_init(o->path_exclude_check, o->dir);
- }
memset(&o->result, 0, sizeof(o->result));
o->result.initialized = 1;
o->result.timestamp.sec = o->src_index->timestamp.sec;
@@ -1155,10 +1151,6 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
done:
clear_exclude_list(&el);
- if (o->path_exclude_check) {
- path_exclude_check_clear(o->path_exclude_check);
- free(o->path_exclude_check);
- }
return ret;
return_failed:
@@ -1375,7 +1367,7 @@ static int check_ok_to_remove(const char *name, int len, int dtype,
return 0;
if (o->dir &&
- is_path_excluded(o->path_exclude_check, name, -1, &dtype))
+ is_excluded(o->dir, name, &dtype))
/*
* ce->name is explicitly excluded, so it is Ok to
* overwrite it.
diff --git a/unpack-trees.h b/unpack-trees.h
index ec74a9f..5e432f5 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -52,7 +52,6 @@ struct unpack_trees_options {
const char *prefix;
int cache_bottom;
struct dir_struct *dir;
- struct path_exclude_check *path_exclude_check;
struct pathspec *pathspec;
merge_fn_t fn;
const char *msgs[NB_UNPACK_TREES_ERROR_TYPES];
--
1.8.1.2.8021.g7e51819
^ permalink raw reply related
* [PATCH 6/8] dir.c: unify is_excluded and is_path_excluded APIs
From: Karsten Blees @ 2013-03-18 20:29 UTC (permalink / raw)
To: Git List
Cc: Junio C Hamano, Erik Faye-Lund, Ramkumar Ramachandra, Robert Zeh,
Duy Nguyen, Antoine Pelisse, Adam Spiers
In-Reply-To: <514775FA.9080304@gmail.com>
The is_excluded and is_path_excluded APIs are very similar, except for a
few noteworthy differences:
is_excluded doesn't handle ignored directories, results for paths within
ignored directories are incorrect. This is probably based on the premise
that recursive directory scans should stop at ignored directories, which
is no longer true (in certain cases, read_directory_recursive currently
calls is_excluded *and* is_path_excluded to get correct ignored state).
is_excluded caches parsed .gitignore files of the last directory in struct
dir_struct. If the directory changes, it finds a common parent directory
and is very careful to drop only as much state as necessary. On the other
hand, is_excluded will also read and parse .gitignore files in already
ignored directories, which are completely irrelevant.
is_path_excluded correctly handles ignored directories by checking if any
component in the path is excluded. As it uses is_excluded internally, this
unfortunately forces is_excluded to drop and re-read all .gitignore files,
as there is no common parent directory for the root dir.
is_path_excluded tracks state in a separate struct path_exclude_check,
which is essentially a wrapper of dir_struct with two more fields. However,
as is_path_excluded also modifies dir_struct, it is not possible to e.g.
use multiple path_exclude_check structures with the same dir_struct in
parallel. The additional structure just unnecessarily complicates the API.
Teach is_excluded / prep_exclude about ignored directories: whenever
entering a new directory, first check if the entire directory is excluded.
Remember the excluded state in dir_struct. Don't traverse into already
ignored directories (i.e. don't read irrelevant .gitignore files).
Directories could also be excluded by exclude patterns specified on the
command line or .git/info/exclude, so we cannot simply skip prep_exclude
entirely if there's no .gitignore file name (dir_struct.exclude_per_dir).
Move this check to just before acutally reading the file.
is_path_excluded is now equivalent to is_excluded, so we can simply
redirect to it (the public API is cleaned up in the next patch).
The performance impact of the additional ignored check per directory is
hardly noticeable when reading directories recursively (e.g. 'git status').
However, performance of git commands using the is_path_excluded API (e.g.
'git ls-files --cached --ignored --exclude-standard') is greatly improved
as this no longer re-reads .gitignore files on each call.
Here's some performance data from the linux and WebKit repos (best of 10
runs on a Debian Linux on SSD, core.preloadIndex=true):
| ls-files -ci | status | status --ignored
| linux | WebKit | linux | WebKit | linux | WebKit
-------+-------+--------+-------+--------+-------+---------
before | 0.506 | 6.539 | 0.212 | 1.555 | 0.323 | 2.541
after | 0.080 | 1.191 | 0.218 | 1.583 | 0.321 | 2.579
gain | 6.325 | 5.490 | 0.972 | 0.982 | 1.006 | 0.985
Signed-off-by: Karsten Blees <blees@dcon.de>
---
dir.c | 106 ++++++++++++++++++++++++++----------------------------------------
dir.h | 6 ++--
2 files changed, 45 insertions(+), 67 deletions(-)
diff --git a/dir.c b/dir.c
index 417feaa..16fee2c 100644
--- a/dir.c
+++ b/dir.c
@@ -710,10 +710,6 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
struct exclude_stack *stk = NULL;
int current;
- if ((!dir->exclude_per_dir) ||
- (baselen + strlen(dir->exclude_per_dir) >= PATH_MAX))
- return; /* too long a path -- ignore */
-
group = &dir->exclude_list_group[EXC_DIRS];
/* Pop the exclude lists from the EXCL_DIRS exclude_list_group
@@ -725,12 +721,17 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
break;
el = &group->el[dir->exclude_stack->exclude_ix];
dir->exclude_stack = stk->prev;
+ dir->exclude = NULL;
free((char *)el->src); /* see strdup() below */
clear_exclude_list(el);
free(stk);
group->nr--;
}
+ /* Skip traversing into sub directories if the parent is excluded */
+ if (dir->exclude)
+ return;
+
/* Read from the parent directories and push them down. */
current = stk ? stk->baselen : -1;
while (current < baselen) {
@@ -749,22 +750,43 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
}
stk->prev = dir->exclude_stack;
stk->baselen = cp - base;
+ stk->exclude_ix = group->nr;
+ el = add_exclude_list(dir, EXC_DIRS, NULL);
memcpy(dir->basebuf + current, base + current,
stk->baselen - current);
- strcpy(dir->basebuf + stk->baselen, dir->exclude_per_dir);
- /*
- * dir->basebuf gets reused by the traversal, but we
- * need fname to remain unchanged to ensure the src
- * member of each struct exclude correctly
- * back-references its source file. Other invocations
- * of add_exclude_list provide stable strings, so we
- * strdup() and free() here in the caller.
- */
- el = add_exclude_list(dir, EXC_DIRS, strdup(dir->basebuf));
- stk->exclude_ix = group->nr - 1;
- add_excludes_from_file_to_list(dir->basebuf,
- dir->basebuf, stk->baselen,
- el, 1);
+
+ /* Abort if the directory is excluded */
+ if (stk->baselen) {
+ int dt = DT_DIR;
+ dir->basebuf[stk->baselen - 1] = 0;
+ dir->exclude = last_exclude_matching_from_lists(dir,
+ dir->basebuf, stk->baselen - 1,
+ dir->basebuf + current, &dt);
+ dir->basebuf[stk->baselen - 1] = '/';
+ if (dir->exclude) {
+ dir->basebuf[stk->baselen] = 0;
+ dir->exclude_stack = stk;
+ return;
+ }
+ }
+
+ /* Try to read per-directory file unless path is too long */
+ if (dir->exclude_per_dir &&
+ stk->baselen + strlen(dir->exclude_per_dir) < PATH_MAX) {
+ strcpy(dir->basebuf + stk->baselen,
+ dir->exclude_per_dir);
+ /*
+ * dir->basebuf gets reused by the traversal, but we
+ * need fname to remain unchanged to ensure the src
+ * member of each struct exclude correctly
+ * back-references its source file. Other invocations
+ * of add_exclude_list provide stable strings, so we
+ * strdup() and free() here in the caller.
+ */
+ el->src = strdup(dir->basebuf);
+ add_excludes_from_file_to_list(dir->basebuf,
+ dir->basebuf, stk->baselen, el, 1);
+ }
dir->exclude_stack = stk;
current = stk->baselen;
}
@@ -786,6 +808,8 @@ static struct exclude *last_exclude_matching(struct dir_struct *dir,
basename = (basename) ? basename+1 : pathname;
prep_exclude(dir, pathname, basename-pathname);
+ if (dir->exclude)
+ return dir->exclude;
return last_exclude_matching_from_lists(dir, pathname, pathlen,
basename, dtype_p);
}
@@ -808,13 +832,10 @@ void path_exclude_check_init(struct path_exclude_check *check,
struct dir_struct *dir)
{
check->dir = dir;
- check->exclude = NULL;
- strbuf_init(&check->path, 256);
}
void path_exclude_check_clear(struct path_exclude_check *check)
{
- strbuf_release(&check->path);
}
/*
@@ -830,49 +851,6 @@ struct exclude *last_exclude_matching_path(struct path_exclude_check *check,
const char *name, int namelen,
int *dtype)
{
- int i;
- struct strbuf *path = &check->path;
- struct exclude *exclude;
-
- /*
- * we allow the caller to pass namelen as an optimization; it
- * must match the length of the name, as we eventually call
- * is_excluded() on the whole name string.
- */
- if (namelen < 0)
- namelen = strlen(name);
-
- /*
- * If path is non-empty, and name is equal to path or a
- * subdirectory of path, name should be excluded, because
- * it's inside a directory which is already known to be
- * excluded and was previously left in check->path.
- */
- if (path->len &&
- path->len <= namelen &&
- !memcmp(name, path->buf, path->len) &&
- (!name[path->len] || name[path->len] == '/'))
- return check->exclude;
-
- strbuf_setlen(path, 0);
- for (i = 0; name[i]; i++) {
- int ch = name[i];
-
- if (ch == '/') {
- int dt = DT_DIR;
- exclude = last_exclude_matching(check->dir,
- path->buf, &dt);
- if (exclude) {
- check->exclude = exclude;
- return exclude;
- }
- }
- strbuf_addch(path, ch);
- }
-
- /* An entry in the index; cannot be a directory with subentries */
- strbuf_setlen(path, 0);
-
return last_exclude_matching(check->dir, name, dtype);
}
diff --git a/dir.h b/dir.h
index c3eb4b5..cd166d0 100644
--- a/dir.h
+++ b/dir.h
@@ -110,9 +110,11 @@ struct dir_struct {
*
* exclude_stack points to the top of the exclude_stack, and
* basebuf contains the full path to the current
- * (sub)directory in the traversal.
+ * (sub)directory in the traversal. Exclude points to the
+ * matching exclude struct if the directory is excluded.
*/
struct exclude_stack *exclude_stack;
+ struct exclude *exclude;
char basebuf[PATH_MAX];
};
@@ -156,8 +158,6 @@ extern int match_pathname(const char *, int,
*/
struct path_exclude_check {
struct dir_struct *dir;
- struct exclude *exclude;
- struct strbuf path;
};
extern void path_exclude_check_init(struct path_exclude_check *, struct dir_struct *);
extern void path_exclude_check_clear(struct path_exclude_check *);
--
1.8.1.2.8021.g7e51819
^ permalink raw reply related
* [PATCH 5/8] dir.c: move prep_exclude and factor out parts of last_exclude_matching
From: Karsten Blees @ 2013-03-18 20:28 UTC (permalink / raw)
To: Git List
Cc: Junio C Hamano, Erik Faye-Lund, Ramkumar Ramachandra, Robert Zeh,
Duy Nguyen, Antoine Pelisse, Adam Spiers
In-Reply-To: <514775FA.9080304@gmail.com>
Move code around in preparation for the next patch.
Signed-off-by: Karsten Blees <blees@dcon.de>
---
dir.c | 181 ++++++++++++++++++++++++++++++++++--------------------------------
1 file changed, 94 insertions(+), 87 deletions(-)
diff --git a/dir.c b/dir.c
index 64457db..417feaa 100644
--- a/dir.c
+++ b/dir.c
@@ -549,78 +549,6 @@ void add_excludes_from_file(struct dir_struct *dir, const char *fname)
die("cannot use %s as an exclude file", fname);
}
-/*
- * Loads the per-directory exclude list for the substring of base
- * which has a char length of baselen.
- */
-static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
-{
- struct exclude_list_group *group;
- struct exclude_list *el;
- struct exclude_stack *stk = NULL;
- int current;
-
- if ((!dir->exclude_per_dir) ||
- (baselen + strlen(dir->exclude_per_dir) >= PATH_MAX))
- return; /* too long a path -- ignore */
-
- group = &dir->exclude_list_group[EXC_DIRS];
-
- /* Pop the exclude lists from the EXCL_DIRS exclude_list_group
- * which originate from directories not in the prefix of the
- * path being checked. */
- while ((stk = dir->exclude_stack) != NULL) {
- if (stk->baselen <= baselen &&
- !strncmp(dir->basebuf, base, stk->baselen))
- break;
- el = &group->el[dir->exclude_stack->exclude_ix];
- dir->exclude_stack = stk->prev;
- free((char *)el->src); /* see strdup() below */
- clear_exclude_list(el);
- free(stk);
- group->nr--;
- }
-
- /* Read from the parent directories and push them down. */
- current = stk ? stk->baselen : -1;
- while (current < baselen) {
- struct exclude_stack *stk = xcalloc(1, sizeof(*stk));
- const char *cp;
-
- if (current < 0) {
- cp = base;
- current = 0;
- }
- else {
- cp = strchr(base + current + 1, '/');
- if (!cp)
- die("oops in prep_exclude");
- cp++;
- }
- stk->prev = dir->exclude_stack;
- stk->baselen = cp - base;
- memcpy(dir->basebuf + current, base + current,
- stk->baselen - current);
- strcpy(dir->basebuf + stk->baselen, dir->exclude_per_dir);
- /*
- * dir->basebuf gets reused by the traversal, but we
- * need fname to remain unchanged to ensure the src
- * member of each struct exclude correctly
- * back-references its source file. Other invocations
- * of add_exclude_list provide stable strings, so we
- * strdup() and free() here in the caller.
- */
- el = add_exclude_list(dir, EXC_DIRS, strdup(dir->basebuf));
- stk->exclude_ix = group->nr - 1;
- add_excludes_from_file_to_list(dir->basebuf,
- dir->basebuf, stk->baselen,
- el, 1);
- dir->exclude_stack = stk;
- current = stk->baselen;
- }
- dir->basebuf[baselen] = '\0';
-}
-
int match_basename(const char *basename, int basenamelen,
const char *pattern, int prefix, int patternlen,
int flags)
@@ -751,25 +679,13 @@ int is_excluded_from_list(const char *pathname,
return -1; /* undecided */
}
-/*
- * Loads the exclude lists for the directory containing pathname, then
- * scans all exclude lists to determine whether pathname is excluded.
- * Returns the exclude_list element which matched, or NULL for
- * undecided.
- */
-static struct exclude *last_exclude_matching(struct dir_struct *dir,
- const char *pathname,
- int *dtype_p)
+static struct exclude *last_exclude_matching_from_lists(struct dir_struct *dir,
+ const char *pathname, int pathlen, const char *basename,
+ int *dtype_p)
{
- int pathlen = strlen(pathname);
int i, j;
struct exclude_list_group *group;
struct exclude *exclude;
- const char *basename = strrchr(pathname, '/');
- basename = (basename) ? basename+1 : pathname;
-
- prep_exclude(dir, pathname, basename-pathname);
-
for (i = EXC_CMDL; i <= EXC_FILE; i++) {
group = &dir->exclude_list_group[i];
for (j = group->nr - 1; j >= 0; j--) {
@@ -781,6 +697,97 @@ static struct exclude *last_exclude_matching(struct dir_struct *dir,
}
}
return NULL;
+};
+
+/*
+ * Loads the per-directory exclude list for the substring of base
+ * which has a char length of baselen.
+ */
+static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
+{
+ struct exclude_list_group *group;
+ struct exclude_list *el;
+ struct exclude_stack *stk = NULL;
+ int current;
+
+ if ((!dir->exclude_per_dir) ||
+ (baselen + strlen(dir->exclude_per_dir) >= PATH_MAX))
+ return; /* too long a path -- ignore */
+
+ group = &dir->exclude_list_group[EXC_DIRS];
+
+ /* Pop the exclude lists from the EXCL_DIRS exclude_list_group
+ * which originate from directories not in the prefix of the
+ * path being checked. */
+ while ((stk = dir->exclude_stack) != NULL) {
+ if (stk->baselen <= baselen &&
+ !strncmp(dir->basebuf, base, stk->baselen))
+ break;
+ el = &group->el[dir->exclude_stack->exclude_ix];
+ dir->exclude_stack = stk->prev;
+ free((char *)el->src); /* see strdup() below */
+ clear_exclude_list(el);
+ free(stk);
+ group->nr--;
+ }
+
+ /* Read from the parent directories and push them down. */
+ current = stk ? stk->baselen : -1;
+ while (current < baselen) {
+ struct exclude_stack *stk = xcalloc(1, sizeof(*stk));
+ const char *cp;
+
+ if (current < 0) {
+ cp = base;
+ current = 0;
+ }
+ else {
+ cp = strchr(base + current + 1, '/');
+ if (!cp)
+ die("oops in prep_exclude");
+ cp++;
+ }
+ stk->prev = dir->exclude_stack;
+ stk->baselen = cp - base;
+ memcpy(dir->basebuf + current, base + current,
+ stk->baselen - current);
+ strcpy(dir->basebuf + stk->baselen, dir->exclude_per_dir);
+ /*
+ * dir->basebuf gets reused by the traversal, but we
+ * need fname to remain unchanged to ensure the src
+ * member of each struct exclude correctly
+ * back-references its source file. Other invocations
+ * of add_exclude_list provide stable strings, so we
+ * strdup() and free() here in the caller.
+ */
+ el = add_exclude_list(dir, EXC_DIRS, strdup(dir->basebuf));
+ stk->exclude_ix = group->nr - 1;
+ add_excludes_from_file_to_list(dir->basebuf,
+ dir->basebuf, stk->baselen,
+ el, 1);
+ dir->exclude_stack = stk;
+ current = stk->baselen;
+ }
+ dir->basebuf[baselen] = '\0';
+}
+
+/*
+ * Loads the exclude lists for the directory containing pathname, then
+ * scans all exclude lists to determine whether pathname is excluded.
+ * Returns the exclude_list element which matched, or NULL for
+ * undecided.
+ */
+static struct exclude *last_exclude_matching(struct dir_struct *dir,
+ const char *pathname,
+ int *dtype_p)
+{
+ int pathlen = strlen(pathname);
+ const char *basename = strrchr(pathname, '/');
+ basename = (basename) ? basename+1 : pathname;
+
+ prep_exclude(dir, pathname, basename-pathname);
+ return last_exclude_matching_from_lists(dir, pathname, pathlen,
+ basename, dtype_p);
}
/*
--
1.8.1.2.8021.g7e51819
^ permalink raw reply related
* [PATCH 3/8] dir.c: git-status --ignored: don't list empty ignored directories
From: Karsten Blees @ 2013-03-18 20:28 UTC (permalink / raw)
To: Git List
Cc: Junio C Hamano, Erik Faye-Lund, Ramkumar Ramachandra, Robert Zeh,
Duy Nguyen, Antoine Pelisse, Adam Spiers
In-Reply-To: <514775FA.9080304@gmail.com>
'git-status --ignored' lists ignored tracked directories without any
ignored files if a tracked file happens to match an exclude pattern.
Always exclude tracked files.
Signed-off-by: Karsten Blees <blees@dcon.de>
---
dir.c | 11 ++++-------
t/t7061-wtstatus-ignore.sh | 24 ++++++++++++++++++++++++
2 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/dir.c b/dir.c
index 7c9bc9c..fd1f088 100644
--- a/dir.c
+++ b/dir.c
@@ -1109,16 +1109,13 @@ static int treat_file(struct dir_struct *dir, struct strbuf *path, int exclude,
struct path_exclude_check check;
int exclude_file = 0;
+ /* Always exclude indexed files */
+ if (index_name_exists(&the_index, path->buf, path->len, ignore_case))
+ return 1;
+
if (exclude)
exclude_file = !(dir->flags & DIR_SHOW_IGNORED);
else if (dir->flags & DIR_SHOW_IGNORED) {
- /* Always exclude indexed files */
- struct cache_entry *ce = index_name_exists(&the_index,
- path->buf, path->len, ignore_case);
-
- if (ce)
- return 1;
-
path_exclude_check_init(&check, dir);
if (!is_path_excluded(&check, path->buf, path->len, dtype))
diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh
index 4ece129..28b7d95 100755
--- a/t/t7061-wtstatus-ignore.sh
+++ b/t/t7061-wtstatus-ignore.sh
@@ -122,10 +122,34 @@ cat >expected <<\EOF
?? .gitignore
?? actual
?? expected
+EOF
+
+test_expect_success 'status ignored tracked directory and ignored file with --ignore' '
+ echo "committed" >>.gitignore &&
+ git status --porcelain --ignored >actual &&
+ test_cmp expected actual
+'
+
+cat >expected <<\EOF
+?? .gitignore
+?? actual
+?? expected
+EOF
+
+test_expect_success 'status ignored tracked directory and ignored file with --ignore -u' '
+ git status --porcelain --ignored -u >actual &&
+ test_cmp expected actual
+'
+
+cat >expected <<\EOF
+?? .gitignore
+?? actual
+?? expected
!! tracked/
EOF
test_expect_success 'status ignored tracked directory and uncommitted file with --ignore' '
+ echo "tracked" >.gitignore &&
: >tracked/uncommitted &&
git status --porcelain --ignored >actual &&
test_cmp expected actual
--
1.8.1.2.8021.g7e51819
^ permalink raw reply related
* [PATCH 2/8] dir.c: git-status --ignored: don't list files in ignored directories
From: Karsten Blees @ 2013-03-18 20:28 UTC (permalink / raw)
To: Git List
Cc: Junio C Hamano, Erik Faye-Lund, Ramkumar Ramachandra, Robert Zeh,
Duy Nguyen, Antoine Pelisse, Adam Spiers
In-Reply-To: <514775FA.9080304@gmail.com>
'git-status --ignored' lists both the ignored directory and the ignored
files if the files are in a tracked sub directory.
When recursing into sub directories in read_directory_recursive, pass on
the check_only parameter so that we don't accidentally add the files.
Signed-off-by: Karsten Blees <blees@dcon.de>
---
dir.c | 4 +---
t/t7061-wtstatus-ignore.sh | 27 +++++++++++++++++++++++++++
2 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/dir.c b/dir.c
index ec4eebf..7c9bc9c 100644
--- a/dir.c
+++ b/dir.c
@@ -1273,7 +1273,6 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,
return path_ignored;
case DT_DIR:
strbuf_addch(path, '/');
-
switch (treat_directory(dir, path->buf, path->len, exclude, simplify)) {
case show_directory:
break;
@@ -1343,8 +1342,7 @@ static int read_directory_recursive(struct dir_struct *dir,
switch (treat_path(dir, de, &path, baselen, simplify)) {
case path_recurse:
contents += read_directory_recursive(dir, path.buf,
- path.len, 0,
- simplify);
+ path.len, check_only, simplify);
continue;
case path_ignored:
continue;
diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh
index 0f1034e..4ece129 100755
--- a/t/t7061-wtstatus-ignore.sh
+++ b/t/t7061-wtstatus-ignore.sh
@@ -170,4 +170,31 @@ test_expect_success 'status ignored tracked directory with uncommitted file in u
test_cmp expected actual
'
+cat >expected <<\EOF
+?? .gitignore
+?? actual
+?? expected
+!! tracked/
+EOF
+
+test_expect_success 'status ignored tracked directory with uncommitted file in tracked subdir with --ignore' '
+ : >tracked/ignored/committed &&
+ git add -f tracked/ignored/committed &&
+ git commit -m. &&
+ git status --porcelain --ignored >actual &&
+ test_cmp expected actual
+'
+
+cat >expected <<\EOF
+?? .gitignore
+?? actual
+?? expected
+!! tracked/ignored/uncommitted
+EOF
+
+test_expect_success 'status ignored tracked directory with uncommitted file in tracked subdir with --ignore -u' '
+ git status --porcelain --ignored -u >actual &&
+ test_cmp expected actual
+'
+
test_done
--
1.8.1.2.8021.g7e51819
^ permalink raw reply related
* [PATCH 1/8] dir.c: git-status --ignored: don't drop ignored directories
From: Karsten Blees @ 2013-03-18 20:28 UTC (permalink / raw)
To: Git List
Cc: Junio C Hamano, Erik Faye-Lund, Ramkumar Ramachandra, Robert Zeh,
Duy Nguyen, Antoine Pelisse, Adam Spiers
In-Reply-To: <514775FA.9080304@gmail.com>
'git-status --ignored' drops ignored directories if they contain untracked
files in an untracked sub directory.
Fix it by getting exact (recursive) excluded status in treat_directory.
Signed-off-by: Karsten Blees <blees@dcon.de>
---
dir.c | 9 +++++++++
t/t7061-wtstatus-ignore.sh | 27 +++++++++++++++++++++++++++
2 files changed, 36 insertions(+)
diff --git a/dir.c b/dir.c
index 57394e4..ec4eebf 100644
--- a/dir.c
+++ b/dir.c
@@ -1060,6 +1060,15 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,
/* This is the "show_other_directories" case */
+ /* might be a sub directory in an excluded directory */
+ if (!exclude) {
+ struct path_exclude_check check;
+ int dt = DT_DIR;
+ path_exclude_check_init(&check, dir);
+ exclude = is_path_excluded(&check, dirname, len, &dt);
+ path_exclude_check_clear(&check);
+ }
+
/*
* We are looking for ignored files and our directory is not ignored,
* check if it contains only ignored files
diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh
index 0da1214..0f1034e 100755
--- a/t/t7061-wtstatus-ignore.sh
+++ b/t/t7061-wtstatus-ignore.sh
@@ -143,4 +143,31 @@ test_expect_success 'status ignored tracked directory and uncommitted file with
test_cmp expected actual
'
+cat >expected <<\EOF
+?? .gitignore
+?? actual
+?? expected
+!! tracked/
+EOF
+
+test_expect_success 'status ignored tracked directory with uncommitted file in untracked subdir with --ignore' '
+ rm -rf tracked/uncommitted &&
+ mkdir tracked/ignored &&
+ : >tracked/ignored/uncommitted &&
+ git status --porcelain --ignored >actual &&
+ test_cmp expected actual
+'
+
+cat >expected <<\EOF
+?? .gitignore
+?? actual
+?? expected
+!! tracked/ignored/uncommitted
+EOF
+
+test_expect_success 'status ignored tracked directory with uncommitted file in untracked subdir with --ignore -u' '
+ git status --porcelain --ignored -u >actual &&
+ test_cmp expected actual
+'
+
test_done
--
1.8.1.2.8021.g7e51819
^ permalink raw reply related
* [PATCH 0/8] Improve git-status --ignored
From: Karsten Blees @ 2013-03-18 20:28 UTC (permalink / raw)
To: Git List
Cc: Junio C Hamano, Erik Faye-Lund, Ramkumar Ramachandra, Robert Zeh,
Duy Nguyen, Antoine Pelisse, Adam Spiers
This patch series addresses several bugs and performance issues in .gitignore processing that came up in the inotify discussion.
Also available here:
https://github.com/kblees/git/tree/kb/improve-git-status-ignored
git pull git://github.com/kblees/git.git kb/improve-git-status-ignored
Patches 1 - 4 fix bugs in 'git-status --ignored' and add appropriate test cases.
Patches 5 - 7 eliminate the is_path_excluded API, in favor of a slightly improved and faster is_excluded. This speeds up 'git-ls-files --cached --ignored' by factor 5 - 6.
Patch 8 finally skips excluded checks for tracked files. With the bugs and is_path_excluded out of the way, it should be obvious that this can safely be done unconditionally without introducing regressions. Speeds up 'git-status [--ignored]' by factor 1.4 - 2.
I still believe that 'git-status --ignored' shouldn't list "ignored tracked" directories, to be consistent with the listing of untracked directories, and because "ignored tracked" contradicts the very definition of ignored content in gitignore(5).
Cheers,
Karsten
Karsten Blees (8):
dir.c: git-status --ignored: don't drop ignored directories
dir.c: git-status --ignored: don't list files in ignored directories
dir.c: git-status --ignored: don't list empty ignored directories
dir.c: git-status --ignored: don't list empty directories as ignored
dir.c: move prep_exclude and factor out parts of last_exclude_matching
dir.c: unify is_excluded and is_path_excluded APIs
dir.c: replace is_path_excluded with now equivalent is_excluded API
dir.c: git-status: avoid is_excluded checks for tracked files
builtin/add.c | 5 +-
builtin/check-ignore.c | 6 +-
builtin/ls-files.c | 15 +-
dir.c | 351 +++++++++++++++++----------------------------
dir.h | 22 +--
t/t7061-wtstatus-ignore.sh | 104 +++++++++++++-
unpack-trees.c | 10 +-
unpack-trees.h | 1 -
8 files changed, 243 insertions(+), 271 deletions(-)
--
^ permalink raw reply
* Re: [ITCH] pull.default
From: Ramkumar Ramachandra @ 2013-03-18 19:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
In-Reply-To: <7vy5dkmt7k.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> I usually use `git fetch`, inspect state, and then merge/ rebase
>> accordingly. Unfortunately, this is very sub-optimal as I can
>> automate this 80% of the time. I want to be able to decide what to do
>> on a repository-specific basis, and hence propose a pull.default which
>> can be set to the following:
>> 1. fetch. Just fetch. (I will set this as my default in ~/.gitconfig)
>> 2. fast-forward. Fetch. If the FETCH_HEAD is directly ahead of HEAD,
>> `stash`, merge, and stash apply. Otherwise, do nothing.
>> 3. rebase. Fetch. stash, rebase, stash apply. `git pull n` will do
>> rebase --onto master HEAD~n instead of rebase.
>> 4. reset. Fetch, stash, reset --hard FETCH_HEAD, stash apply.
>>
>> Ofcourse, it should print a message saying what it did at the end.
>>
>> What do you think?
>
> Many other possibilities are missing. "fetch and merge", for
> example.
>
> You seem to be generalizing the "--rebase" and "--ff-only" options
> of "git pull" with 2 and 3, which may (or may not) make sense.
>
> I think you can shrink and enhance the above repertoire at the same
> time by separating "do I want to have stash and stash pop around"
> bit into an orthogonal axis. The other orthogonal axes are "Under
> what condition do I integrate the work from the upstream?" (e.g.
> "only when I do not have anything, aka, ff-only") and "How would I
> integrate the work from the upstream?" (e.g. "rebase my work" and
> "discard anything I did aka reset --hard").
Excellent I was hoping to start a discussion like this. My initial
thought was designing a custom script that git-pull will execute like
a hook; we should, however, be able to get away with designing enough
configuration orthogonal configuration variables. For anything more
complex, just do it by hand!
> By the way, I do not think you should start your design from a
> configuration (this is a general principle, not limited to this
> case). Think about what the smallest number of independent options
> you need to add to express various workflows, and then turn them
> into configuration variables that can set the default, all of which
> have to be overridable from the command line.
Will do. Expect a draft soon.
^ permalink raw reply
* Re: [ITCH] pull.default
From: Junio C Hamano @ 2013-03-18 18:57 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List
In-Reply-To: <CALkWK0nT9hE_kRt3DLXfP45OwCSLurMOzuTqepxhkWjagbFDUQ@mail.gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> I usually use `git fetch`, inspect state, and then merge/ rebase
> accordingly. Unfortunately, this is very sub-optimal as I can
> automate this 80% of the time. I want to be able to decide what to do
> on a repository-specific basis, and hence propose a pull.default which
> can be set to the following:
> 1. fetch. Just fetch. (I will set this as my default in ~/.gitconfig)
> 2. fast-forward. Fetch. If the FETCH_HEAD is directly ahead of HEAD,
> `stash`, merge, and stash apply. Otherwise, do nothing.
> 3. rebase. Fetch. stash, rebase, stash apply. `git pull n` will do
> rebase --onto master HEAD~n instead of rebase.
> 4. reset. Fetch, stash, reset --hard FETCH_HEAD, stash apply.
>
> Ofcourse, it should print a message saying what it did at the end.
>
> What do you think?
Many other possibilities are missing. "fetch and merge", for
example.
You seem to be generalizing the "--rebase" and "--ff-only" options
of "git pull" with 2 and 3, which may (or may not) make sense.
I think you can shrink and enhance the above repertoire at the same
time by separating "do I want to have stash and stash pop around"
bit into an orthogonal axis. The other orthogonal axes are "Under
what condition do I integrate the work from the upstream?" (e.g.
"only when I do not have anything, aka, ff-only") and "How would I
integrate the work from the upstream?" (e.g. "rebase my work" and
"discard anything I did aka reset --hard").
By the way, I do not think you should start your design from a
configuration (this is a general principle, not limited to this
case). Think about what the smallest number of independent options
you need to add to express various workflows, and then turn them
into configuration variables that can set the default, all of which
have to be overridable from the command line.
^ permalink raw reply
* Re: Make GIT_USE_LOOKUP default?
From: Junio C Hamano @ 2013-03-18 18:40 UTC (permalink / raw)
To: Jeff King; +Cc: Ingo Molnar, Jonathan Nieder, Duy Nguyen, Git Mailing List
In-Reply-To: <20130318171949.GB15924@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I do not see anything obviously wrong in it, though I did not walk
> through all of the ofs calculation to look for any clever speedups.
> However, I think it is clear from the other timings and Ingo's thread
> that glibc 2.11's memcmp does not perform very well on many short reads.
> And sha1_entry_pos will do memcmps even smaller than 20 bytes.
>
> What happens if you do this?
The overall trend is the same.
[without GIT_USE_LOOKUP]
real 0m40.044s
real 0m40.054s
real 0m40.072s
real 0m40.097s
real 0m40.159s
[with GIT_USE_LOOKUP]
real 0m40.257s
real 0m40.281s
real 0m40.311s
real 0m40.366s
real 0m40.407s
I suspect that after the first few rounds the range shrinks small
enough that the difference between a simple "mi = (hi + lo)/2" and
convoluted ofs computation becomes dominant. Perhaps we should only
do N-R for the initial midpoint selection and then fall back to a
stupid binary search?
^ permalink raw reply
* [ITCH] pull.default
From: Ramkumar Ramachandra @ 2013-03-18 18:39 UTC (permalink / raw)
To: Git List
Hi,
I usually use `git fetch`, inspect state, and then merge/ rebase
accordingly. Unfortunately, this is very sub-optimal as I can
automate this 80% of the time. I want to be able to decide what to do
on a repository-specific basis, and hence propose a pull.default which
can be set to the following:
1. fetch. Just fetch. (I will set this as my default in ~/.gitconfig)
2. fast-forward. Fetch. If the FETCH_HEAD is directly ahead of HEAD,
`stash`, merge, and stash apply. Otherwise, do nothing.
3. rebase. Fetch. stash, rebase, stash apply. `git pull n` will do
rebase --onto master HEAD~n instead of rebase.
4. reset. Fetch, stash, reset --hard FETCH_HEAD, stash apply.
Ofcourse, it should print a message saying what it did at the end.
What do you think?
Ram
^ permalink raw reply
* Re: [PATCH v1 27/45] Convert run_add_interactive to use struct pathspec
From: John Keeping @ 2013-03-18 18:26 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano
In-Reply-To: <1363327620-29017-28-git-send-email-pclouds@gmail.com>
On Fri, Mar 15, 2013 at 01:06:42PM +0700, Nguyễn Thái Ngọc Duy wrote:
> This passes the pathspec, more or less unmodified, to
> git-add--interactive. The command itself does not process pathspec. It
> simply passes the pathspec to other builtin commands. So if all those
> commands support pathspec, we're good.
This breaks "git reset --keep" in a subdirectory for me.
I ran "git reset --keep <branch>" in a subdirectory and got:
fatal: BUG: parse_pathspec cannot take no argument in this case
Bisecting points to this commit.
The simplest test case is:
( cd t && ../bin-wrappers/git reset --keep HEAD )
which works on master but not pu.
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> builtin/add.c | 26 ++++++++++----------------
> builtin/checkout.c | 9 ++++-----
> builtin/reset.c | 8 ++++----
> commit.h | 2 +-
> 4 files changed, 19 insertions(+), 26 deletions(-)
>
> diff --git a/builtin/add.c b/builtin/add.c
> index ec6fbe3..2b20d7d 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -139,16 +139,12 @@ static void refresh(int verbose, const char **pathspec)
> }
>
> int run_add_interactive(const char *revision, const char *patch_mode,
> - const char **pathspec)
> + const struct pathspec *pathspec)
> {
> - int status, ac, pc = 0;
> + int status, ac, i;
> const char **args;
>
> - if (pathspec)
> - while (pathspec[pc])
> - pc++;
> -
> - args = xcalloc(sizeof(const char *), (pc + 5));
> + args = xcalloc(sizeof(const char *), (pathspec->nr + 6));
> ac = 0;
> args[ac++] = "add--interactive";
> if (patch_mode)
> @@ -156,11 +152,9 @@ int run_add_interactive(const char *revision, const char *patch_mode,
> if (revision)
> args[ac++] = revision;
> args[ac++] = "--";
> - if (pc) {
> - memcpy(&(args[ac]), pathspec, sizeof(const char *) * pc);
> - ac += pc;
> - }
> - args[ac] = NULL;
> + for (i = 0; i < pathspec->nr; i++)
> + /* pass original pathspec, to be re-parsed */
> + args[ac++] = pathspec->items[i].original;
>
> status = run_command_v_opt(args, RUN_GIT_CMD);
> free(args);
> @@ -175,17 +169,17 @@ int interactive_add(int argc, const char **argv, const char *prefix, int patch)
> * git-add--interactive itself does not parse pathspec. It
> * simply passes the pathspec to other builtin commands. Let's
> * hope all of them support all magic, or we'll need to limit
> - * the magic here. There is still a problem with prefix. But
> - * that'll be worked on later on.
> + * the magic here.
> */
> parse_pathspec(&pathspec, PATHSPEC_ALL_MAGIC & ~PATHSPEC_FROMTOP,
> PATHSPEC_PREFER_FULL |
> - PATHSPEC_SYMLINK_LEADING_PATH,
> + PATHSPEC_SYMLINK_LEADING_PATH |
> + PATHSPEC_PREFIX_ORIGIN,
> prefix, argv);
>
> return run_add_interactive(NULL,
> patch ? "--patch" : NULL,
> - pathspec.raw);
> + &pathspec);
> }
>
> static int edit_patch(int argc, const char **argv, const char *prefix)
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 3c19cb4..2ddff95 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -256,7 +256,7 @@ static int checkout_paths(const struct checkout_opts *opts,
>
> if (opts->patch_mode)
> return run_add_interactive(revision, "--patch=checkout",
> - opts->pathspec.raw);
> + &opts->pathspec);
>
> lock_file = xcalloc(1, sizeof(struct lock_file));
>
> @@ -1115,10 +1115,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
> * cannot handle. Magic mask is pretty safe to be
> * lifted for new magic when opts.patch_mode == 0.
> */
> - parse_pathspec(&opts.pathspec,
> - opts.patch_mode == 0 ? 0 :
> - (PATHSPEC_ALL_MAGIC & ~PATHSPEC_FROMTOP),
> - 0, prefix, argv);
> + parse_pathspec(&opts.pathspec, 0,
> + opts.patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0,
> + prefix, argv);
>
> if (!opts.pathspec.nr)
> die(_("invalid path specification"));
> diff --git a/builtin/reset.c b/builtin/reset.c
> index da7282e..7c6e8b6 100644
> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -216,9 +216,9 @@ static void parse_args(struct pathspec *pathspec,
> }
> }
> *rev_ret = rev;
> - parse_pathspec(pathspec,
> - patch_mode ? PATHSPEC_ALL_MAGIC & ~PATHSPEC_FROMTOP : 0,
> - PATHSPEC_PREFER_FULL,
> + parse_pathspec(pathspec, 0,
> + PATHSPEC_PREFER_FULL |
> + patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0,
> prefix, argv);
> }
>
> @@ -296,7 +296,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
> if (patch_mode) {
> if (reset_type != NONE)
> die(_("--patch is incompatible with --{hard,mixed,soft}"));
> - return run_add_interactive(sha1_to_hex(sha1), "--patch=reset", pathspec.raw);
> + return run_add_interactive(sha1_to_hex(sha1), "--patch=reset", &pathspec);
> }
>
> /* git reset tree [--] paths... can be used to
> diff --git a/commit.h b/commit.h
> index 4138bb4..9448fda 100644
> --- a/commit.h
> +++ b/commit.h
> @@ -179,7 +179,7 @@ int in_merge_bases(struct commit *, struct commit *);
>
> extern int interactive_add(int argc, const char **argv, const char *prefix, int patch);
> extern int run_add_interactive(const char *revision, const char *patch_mode,
> - const char **pathspec);
> + const struct pathspec *pathspec);
>
> static inline int single_parent(struct commit *commit)
> {
> --
> 1.8.0.rc0.19.g7bbb31d
^ permalink raw reply
* Re: random server hacks on top of git
From: Junio C Hamano @ 2013-03-18 18:24 UTC (permalink / raw)
To: Jeff King; +Cc: René Scharfe, git
In-Reply-To: <20130318121243.GC14789@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> - blame-tree; re-rolled from my submission last year to build on top
>
> - diff --max-depth; this is a requirement to do blame-tree efficiently
Both look mildly interesting, especially after the magic pathspec
settles the latter may be a good addition.
> - share ref selection code between "git branch", "git tag", and "git
> for-each-ref".
Nice.
> - receive.maxsize; index-pack will happily spool data to disk
Again nice.
> - receive.advertisealternates; basically turn off ".have"
> advertisement.
I think this is a sane thing to do, especially with some hints on
the most common reference tree everybody is expected to know about.
> - receive.hiderefs; this is going to become redundant with Junio's
> implementation
Yup.
> - an audit reflog; we keep a reflog for all refs at the root of the
> repository. It differs from a regular reflog in that:
>
> 1. It never expires.
>
> 2. It is not part of reachability analysis.
>
> 3. It includes the refname for each entry, so you can see
> deletions.
Interesting.
> - ignore some fsck warnings under transfer.fsckobjects; some of them
> are annoyingly common when people pull old history from an
> existing project and try to push it back up.
Depending on the implementation, this may be very much valuable.
^ permalink raw reply
* Re: [PATCH] read-cache: avoid memcpy in expand_name_field in index v4
From: Junio C Hamano @ 2013-03-18 17:50 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1363611482-1015-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> perf reports memcpy at the the 6th position [1] in "git status -uno"
> using index v4, and strbuf_remove() in expand_name_field() accounts
> for 25% of that. What we need here is a simple string cut and a
> cheaper strbuf_setlen() should be enough.
While it is true that strbuf_remove(&sb, sb.len - trim, trim) is
equivalent to strbuf_setlen(&sb, sb.len - trim), I wonder why we see
any memcpy() in the first place.
strbuf_remove(&sb, sb.len - trim, trim) is turned into
strbuf_splice(&sb, sb.len - trim, trim, NULL, 0) and then in turn it
does these two:
memmove(sb.buf + (sb.len - trim) + 0,
sb.buf + sb.len, 0);
memcpy(sb.buf + (sb.len - trim), NULL, 0);
both of which should be a no-op, no?
There also is this call that has the same "trim at the right end":
pretty.c: strbuf_remove(sb, sb->len - trimlen, trimlen);
It almost makes me suggest that it may be a better solution to make
strbuf_remove() more intelligent about such a call pattern _if_
these empty memmove/memcpy are so expensive, perhaps like the
attached. It could be that strbuf_splice() should be the one that
ought to be more intelligent, but I'll leave it up to you to
benchmark to find out where the best place to optimize is.
strbuf.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/strbuf.c b/strbuf.c
index 05d0693..12db700 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -179,7 +179,10 @@ void strbuf_insert(struct strbuf *sb, size_t pos, const void *data, size_t len)
void strbuf_remove(struct strbuf *sb, size_t pos, size_t len)
{
- strbuf_splice(sb, pos, len, NULL, 0);
+ if (pos + len == sb->len)
+ strbuf_setlen(sb, pos);
+ else
+ strbuf_splice(sb, pos, len, NULL, 0);
}
void strbuf_add(struct strbuf *sb, const void *data, size_t len)
^ permalink raw reply related
* Re: Make GIT_USE_LOOKUP default?
From: Jeff King @ 2013-03-18 17:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ingo Molnar, Jonathan Nieder, Duy Nguyen, Git Mailing List
In-Reply-To: <7vli9kocuc.fsf@alter.siamese.dyndns.org>
On Mon, Mar 18, 2013 at 10:08:11AM -0700, Junio C Hamano wrote:
> Just for fun, here is a totally unrelated comparison, both with
> current master, compiled with -O2 and running on the same box.
>
> [without GIT_USE_LOOKUP]
> real 0m39.906s
> real 0m40.020s
> real 0m40.022s
> real 0m40.027s
> real 0m40.146s
>
> [with GIT_USE_LOOKUP]
> real 0m40.336s
> real 0m40.376s
> real 0m40.452s
> real 0m40.503s
> real 0m40.572s
>
> Maybe the Newton-Raphson is right as a concept (it does seem to
> result in fewer minor-faults) but the current code is implemented
> poorly and has a huge room for improvement?
I do not see anything obviously wrong in it, though I did not walk
through all of the ofs calculation to look for any clever speedups.
However, I think it is clear from the other timings and Ingo's thread
that glibc 2.11's memcmp does not perform very well on many short reads.
And sha1_entry_pos will do memcmps even smaller than 20 bytes.
What happens if you do this?
diff --git a/sha1-lookup.c b/sha1-lookup.c
index c4dc55d..4ea03bd 100644
--- a/sha1-lookup.c
+++ b/sha1-lookup.c
@@ -102,6 +102,17 @@ int sha1_pos(const unsigned char *sha1, void *table, size_t nr,
return -lo-1;
}
+static int short_memcmp(const unsigned char *a,
+ const unsigned char *b,
+ int len)
+{
+ int i;
+ for (i = 0; i < len; i++, a++, b++)
+ if (*a != *b)
+ return *a - *b;
+ return 0;
+}
+
/*
* Conventional binary search loop looks like this:
*
@@ -257,7 +268,7 @@ int sha1_entry_pos(const void *table,
lo, mi, hi, sha1_to_hex(key));
mi_key = base + elem_size * mi + key_offset;
- cmp = memcmp(mi_key + ofs_0, key + ofs_0, 20 - ofs_0);
+ cmp = short_memcmp(mi_key + ofs_0, key + ofs_0, 20 - ofs_0);
if (!cmp)
return mi;
if (cmp > 0) {
^ permalink raw reply related
* Re: Make GIT_USE_LOOKUP default?
From: Junio C Hamano @ 2013-03-18 17:08 UTC (permalink / raw)
To: Jeff King; +Cc: Ingo Molnar, Jonathan Nieder, Duy Nguyen, Git Mailing List
In-Reply-To: <20130318164950.GA14844@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Mar 18, 2013 at 09:44:20AM -0700, Junio C Hamano wrote:
>
>> FWIW, I am getting something like this on my
>>
>> $ grep 'model name' /proc/cpuinfo | uniq -c
>> 4 model name : Intel(R) Core(TM)2 Quad CPU Q9450 @ 2.66GHz
>>
>> The same "rev-list --objects --all >/dev/null", best of five:
>>
>> [current master, compiled with -O2]
>> real 0m39.956s
>> user 0m39.562s
>> sys 0m0.396s
>>
>> [revert 1a812f3 (i.e., go back to memcmp), -O2]
>> real 0m42.161s
>> user 0m41.879s
>> sys 0m0.284s
>>
>> It could be that the difference may be how well memcmp() is
>> optimized, no? My box runs Debian with libc6 2.11.3-4 and gcc
>> 4.4.5.
>
> Yeah, that would make sense. I have libc6 2.13-38 and gcc 4.7.2 (debian
> unstable).
Just for fun, here is a totally unrelated comparison, both with
current master, compiled with -O2 and running on the same box.
[without GIT_USE_LOOKUP]
real 0m39.906s
real 0m40.020s
real 0m40.022s
real 0m40.027s
real 0m40.146s
[with GIT_USE_LOOKUP]
real 0m40.336s
real 0m40.376s
real 0m40.452s
real 0m40.503s
real 0m40.572s
Maybe the Newton-Raphson is right as a concept (it does seem to
result in fewer minor-faults) but the current code is implemented
poorly and has a huge room for improvement?
[without GIT_USE_LOOKUP]
0inputs+0outputs (0major+182895minor)pagefaults 0swaps
0inputs+0outputs (0major+182920minor)pagefaults 0swaps
0inputs+0outputs (0major+183004minor)pagefaults 0swaps
0inputs+0outputs (0major+183006minor)pagefaults 0swaps
0inputs+0outputs (0major+183036minor)pagefaults 0swaps
[with GIT_USE_LOOKUP]
0inputs+0outputs (0major+182803minor)pagefaults 0swaps
0inputs+0outputs (0major+182886minor)pagefaults 0swaps
0inputs+0outputs (0major+182967minor)pagefaults 0swaps
0inputs+0outputs (0major+182997minor)pagefaults 0swaps
0inputs+0outputs (0major+182998minor)pagefaults 0swaps
^ permalink raw reply
* Re: [ITCH] Specify refspec without remote
From: Jeff King @ 2013-03-18 17:08 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List
In-Reply-To: <CALkWK0nYECHZaxit9jR-tS=7fXyOP5dy6mqUz0DKmbTRU-xRNw@mail.gmail.com>
On Mon, Mar 18, 2013 at 10:28:59PM +0530, Ramkumar Ramachandra wrote:
> This has irritated me for a long time. I often end up doing:
>
> $ git push master:master +pu:pu
Me too.
> Is there a reason for the remote not being optional, or are we just
> waiting for a patch? The only problem I can foresee is very minor:
> there is a ref with the same name as a remote; in this case, we'd have
> to specify both the remote and the ref.
I think the ambiguity is a little more complex than that, because we
cannot enumerate the universe of all remotes. Keep in mind that we can
take either a configured remote or a URL (or ssh host). So what does:
git push foo:bar
mean? Is it pushing "refs/heads/foo" to "refs/heads/bar" on "origin"? Or
is it using the default refspecs to push to the "bar" repo on the host
"foo" over ssh?
So you would need some heuristics based on whether something was a valid
refspec, or could be a valid remote name or URL.
-Peff
^ permalink raw reply
* [ITCH] Specify refspec without remote
From: Ramkumar Ramachandra @ 2013-03-18 16:58 UTC (permalink / raw)
To: Git List
Hi,
This has irritated me for a long time. I often end up doing:
$ git push master:master +pu:pu
only to get an error, because I have to specify the remote before
specifying the refspec. I find this ugly because I rarely want to
push to a remote that is different from the one configured, but often
want to specify my own refspec (say which branches to push, force
push). Ofcourse, the same is the case while fetching.
Is there a reason for the remote not being optional, or are we just
waiting for a patch? The only problem I can foresee is very minor:
there is a ref with the same name as a remote; in this case, we'd have
to specify both the remote and the ref.
Thanks.
Ram
^ permalink raw reply
* Re: Make GIT_USE_LOOKUP default?
From: Jeff King @ 2013-03-18 16:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ingo Molnar, Jonathan Nieder, Duy Nguyen, Git Mailing List
In-Reply-To: <7vtxo8ody3.fsf@alter.siamese.dyndns.org>
On Mon, Mar 18, 2013 at 09:44:20AM -0700, Junio C Hamano wrote:
> FWIW, I am getting something like this on my
>
> $ grep 'model name' /proc/cpuinfo | uniq -c
> 4 model name : Intel(R) Core(TM)2 Quad CPU Q9450 @ 2.66GHz
>
> The same "rev-list --objects --all >/dev/null", best of five:
>
> [current master, compiled with -O2]
> real 0m39.956s
> user 0m39.562s
> sys 0m0.396s
>
> [revert 1a812f3 (i.e., go back to memcmp), -O2]
> real 0m42.161s
> user 0m41.879s
> sys 0m0.284s
>
> It could be that the difference may be how well memcmp() is
> optimized, no? My box runs Debian with libc6 2.11.3-4 and gcc
> 4.4.5.
Yeah, that would make sense. I have libc6 2.13-38 and gcc 4.7.2 (debian
unstable).
-Peff
^ permalink raw reply
* Re: Make GIT_USE_LOOKUP default?
From: Junio C Hamano @ 2013-03-18 16:44 UTC (permalink / raw)
To: Jeff King; +Cc: Ingo Molnar, Jonathan Nieder, Duy Nguyen, Git Mailing List
In-Reply-To: <20130318073229.GA5551@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> By the way, looking at that made me think for a few minutes about
> hashcmp, and I was surprised to find that we use an open-coded
> comparison loop. That dates back to this thread by Ingo:
>
> http://article.gmane.org/gmane.comp.version-control.git/172286
>
> I could not replicate his benchmarks at all. In fact, my measurements
> showed a slight slowdown with 1a812f3 (hashcmp(): inline memcmp() by
> hand to optimize, 2011-04-28).
>
> Here are my best-of-five numbers for running "git rev-list --objects
> --all >/dev/null" on linux-2.6.git:
>
> [current master, compiled with -O2]
> real 0m45.612s
> user 0m45.140s
> sys 0m0.300s
> ...
> [revert 1a812f3 (i.e., go back to memcmp), -O2]
> real 0m44.358s
> user 0m43.876s
> sys 0m0.316s
> ...
> I wonder why we get such different numbers. Ingo said his tests are on a
> Nehalem CPU, as are mine (mine is an i7-840QM). I wonder if we should be
> wrapping the optimization in an #ifdef, but I'm not sure which flag we
> should be checking.
FWIW, I am getting something like this on my
$ grep 'model name' /proc/cpuinfo | uniq -c
4 model name : Intel(R) Core(TM)2 Quad CPU Q9450 @ 2.66GHz
The same "rev-list --objects --all >/dev/null", best of five:
[current master, compiled with -O2]
real 0m39.956s
user 0m39.562s
sys 0m0.396s
[revert 1a812f3 (i.e., go back to memcmp), -O2]
real 0m42.161s
user 0m41.879s
sys 0m0.284s
It could be that the difference may be how well memcmp() is
optimized, no? My box runs Debian with libc6 2.11.3-4 and gcc
4.4.5.
^ permalink raw reply
* Re: [PATCH v3 4/4] pack-refs: add fully-peeled trait
From: Junio C Hamano @ 2013-03-18 16:26 UTC (permalink / raw)
To: Jeff King; +Cc: git, Michael Haggerty
In-Reply-To: <20130318113732.GA14789@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Here's the re-roll.
Above reasoning (elided) look sensible. Thanks; will replace.
^ permalink raw reply
* Re: [PATCH 0/4] Support triangular workflows
From: Marc Branchaud @ 2013-03-18 15:55 UTC (permalink / raw)
To: Jeff King; +Cc: Ramkumar Ramachandra, Git List, Junio C Hamano
In-Reply-To: <20130318142526.GA23075@sigill.intra.peff.net>
On 13-03-18 10:25 AM, Jeff King wrote:
> On Mon, Mar 18, 2013 at 06:46:11PM +0530, Ramkumar Ramachandra wrote:
>
>> I've put off implementing remote.default corresponding to
>> remote.pushdefault, as Jeff suggested in [1], because it's currently
>> not an itch; apart from the obvious symmetry, I don't know what
>> purpose it serves: why would anyone want to fetch from a remote other
>> than origin by default? Why wouldn't they simply swap that remote's
>> name with "origin"? However, it's a nice thing to have for symmetry,
>> and it should be trivial to implement: any interested person is
>> welcome to pick it up.
>
> Yeah, I agree that it does not have much point, aside from people who
> have an unreasonable aversion to using the word "origin". There was a
> series posted last summer to add remote.default:
>
> http://article.gmane.org/gmane.comp.version-control.git/201065
>
> It ended up stalled and never got merged. I think the main impetus was
> that "git clone -o foo" should leave "foo" in remote.default (of course,
> that still leaves unanswered why anyone would really want to use "-o
> foo" in the first place).
I'm the guy who dropped the ball on that series. I still intend to pick it
up (honest!) but I just haven't had the time.
The impetus was originally getting relative submodule paths to work on
detached HEADs [1]. My patch for that doesn't work when someone does "clone
-o", because various parts of git assume there's a remote named "origin".
The discussion led to the idea of using the remote name specified during the
initial clone, and implementing that as a remote.default config value.
As for why "clone -o" exists, it was added in v1.1.0:
commit e6c310fd0d7384973efc6b1d5999a5e8a5b2f3bd
Author: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Date: Thu Dec 22 23:37:24 2005 +0100
git-clone: Support changing the origin branch with -o
Earlier, git-clone stored upstream's master in the branch named 'origin',
possibly overwriting an existing such branch.
Now you can change it by calling git-clone with '-o <other_name>'.
[jc: added ref format check, subdirectory safety, documentation
and usage string.]
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Junio C Hamano <junkio@cox.net>
It sounds like the original need for the -o option is no longer pertinent.
OTOH, for folks who deal with several remotes it's nice to name them, and
"origin" isn't necessarily a useful or intuitive name.
M.
[1] http://thread.gmane.org/gmane.comp.version-control.git/200145
^ permalink raw reply
* Re: Git build fails on `make`, undefined references in libcrypto.a.
From: zero modulo @ 2013-03-18 16:04 UTC (permalink / raw)
To: Konstantin Khomoutov; +Cc: git
In-Reply-To: <20130318072933.GC5434@localhost.localdomain>
On Mon, Mar 18, 2013 at 1:29 AM, Konstantin Khomoutov
<kostix+git@007spb.ru> wrote:
> FYI, I've already tried to answer this exact question [1] with no
> comments from the OP.
>
> 1. http://serverfault.com/a/488604/118848
It is I who posted that question. :P
I haven't made any comments yet because this issue is still a work in
progress. I re-compiled OpenSSL taking into consideration that libdl.a
probably wasn't linked properly to libcrypto.a, and have also tried
re-compiling Git afterward, but with the same errors. I have also
created a .conf file for `ld.so`, and ran `sudo ldconfig` which solved
some other issues I was having. Running `ldd msgfmt` revealed that
run-time libraries were not being found, and after running `sudo
ldconfig`, `ldd msgfmt` then showed the libgettext .so was linked. So,
I solved some of my issues, and if there was an issue with statically
linked libraries not being found, as in this case, then, I believe
re-compiling OpenSSL with the proper LDFLAGS and CPPFLAGS would have
solved the issue, but they have not.
I'm currently attempting to install GCC 4.7.2, which is having some
other issues with texinfo 5.1. I can't find the link, but I someone
said it could be the compiler version... since everything else that
seems like might be the issue isn't fixing it, I'm going to try
re-compiling OpenSSL with GCC 4.7.2 and see how that goes.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox