Re: [PATCH 14/21] Convert ce_path_match() to use struct pathspec

4 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 14/21] Convert ce_path_match() to use struct pathspec

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:50:16

Nguyễn Thái Ngọc Duy  [off-list ref] writes:

quoted hunk

diff --git a/diff-lib.c b/diff-lib.c
index 3b809f2..63db7f4 100644
--- a/diff-lib.c
+++ b/diff-lib.c

@@ -89,9 +89,11 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 	int silent_on_removed = option & DIFF_SILENT_ON_REMOVED;
 	unsigned ce_option = ((option & DIFF_RACY_IS_MODIFIED)
 			      ? CE_MATCH_RACY_IS_DIRTY : 0);
+	struct pathspec pathspec;
 
 	diff_set_mnemonic_prefix(&revs->diffopt, "i/", "w/");
 
+	init_pathspec(&pathspec, revs->prune_data);

I wonder if it makes more sense to change the type of revs->prune_data
from an array of pointers to strings to a pointer to struct pathspec.
Is there a downside?

quoted hunk

diff --git a/preload-index.c b/preload-index.c
index e3d0bda..49cb08d 100644
--- a/preload-index.c
+++ b/preload-index.c

@@ -35,7 +35,9 @@ static void *preload_thread(void *_data)
 	struct index_state *index = p->index;
 	struct cache_entry **cep = index->cache + p->offset;
 	struct cache_def cache;
+	struct pathspec pathspec;
 
+	init_pathspec(&pathspec, p->pathspec);

Likewise; would it make the API cleaner to make read_cache_preload() and
read_index_preload() take "const struct pathspec *" instead of the
traditional "const char **"?

Re: [PATCH 14/21] Convert ce_path_match() to use struct pathspec

From: Nguyen Thai Ngoc Duy <hidden>
Date: 2016-06-15 22:50:16

2010/12/17 Junio C Hamano [off-list ref]:

Nguyễn Thái Ngọc Duy  [off-list ref] writes:

quoted

diff --git a/diff-lib.c b/diff-lib.c
index 3b809f2..63db7f4 100644
--- a/diff-lib.c
+++ b/diff-lib.c

@@ -89,9 +89,11 @@ int run_diff_files(struct rev_info *revs, unsigned int option)

      int silent_on_removed = option & DIFF_SILENT_ON_REMOVED;
      unsigned ce_option = ((option & DIFF_RACY_IS_MODIFIED)
                            ? CE_MATCH_RACY_IS_DIRTY : 0);
+     struct pathspec pathspec;

      diff_set_mnemonic_prefix(&revs->diffopt, "i/", "w/");

+     init_pathspec(&pathspec, revs->prune_data);

I wonder if it makes more sense to change the type of revs->prune_data
from an array of pointers to strings to a pointer to struct pathspec.
Is there a downside?

Converting a pointer to another pointer means mis typecasting can
happen and the compiler won't help catching them. I thinking of
changing prune_data to simply struct pathspec. Looks like it breaks
thing.. hm...
-- 
Duy

[PATCH 14/21] struct rev_info: convert prune_data to struct pathspec

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 22:50:16

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 New patch. rev_info->pathspec is now struct pathspec.

 builtin/add.c         |    2 +-
 builtin/diff.c        |   12 ++++--------
 builtin/fast-export.c |    2 +-
 diff-lib.c            |    6 +++---
 revision.c            |   15 ++++++++-------
 revision.h            |    2 +-
 wt-status.c           |    4 ++--
 7 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index 56a4e0a..3fc79a5 100644
--- a/builtin/add.c
+++ b/builtin/add.c

@@ -86,7 +86,7 @@ int add_files_to_cache(const char *prefix, const char **pathspec, int flags)
 	struct rev_info rev;
 	init_revisions(&rev, prefix);
 	setup_revisions(0, NULL, &rev, NULL);
-	rev.prune_data = pathspec;
+	init_pathspec(&rev.prune_data, pathspec);
 	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK;
 	rev.diffopt.format_callback = update_callback;
 	data.flags = flags;

diff --git a/builtin/diff.c b/builtin/diff.c
index 76c42d8..4ebb1b6 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c

@@ -371,14 +371,10 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 		}
 		die("unhandled object '%s' given.", name);
 	}
-	if (rev.prune_data) {
-		const char **pathspec = rev.prune_data;
-		while (*pathspec) {
-			if (!path)
-				path = *pathspec;
-			paths++;
-			pathspec++;
-		}
+	if (rev.prune_data.nr) {
+		if (!path)
+			path = rev.prune_data.items[0].match;
+		paths += rev.prune_data.nr;
 	}
 
 	/*

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index c8fd46b..ba57457 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c

@@ -651,7 +651,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
 	if (import_filename)
 		import_marks(import_filename);
 
-	if (import_filename && revs.prune_data)
+	if (import_filename && revs.prune_data.nr)
 		full_tree = 1;
 
 	get_tags_and_duplicates(&revs.pending, &extra_refs);

diff --git a/diff-lib.c b/diff-lib.c
index 3b809f2..2251f3d 100644
--- a/diff-lib.c
+++ b/diff-lib.c

@@ -106,7 +106,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 			DIFF_OPT_TST(&revs->diffopt, HAS_CHANGES))
 			break;
 
-		if (!ce_path_match(ce, revs->prune_data))
+		if (!ce_path_match(ce, revs->prune_data.raw))
 			continue;
 
 		if (ce_stage(ce)) {

@@ -427,7 +427,7 @@ static int oneway_diff(struct cache_entry **src, struct unpack_trees_options *o)
 	if (tree == o->df_conflict_entry)
 		tree = NULL;
 
-	if (ce_path_match(idx ? idx : tree, revs->prune_data))
+	if (ce_path_match(idx ? idx : tree, revs->prune_data.raw))
 		do_oneway_diff(o, idx, tree);
 
 	return 0;

@@ -501,7 +501,7 @@ int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt)
 	active_nr = dst - active_cache;
 
 	init_revisions(&revs, NULL);
-	revs.prune_data = opt->pathspec.raw;
+	init_pathspec(&revs.prune_data, opt->pathspec.raw);
 	tree = parse_tree_indirect(tree_sha1);
 	if (!tree)
 		die("bad tree object %s", sha1_to_hex(tree_sha1));

diff --git a/revision.c b/revision.c
index b2a5867..515e2dd 100644
--- a/revision.c
+++ b/revision.c

@@ -323,7 +323,7 @@ static int rev_compare_tree(struct rev_info *revs, struct commit *parent, struct
 		 * tagged commit by specifying both --simplify-by-decoration
 		 * and pathspec.
 		 */
-		if (!revs->prune_data)
+		if (!revs->prune_data.nr)
 			return REV_TREE_SAME;
 	}

@@ -969,7 +969,7 @@ static void prepare_show_merge(struct rev_info *revs)
 		struct cache_entry *ce = active_cache[i];
 		if (!ce_stage(ce))
 			continue;
-		if (ce_path_match(ce, revs->prune_data)) {
+		if (ce_path_match(ce, revs->prune_data.raw)) {
 			prune_num++;
 			prune = xrealloc(prune, sizeof(*prune) * prune_num);
 			prune[prune_num-2] = ce->name;

@@ -979,7 +979,8 @@ static void prepare_show_merge(struct rev_info *revs)
 		       ce_same_name(ce, active_cache[i+1]))
 			i++;
 	}
-	revs->prune_data = prune;
+	free_pathspec(&revs->prune_data);
+	init_pathspec(&revs->prune_data, prune);
 	revs->limited = 1;
 }

@@ -1616,7 +1617,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	}
 
 	if (prune_data)
-		revs->prune_data = get_pathspec(revs->prefix, prune_data);
+		init_pathspec(&revs->prune_data, get_pathspec(revs->prefix, prune_data));
 
 	if (revs->def == NULL)
 		revs->def = opt ? opt->def : NULL;

@@ -1647,13 +1648,13 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	if (revs->topo_order)
 		revs->limited = 1;
 
-	if (revs->prune_data) {
-		diff_tree_setup_paths(revs->prune_data, &revs->pruning);
+	if (revs->prune_data.nr) {
+		diff_tree_setup_paths(revs->prune_data.raw, &revs->pruning);
 		/* Can't prune commits with rename following: the paths change.. */
 		if (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
 			revs->prune = 1;
 		if (!revs->full_diff)
-			diff_tree_setup_paths(revs->prune_data, &revs->diffopt);
+			diff_tree_setup_paths(revs->prune_data.raw, &revs->diffopt);
 	}
 	if (revs->combine_merges)
 		revs->ignore_merges = 0;

diff --git a/revision.h b/revision.h
index 05659c6..82509dd 100644
--- a/revision.h
+++ b/revision.h

@@ -34,7 +34,7 @@ struct rev_info {
 	/* Basic information */
 	const char *prefix;
 	const char *def;
-	void *prune_data;
+	struct pathspec prune_data;
 	unsigned int early_output;
 
 	/* Traversal flags */

diff --git a/wt-status.c b/wt-status.c
index 54b6b03..5c6b118 100644
--- a/wt-status.c
+++ b/wt-status.c

@@ -319,7 +319,7 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)
     }
 	rev.diffopt.format_callback = wt_status_collect_changed_cb;
 	rev.diffopt.format_callback_data = s;
-	rev.prune_data = s->pathspec;
+	init_pathspec(&rev.prune_data, s->pathspec);
 	run_diff_files(&rev, 0);
 }

@@ -344,7 +344,7 @@ static void wt_status_collect_changes_index(struct wt_status *s)
 	rev.diffopt.detect_rename = 1;
 	rev.diffopt.rename_limit = 200;
 	rev.diffopt.break_opt = 0;
-	rev.prune_data = s->pathspec;
+	init_pathspec(&rev.prune_data, s->pathspec);
 	run_diff_index(&rev, 1);
 }

-- 
1.7.3.3.476.g10a82

[PATCH 15/21] Convert ce_path_match() to use struct pathspec

From: Nguyễn Thái Ngọc Duy <hidden>
Date: 2016-06-15 22:50:16

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 The old 14/21. Slightly changed because of rev_info->pathspec change.

 builtin/update-index.c |    8 ++++++--
 cache.h                |    2 +-
 diff-lib.c             |    4 ++--
 preload-index.c        |    5 ++++-
 read-cache.c           |    7 ++++---
 revision.c             |    2 +-
 wt-status.c            |    5 ++++-
 7 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 3ab214d..9d1f67e 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c

@@ -543,7 +543,10 @@ static int do_reupdate(int ac, const char **av,
 	 */
 	int pos;
 	int has_head = 1;
-	const char **pathspec = get_pathspec(prefix, av + 1);
+	const char **paths = get_pathspec(prefix, av + 1);
+	struct pathspec pathspec;
+
+	init_pathspec(&pathspec, paths);
 
 	if (read_ref("HEAD", head_sha1))
 		/* If there is no HEAD, that means it is an initial

@@ -556,7 +559,7 @@ static int do_reupdate(int ac, const char **av,
 		struct cache_entry *old = NULL;
 		int save_nr;
 
-		if (ce_stage(ce) || !ce_path_match(ce, pathspec))
+		if (ce_stage(ce) || !ce_path_match(ce, &pathspec))
 			continue;
 		if (has_head)
 			old = read_one_ent(NULL, head_sha1,

@@ -575,6 +578,7 @@ static int do_reupdate(int ac, const char **av,
 		if (save_nr != active_nr)
 			goto redo;
 	}
+	free_pathspec(&pathspec);
 	return 0;
 }

diff --git a/cache.h b/cache.h
index dc0bfb4..b5cd61c 100644
--- a/cache.h
+++ b/cache.h

@@ -508,7 +508,7 @@ struct pathspec {
 
 extern int init_pathspec(struct pathspec *, const char **);
 extern void free_pathspec(struct pathspec *);
-extern int ce_path_match(const struct cache_entry *ce, const char **pathspec);
+extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec);
 extern int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object, enum object_type type, const char *path);
 extern int index_path(unsigned char *sha1, const char *path, struct stat *st, int write_object);
 extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st);

diff --git a/diff-lib.c b/diff-lib.c
index 2251f3d..1e22992 100644
--- a/diff-lib.c
+++ b/diff-lib.c

@@ -106,7 +106,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 			DIFF_OPT_TST(&revs->diffopt, HAS_CHANGES))
 			break;
 
-		if (!ce_path_match(ce, revs->prune_data.raw))
+		if (!ce_path_match(ce, &revs->prune_data))
 			continue;
 
 		if (ce_stage(ce)) {

@@ -427,7 +427,7 @@ static int oneway_diff(struct cache_entry **src, struct unpack_trees_options *o)
 	if (tree == o->df_conflict_entry)
 		tree = NULL;
 
-	if (ce_path_match(idx ? idx : tree, revs->prune_data.raw))
+	if (ce_path_match(idx ? idx : tree, &revs->prune_data))
 		do_oneway_diff(o, idx, tree);
 
 	return 0;

diff --git a/preload-index.c b/preload-index.c
index e3d0bda..49cb08d 100644
--- a/preload-index.c
+++ b/preload-index.c

@@ -35,7 +35,9 @@ static void *preload_thread(void *_data)
 	struct index_state *index = p->index;
 	struct cache_entry **cep = index->cache + p->offset;
 	struct cache_def cache;
+	struct pathspec pathspec;
 
+	init_pathspec(&pathspec, p->pathspec);
 	memset(&cache, 0, sizeof(cache));
 	nr = p->nr;
 	if (nr + p->offset > index->cache_nr)

@@ -51,7 +53,7 @@ static void *preload_thread(void *_data)
 			continue;
 		if (ce_uptodate(ce))
 			continue;
-		if (!ce_path_match(ce, p->pathspec))
+		if (!ce_path_match(ce, &pathspec))
 			continue;
 		if (threaded_has_symlink_leading_path(&cache, ce->name, ce_namelen(ce)))
 			continue;

@@ -61,6 +63,7 @@ static void *preload_thread(void *_data)
 			continue;
 		ce_mark_uptodate(ce);
 	} while (--nr > 0);
+	free_pathspec(&pathspec);
 	return NULL;
 }

diff --git a/read-cache.c b/read-cache.c
index 1f42473..f1141a3 100644
--- a/read-cache.c
+++ b/read-cache.c

@@ -683,17 +683,18 @@ int ce_same_name(struct cache_entry *a, struct cache_entry *b)
 	return ce_namelen(b) == len && !memcmp(a->name, b->name, len);
 }
 
-int ce_path_match(const struct cache_entry *ce, const char **pathspec)
+int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec)
 {
 	const char *match, *name;
+	const char **ps = pathspec->raw;
 	int len;
 
-	if (!pathspec)
+	if (!pathspec->nr)
 		return 1;
 
 	len = ce_namelen(ce);
 	name = ce->name;
-	while ((match = *pathspec++) != NULL) {
+	while ((match = *ps++) != NULL) {
 		int matchlen = strlen(match);
 		if (matchlen > len)
 			continue;

diff --git a/revision.c b/revision.c
index 515e2dd..a0d3816 100644
--- a/revision.c
+++ b/revision.c

@@ -969,7 +969,7 @@ static void prepare_show_merge(struct rev_info *revs)
 		struct cache_entry *ce = active_cache[i];
 		if (!ce_stage(ce))
 			continue;
-		if (ce_path_match(ce, revs->prune_data.raw)) {
+		if (ce_path_match(ce, &revs->prune_data)) {
 			prune_num++;
 			prune = xrealloc(prune, sizeof(*prune) * prune_num);
 			prune[prune_num-2] = ce->name;

diff --git a/wt-status.c b/wt-status.c
index 5c6b118..457d265 100644
--- a/wt-status.c
+++ b/wt-status.c

@@ -350,14 +350,16 @@ static void wt_status_collect_changes_index(struct wt_status *s)
 
 static void wt_status_collect_changes_initial(struct wt_status *s)
 {
+	struct pathspec pathspec;
 	int i;
 
+	init_pathspec(&pathspec, s->pathspec);
 	for (i = 0; i < active_nr; i++) {
 		struct string_list_item *it;
 		struct wt_status_change_data *d;
 		struct cache_entry *ce = active_cache[i];
 
-		if (!ce_path_match(ce, s->pathspec))
+		if (!ce_path_match(ce, &pathspec))
 			continue;
 		it = string_list_insert(&s->change, ce->name);
 		d = it->util;

@@ -372,6 +374,7 @@ static void wt_status_collect_changes_initial(struct wt_status *s)
 		else
 			d->index_status = DIFF_STATUS_ADDED;
 	}
+	free_pathspec(&pathspec);
 }
 
 static void wt_status_collect_untracked(struct wt_status *s)

-- 
1.7.3.3.476.g10a82

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help