git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
* [PATCH] Create 'bisect_flags' parameter in find_bisection() in preparation of passing multiple options
@ 2018-04-15  8:58 Harald Nordgren
  2018-04-15  9:55 ` Christian Couder
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Harald Nordgren @ 2018-04-15  8:58 UTC (permalink / raw)
  To: git, Johannes.Schindelin, tiago, christian.couder, sbeller
  Cc: Harald Nordgren

---

Notes:
    Preperatory patch to enable either Tiago Botelho's or my patch, to do bisection on first parents / merge commits

 bisect.c           | 21 ++++++++++++---------
 bisect.h           |  5 +++--
 builtin/rev-list.c |  6 +++---
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/bisect.c b/bisect.c
index a579b50884..d85550fd89 100644
--- a/bisect.c
+++ b/bisect.c
@@ -254,7 +254,7 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n
  */
 static struct commit_list *do_find_bisection(struct commit_list *list,
 					     int nr, int *weights,
-					     int find_all)
+					     int bisect_flags)
 {
 	int n, counted;
 	struct commit_list *p;
@@ -313,7 +313,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
 		clear_distance(list);
 
 		/* Does it happen to be at exactly half-way? */
-		if (!find_all && halfway(p, nr))
+		if (!(bisect_flags & BISECT_FIND_ALL) && halfway(p, nr))
 			return p;
 		counted++;
 	}
@@ -351,21 +351,21 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
 				weight_set(p, weight(q));
 
 			/* Does it happen to be at exactly half-way? */
-			if (!find_all && halfway(p, nr))
+			if (!(bisect_flags & BISECT_FIND_ALL) && halfway(p, nr))
 				return p;
 		}
 	}
 
 	show_list("bisection 2 counted all", counted, nr, list);
 
-	if (!find_all)
+	if (!(bisect_flags & BISECT_FIND_ALL))
 		return best_bisection(list, nr);
 	else
 		return best_bisection_sorted(list, nr);
 }
 
 void find_bisection(struct commit_list **commit_list, int *reaches,
-		    int *all, int find_all)
+		    int *all, int bisect_flags)
 {
 	int nr, on_list;
 	struct commit_list *list, *p, *best, *next, *last;
@@ -400,9 +400,9 @@ void find_bisection(struct commit_list **commit_list, int *reaches,
 	weights = xcalloc(on_list, sizeof(*weights));
 
 	/* Do the real work of finding bisection commit. */
-	best = do_find_bisection(list, nr, weights, find_all);
+	best = do_find_bisection(list, nr, weights, bisect_flags);
 	if (best) {
-		if (!find_all) {
+		if (!(bisect_flags & BISECT_FIND_ALL)) {
 			list->item = best->item;
 			free_commit_list(list->next);
 			best = list;
@@ -942,7 +942,7 @@ int bisect_next_all(const char *prefix, int no_checkout)
 {
 	struct rev_info revs;
 	struct commit_list *tried;
-	int reaches = 0, all = 0, nr, steps;
+	int reaches = 0, all = 0, bisect_flags = 0, nr, steps;
 	struct object_id *bisect_rev;
 	char *steps_msg;
 
@@ -957,7 +957,10 @@ int bisect_next_all(const char *prefix, int no_checkout)
 
 	bisect_common(&revs);
 
-	find_bisection(&revs.commits, &reaches, &all, !!skipped_revs.nr);
+	if (skipped_revs.nr)
+		bisect_flags |= BISECT_FIND_ALL;
+
+	find_bisection(&revs.commits, &reaches, &all, bisect_flags);
 	revs.commits = managed_skipped(revs.commits, &tried);
 
 	if (!revs.commits) {
diff --git a/bisect.h b/bisect.h
index a5d9248a47..d46b078871 100644
--- a/bisect.h
+++ b/bisect.h
@@ -6,10 +6,10 @@
  * commits that the best commit reaches. `all` will be the count of
  * non-SAMETREE commits. If nothing is found, `list` will be NULL.
  * Otherwise, it will be either all non-SAMETREE commits or the single
- * best commit, as chosen by `find_all`.
+ * best commit, as chosen by flag `BISECT_FIND_ALL`.
  */
 extern void find_bisection(struct commit_list **list, int *reaches, int *all,
-			   int find_all);
+			   int bisect_flags);
 
 extern struct commit_list *filter_skipped(struct commit_list *list,
 					  struct commit_list **tried,
@@ -19,6 +19,7 @@ extern struct commit_list *filter_skipped(struct commit_list *list,
 
 #define BISECT_SHOW_ALL		(1<<0)
 #define REV_LIST_QUIET		(1<<1)
+#define BISECT_FIND_ALL		(1<<2)
 
 struct rev_list_info {
 	struct rev_info *revs;
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index fadd3ec14c..88700e897d 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -360,7 +360,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	int i;
 	int bisect_list = 0;
 	int bisect_show_vars = 0;
-	int bisect_find_all = 0;
+	int bisect_flags = 0;
 	int use_bitmap_index = 0;
 	const char *show_progress = NULL;
 
@@ -426,7 +426,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 		}
 		if (!strcmp(arg, "--bisect-all")) {
 			bisect_list = 1;
-			bisect_find_all = 1;
+			bisect_flags |= BISECT_FIND_ALL;
 			info.flags |= BISECT_SHOW_ALL;
 			revs.show_decorations = 1;
 			continue;
@@ -538,7 +538,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	if (bisect_list) {
 		int reaches, all;
 
-		find_bisection(&revs.commits, &reaches, &all, bisect_find_all);
+		find_bisection(&revs.commits, &reaches, &all, bisect_flags);
 
 		if (bisect_show_vars)
 			return show_bisect_vars(&info, reaches, all);
-- 
2.14.3 (Apple Git-98)


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Create 'bisect_flags' parameter in find_bisection() in preparation of passing multiple options
  2018-04-15  8:58 [PATCH] Create 'bisect_flags' parameter in find_bisection() in preparation of passing multiple options Harald Nordgren
@ 2018-04-15  9:55 ` Christian Couder
  2018-04-15 10:44 ` [PATCH v2] Make it possible to implement bisecting only on first parents or on merge commits by passing flags to find_bisection(), instead of just a find_all boolean Harald Nordgren
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Christian Couder @ 2018-04-15  9:55 UTC (permalink / raw)
  To: Harald Nordgren; +Cc: git, Johannes Schindelin, Tiago Botelho, Stefan Beller

Hi,

On Sun, Apr 15, 2018 at 10:58 AM, Harald Nordgren
<haraldnordgren@gmail.com> wrote:
> ---
>
> Notes:
>     Preperatory patch to enable either Tiago Botelho's or my patch, to do bisection on first parents / merge commits

It would be nice if you could move some part of the above note into
the commit message (above the ---). For example:

"Make it possible to implement bisecting only on first parents or on
merge commits by passing flags to find_bisection(), instead of just a
find_all boolean".

While at it maybe the subject line of the commit message could start
with "bisect: create 'bisect_flags' parameter ..." so that we can
quickly tell which area of the code it is about.

>  bisect.c           | 21 ++++++++++++---------
>  bisect.h           |  5 +++--
>  builtin/rev-list.c |  6 +++---
>  3 files changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/bisect.c b/bisect.c
> index a579b50884..d85550fd89 100644
> --- a/bisect.c
> +++ b/bisect.c
> @@ -254,7 +254,7 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n
>   */
>  static struct commit_list *do_find_bisection(struct commit_list *list,
>                                              int nr, int *weights,
> -                                            int find_all)
> +                                            int bisect_flags)

I think it's better to use "unsigned int" rather than just "int" for flags.

>  {
>         int n, counted;
>         struct commit_list *p;

[...]

> @@ -19,6 +19,7 @@ extern struct commit_list *filter_skipped(struct commit_list *list,
>
>  #define BISECT_SHOW_ALL                (1<<0)
>  #define REV_LIST_QUIET         (1<<1)
> +#define BISECT_FIND_ALL                (1<<2)

Is BISECT_FIND_ALL really related to the other flags, or is this
mixing rev list flags with bisect flags?

Thanks for working on this,
Christian.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] Make it possible to implement bisecting only on first parents or on merge commits by passing flags to find_bisection(), instead of just a find_all boolean
  2018-04-15  8:58 [PATCH] Create 'bisect_flags' parameter in find_bisection() in preparation of passing multiple options Harald Nordgren
  2018-04-15  9:55 ` Christian Couder
@ 2018-04-15 10:44 ` Harald Nordgren
  2018-04-15 13:35   ` Christian Couder
  2018-04-15 13:49 ` [PATCH v3] bisect: create 'bisect_flags' parameter in find_bisection() Harald Nordgren
  2018-04-18 21:05 ` [PATCH v4] " Harald Nordgren
  3 siblings, 1 reply; 9+ messages in thread
From: Harald Nordgren @ 2018-04-15 10:44 UTC (permalink / raw)
  To: git, Johannes.Schindelin, tiago, christian.couder, sbeller
  Cc: Harald Nordgren

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---

Notes:
    Updates according to Christian Couder's code review

 bisect.c           | 20 ++++++++++++--------
 bisect.h           |  6 ++++--
 builtin/rev-list.c |  6 +++---
 3 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/bisect.c b/bisect.c
index a579b50884..19dac7491d 100644
--- a/bisect.c
+++ b/bisect.c
@@ -254,7 +254,7 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n
  */
 static struct commit_list *do_find_bisection(struct commit_list *list,
 					     int nr, int *weights,
-					     int find_all)
+					     unsigned int bisect_flags)
 {
 	int n, counted;
 	struct commit_list *p;
@@ -313,7 +313,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
 		clear_distance(list);
 
 		/* Does it happen to be at exactly half-way? */
-		if (!find_all && halfway(p, nr))
+		if (!(bisect_flags & BISECT_FIND_ALL) && halfway(p, nr))
 			return p;
 		counted++;
 	}
@@ -351,21 +351,21 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
 				weight_set(p, weight(q));
 
 			/* Does it happen to be at exactly half-way? */
-			if (!find_all && halfway(p, nr))
+			if (!(bisect_flags & BISECT_FIND_ALL) && halfway(p, nr))
 				return p;
 		}
 	}
 
 	show_list("bisection 2 counted all", counted, nr, list);
 
-	if (!find_all)
+	if (!(bisect_flags & BISECT_FIND_ALL))
 		return best_bisection(list, nr);
 	else
 		return best_bisection_sorted(list, nr);
 }
 
 void find_bisection(struct commit_list **commit_list, int *reaches,
-		    int *all, int find_all)
+		    int *all, unsigned int bisect_flags)
 {
 	int nr, on_list;
 	struct commit_list *list, *p, *best, *next, *last;
@@ -400,9 +400,9 @@ void find_bisection(struct commit_list **commit_list, int *reaches,
 	weights = xcalloc(on_list, sizeof(*weights));
 
 	/* Do the real work of finding bisection commit. */
-	best = do_find_bisection(list, nr, weights, find_all);
+	best = do_find_bisection(list, nr, weights, bisect_flags);
 	if (best) {
-		if (!find_all) {
+		if (!(bisect_flags & BISECT_FIND_ALL)) {
 			list->item = best->item;
 			free_commit_list(list->next);
 			best = list;
@@ -943,6 +943,7 @@ int bisect_next_all(const char *prefix, int no_checkout)
 	struct rev_info revs;
 	struct commit_list *tried;
 	int reaches = 0, all = 0, nr, steps;
+	unsigned int bisect_flags = 0;
 	struct object_id *bisect_rev;
 	char *steps_msg;
 
@@ -957,7 +958,10 @@ int bisect_next_all(const char *prefix, int no_checkout)
 
 	bisect_common(&revs);
 
-	find_bisection(&revs.commits, &reaches, &all, !!skipped_revs.nr);
+	if (skipped_revs.nr)
+		bisect_flags |= BISECT_FIND_ALL;
+
+	find_bisection(&revs.commits, &reaches, &all, bisect_flags);
 	revs.commits = managed_skipped(revs.commits, &tried);
 
 	if (!revs.commits) {
diff --git a/bisect.h b/bisect.h
index a5d9248a47..8efe243a34 100644
--- a/bisect.h
+++ b/bisect.h
@@ -1,15 +1,17 @@
 #ifndef BISECT_H
 #define BISECT_H
 
+#define BISECT_FIND_ALL		(1u<<0)
+
 /*
  * Find bisection. If something is found, `reaches` will be the number of
  * commits that the best commit reaches. `all` will be the count of
  * non-SAMETREE commits. If nothing is found, `list` will be NULL.
  * Otherwise, it will be either all non-SAMETREE commits or the single
- * best commit, as chosen by `find_all`.
+ * best commit, as chosen by flag `BISECT_FIND_ALL`.
  */
 extern void find_bisection(struct commit_list **list, int *reaches, int *all,
-			   int find_all);
+			   unsigned int bisect_flags);
 
 extern struct commit_list *filter_skipped(struct commit_list *list,
 					  struct commit_list **tried,
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index fadd3ec14c..22c3d479fb 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -360,8 +360,8 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	int i;
 	int bisect_list = 0;
 	int bisect_show_vars = 0;
-	int bisect_find_all = 0;
 	int use_bitmap_index = 0;
+	unsigned int bisect_flags = 0;
 	const char *show_progress = NULL;
 
 	if (argc == 2 && !strcmp(argv[1], "-h"))
@@ -426,7 +426,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 		}
 		if (!strcmp(arg, "--bisect-all")) {
 			bisect_list = 1;
-			bisect_find_all = 1;
+			bisect_flags |= BISECT_FIND_ALL;
 			info.flags |= BISECT_SHOW_ALL;
 			revs.show_decorations = 1;
 			continue;
@@ -538,7 +538,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	if (bisect_list) {
 		int reaches, all;
 
-		find_bisection(&revs.commits, &reaches, &all, bisect_find_all);
+		find_bisection(&revs.commits, &reaches, &all, bisect_flags);
 
 		if (bisect_show_vars)
 			return show_bisect_vars(&info, reaches, all);
-- 
2.14.3 (Apple Git-98)


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] Make it possible to implement bisecting only on first parents or on merge commits by passing flags to find_bisection(), instead of just a find_all boolean
  2018-04-15 10:44 ` [PATCH v2] Make it possible to implement bisecting only on first parents or on merge commits by passing flags to find_bisection(), instead of just a find_all boolean Harald Nordgren
@ 2018-04-15 13:35   ` Christian Couder
  0 siblings, 0 replies; 9+ messages in thread
From: Christian Couder @ 2018-04-15 13:35 UTC (permalink / raw)
  To: Harald Nordgren; +Cc: git, Johannes Schindelin, Tiago Botelho, Stefan Beller

On Sun, Apr 15, 2018 at 12:44 PM, Harald Nordgren
<haraldnordgren@gmail.com> wrote:
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
>
> Notes:
>     Updates according to Christian Couder's code review

About the commit message (its first line and its body), I was
suggesting a patch like the following:

From 3a0ced0953ccc6038a43885d98a507eb18c19e42 Mon Sep 17 00:00:00 2001
From: Harald Nordgren <haraldnordgren@gmail.com>
Date: Sun, 15 Apr 2018 12:44:38 +0200
Subject: [PATCH] bisect: create 'bisect_flags' parameter in find_bisection()

Make it possible to implement bisecting only on first parents or on
merge commits by passing flags to find_bisection(), instead of just
a 'find_all' boolean.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 bisect.c           | 20 ++++++++++++--------
 bisect.h           |  6 ++++--
 builtin/rev-list.c |  6 +++---
 3 files changed, 19 insertions(+), 13 deletions(-)
...

The rest of the patch looks good to me.

Thanks!

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3] bisect: create 'bisect_flags' parameter in find_bisection()
  2018-04-15  8:58 [PATCH] Create 'bisect_flags' parameter in find_bisection() in preparation of passing multiple options Harald Nordgren
  2018-04-15  9:55 ` Christian Couder
  2018-04-15 10:44 ` [PATCH v2] Make it possible to implement bisecting only on first parents or on merge commits by passing flags to find_bisection(), instead of just a find_all boolean Harald Nordgren
@ 2018-04-15 13:49 ` Harald Nordgren
  2018-04-17  2:15   ` Junio C Hamano
  2018-04-18 20:31   ` Johannes Schindelin
  2018-04-18 21:05 ` [PATCH v4] " Harald Nordgren
  3 siblings, 2 replies; 9+ messages in thread
From: Harald Nordgren @ 2018-04-15 13:49 UTC (permalink / raw)
  To: git, Johannes.Schindelin, tiago, christian.couder, sbeller
  Cc: Harald Nordgren

Make it possible to implement bisecting only on first parents or on
merge commits by passing flags to find_bisection(), instead of just
a 'find_all' boolean.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---

Notes:
    Updating commit message

 bisect.c           | 20 ++++++++++++--------
 bisect.h           |  6 ++++--
 builtin/rev-list.c |  6 +++---
 3 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/bisect.c b/bisect.c
index a579b50884..19dac7491d 100644
--- a/bisect.c
+++ b/bisect.c
@@ -254,7 +254,7 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n
  */
 static struct commit_list *do_find_bisection(struct commit_list *list,
 					     int nr, int *weights,
-					     int find_all)
+					     unsigned int bisect_flags)
 {
 	int n, counted;
 	struct commit_list *p;
@@ -313,7 +313,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
 		clear_distance(list);
 
 		/* Does it happen to be at exactly half-way? */
-		if (!find_all && halfway(p, nr))
+		if (!(bisect_flags & BISECT_FIND_ALL) && halfway(p, nr))
 			return p;
 		counted++;
 	}
@@ -351,21 +351,21 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
 				weight_set(p, weight(q));
 
 			/* Does it happen to be at exactly half-way? */
-			if (!find_all && halfway(p, nr))
+			if (!(bisect_flags & BISECT_FIND_ALL) && halfway(p, nr))
 				return p;
 		}
 	}
 
 	show_list("bisection 2 counted all", counted, nr, list);
 
-	if (!find_all)
+	if (!(bisect_flags & BISECT_FIND_ALL))
 		return best_bisection(list, nr);
 	else
 		return best_bisection_sorted(list, nr);
 }
 
 void find_bisection(struct commit_list **commit_list, int *reaches,
-		    int *all, int find_all)
+		    int *all, unsigned int bisect_flags)
 {
 	int nr, on_list;
 	struct commit_list *list, *p, *best, *next, *last;
@@ -400,9 +400,9 @@ void find_bisection(struct commit_list **commit_list, int *reaches,
 	weights = xcalloc(on_list, sizeof(*weights));
 
 	/* Do the real work of finding bisection commit. */
-	best = do_find_bisection(list, nr, weights, find_all);
+	best = do_find_bisection(list, nr, weights, bisect_flags);
 	if (best) {
-		if (!find_all) {
+		if (!(bisect_flags & BISECT_FIND_ALL)) {
 			list->item = best->item;
 			free_commit_list(list->next);
 			best = list;
@@ -943,6 +943,7 @@ int bisect_next_all(const char *prefix, int no_checkout)
 	struct rev_info revs;
 	struct commit_list *tried;
 	int reaches = 0, all = 0, nr, steps;
+	unsigned int bisect_flags = 0;
 	struct object_id *bisect_rev;
 	char *steps_msg;
 
@@ -957,7 +958,10 @@ int bisect_next_all(const char *prefix, int no_checkout)
 
 	bisect_common(&revs);
 
-	find_bisection(&revs.commits, &reaches, &all, !!skipped_revs.nr);
+	if (skipped_revs.nr)
+		bisect_flags |= BISECT_FIND_ALL;
+
+	find_bisection(&revs.commits, &reaches, &all, bisect_flags);
 	revs.commits = managed_skipped(revs.commits, &tried);
 
 	if (!revs.commits) {
diff --git a/bisect.h b/bisect.h
index a5d9248a47..8efe243a34 100644
--- a/bisect.h
+++ b/bisect.h
@@ -1,15 +1,17 @@
 #ifndef BISECT_H
 #define BISECT_H
 
+#define BISECT_FIND_ALL		(1u<<0)
+
 /*
  * Find bisection. If something is found, `reaches` will be the number of
  * commits that the best commit reaches. `all` will be the count of
  * non-SAMETREE commits. If nothing is found, `list` will be NULL.
  * Otherwise, it will be either all non-SAMETREE commits or the single
- * best commit, as chosen by `find_all`.
+ * best commit, as chosen by flag `BISECT_FIND_ALL`.
  */
 extern void find_bisection(struct commit_list **list, int *reaches, int *all,
-			   int find_all);
+			   unsigned int bisect_flags);
 
 extern struct commit_list *filter_skipped(struct commit_list *list,
 					  struct commit_list **tried,
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index fadd3ec14c..22c3d479fb 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -360,8 +360,8 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	int i;
 	int bisect_list = 0;
 	int bisect_show_vars = 0;
-	int bisect_find_all = 0;
 	int use_bitmap_index = 0;
+	unsigned int bisect_flags = 0;
 	const char *show_progress = NULL;
 
 	if (argc == 2 && !strcmp(argv[1], "-h"))
@@ -426,7 +426,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 		}
 		if (!strcmp(arg, "--bisect-all")) {
 			bisect_list = 1;
-			bisect_find_all = 1;
+			bisect_flags |= BISECT_FIND_ALL;
 			info.flags |= BISECT_SHOW_ALL;
 			revs.show_decorations = 1;
 			continue;
@@ -538,7 +538,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	if (bisect_list) {
 		int reaches, all;
 
-		find_bisection(&revs.commits, &reaches, &all, bisect_find_all);
+		find_bisection(&revs.commits, &reaches, &all, bisect_flags);
 
 		if (bisect_show_vars)
 			return show_bisect_vars(&info, reaches, all);
-- 
2.14.3 (Apple Git-98)


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] bisect: create 'bisect_flags' parameter in find_bisection()
  2018-04-15 13:49 ` [PATCH v3] bisect: create 'bisect_flags' parameter in find_bisection() Harald Nordgren
@ 2018-04-17  2:15   ` Junio C Hamano
  2018-04-18 20:31   ` Johannes Schindelin
  1 sibling, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2018-04-17  2:15 UTC (permalink / raw)
  To: Harald Nordgren
  Cc: git, Johannes.Schindelin, tiago, christian.couder, sbeller

Harald Nordgren <haraldnordgren@gmail.com> writes:

> Make it possible to implement bisecting only on first parents or on
> merge commits by passing flags to find_bisection(), instead of just
> a 'find_all' boolean.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
>
> Notes:
>     Updating commit message

Looks like a reasonable preparatory step *if* we want to later be
able to pass other options down the same callchain.  It would be
nice to have the second step that this praparatory step is meant to
help as a follow-up (or 2/2 of a two-patch series, where this one is
1/2) not in too distant future.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] bisect: create 'bisect_flags' parameter in find_bisection()
  2018-04-15 13:49 ` [PATCH v3] bisect: create 'bisect_flags' parameter in find_bisection() Harald Nordgren
  2018-04-17  2:15   ` Junio C Hamano
@ 2018-04-18 20:31   ` Johannes Schindelin
  1 sibling, 0 replies; 9+ messages in thread
From: Johannes Schindelin @ 2018-04-18 20:31 UTC (permalink / raw)
  To: Harald Nordgren; +Cc: git, tiago, christian.couder, sbeller

Hi Harald,

On Sun, 15 Apr 2018, Harald Nordgren wrote:

> Make it possible to implement bisecting only on first parents or on
> merge commits by passing flags to find_bisection(), instead of just
> a 'find_all' boolean.
> 
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>

Very nice. Just two little suggestions:

> diff --git a/bisect.c b/bisect.c
> index a579b50884..19dac7491d 100644
> --- a/bisect.c
> +++ b/bisect.c
> @@ -254,7 +254,7 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n
>   */
>  static struct commit_list *do_find_bisection(struct commit_list *list,
>  					     int nr, int *weights,
> -					     int find_all)
> +					     unsigned int bisect_flags)

For flags, we frequently seem to use the type `unsigned` (which is the
same as `unsigned int`, just shorter).

>  {
>  	int n, counted;
>  	struct commit_list *p;
> @@ -313,7 +313,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
>  		clear_distance(list);
>  
>  		/* Does it happen to be at exactly half-way? */
> -		if (!find_all && halfway(p, nr))
> +		if (!(bisect_flags & BISECT_FIND_ALL) && halfway(p, nr))

Rather than expanding the short & sweet `find_all` to `(bisect_flags &
BISECT_FIND_ALL)` in three places in this function, I would rather just
define this local variable in the beginning:

	unsigned int find_all = bisect_flags & BISECT_FIND_ALL;

This would also reduce the size of the diff.

All in all, very nice!

Ciao,
Johannes

P.S.: I fully agree with Junio and eagerly await what comes next ;-)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v4] bisect: create 'bisect_flags' parameter in find_bisection()
  2018-04-15  8:58 [PATCH] Create 'bisect_flags' parameter in find_bisection() in preparation of passing multiple options Harald Nordgren
                   ` (2 preceding siblings ...)
  2018-04-15 13:49 ` [PATCH v3] bisect: create 'bisect_flags' parameter in find_bisection() Harald Nordgren
@ 2018-04-18 21:05 ` " Harald Nordgren
  2018-04-20 12:23   ` Johannes Schindelin
  3 siblings, 1 reply; 9+ messages in thread
From: Harald Nordgren @ 2018-04-18 21:05 UTC (permalink / raw)
  To: git, Johannes.Schindelin, tiago, christian.couder, sbeller
  Cc: Harald Nordgren

Make it possible to implement bisecting only on first parents or on
merge commits by passing flags to find_bisection(), instead of just
a 'find_all' boolean.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---

Notes:
    Use unsigned type and cache flag value

 bisect.c           | 15 ++++++++++-----
 bisect.h           |  6 ++++--
 builtin/rev-list.c |  6 +++---
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/bisect.c b/bisect.c
index a579b50884..4eafc8262b 100644
--- a/bisect.c
+++ b/bisect.c
@@ -254,9 +254,10 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n
  */
 static struct commit_list *do_find_bisection(struct commit_list *list,
 					     int nr, int *weights,
-					     int find_all)
+					     unsigned bisect_flags)
 {
 	int n, counted;
+	unsigned find_all = bisect_flags & BISECT_FIND_ALL;
 	struct commit_list *p;
 
 	counted = 0;
@@ -365,7 +366,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list,
 }
 
 void find_bisection(struct commit_list **commit_list, int *reaches,
-		    int *all, int find_all)
+		    int *all, unsigned bisect_flags)
 {
 	int nr, on_list;
 	struct commit_list *list, *p, *best, *next, *last;
@@ -400,9 +401,9 @@ void find_bisection(struct commit_list **commit_list, int *reaches,
 	weights = xcalloc(on_list, sizeof(*weights));
 
 	/* Do the real work of finding bisection commit. */
-	best = do_find_bisection(list, nr, weights, find_all);
+	best = do_find_bisection(list, nr, weights, bisect_flags);
 	if (best) {
-		if (!find_all) {
+		if (!(bisect_flags & BISECT_FIND_ALL)) {
 			list->item = best->item;
 			free_commit_list(list->next);
 			best = list;
@@ -943,6 +944,7 @@ int bisect_next_all(const char *prefix, int no_checkout)
 	struct rev_info revs;
 	struct commit_list *tried;
 	int reaches = 0, all = 0, nr, steps;
+	unsigned bisect_flags = 0;
 	struct object_id *bisect_rev;
 	char *steps_msg;
 
@@ -957,7 +959,10 @@ int bisect_next_all(const char *prefix, int no_checkout)
 
 	bisect_common(&revs);
 
-	find_bisection(&revs.commits, &reaches, &all, !!skipped_revs.nr);
+	if (skipped_revs.nr)
+		bisect_flags |= BISECT_FIND_ALL;
+
+	find_bisection(&revs.commits, &reaches, &all, bisect_flags);
 	revs.commits = managed_skipped(revs.commits, &tried);
 
 	if (!revs.commits) {
diff --git a/bisect.h b/bisect.h
index a5d9248a47..1d40a33ad2 100644
--- a/bisect.h
+++ b/bisect.h
@@ -1,15 +1,17 @@
 #ifndef BISECT_H
 #define BISECT_H
 
+#define BISECT_FIND_ALL		(1u<<0)
+
 /*
  * Find bisection. If something is found, `reaches` will be the number of
  * commits that the best commit reaches. `all` will be the count of
  * non-SAMETREE commits. If nothing is found, `list` will be NULL.
  * Otherwise, it will be either all non-SAMETREE commits or the single
- * best commit, as chosen by `find_all`.
+ * best commit, as chosen by flag `BISECT_FIND_ALL`.
  */
 extern void find_bisection(struct commit_list **list, int *reaches, int *all,
-			   int find_all);
+			   unsigned bisect_flags);
 
 extern struct commit_list *filter_skipped(struct commit_list *list,
 					  struct commit_list **tried,
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index fadd3ec14c..8752f5bbed 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -360,8 +360,8 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	int i;
 	int bisect_list = 0;
 	int bisect_show_vars = 0;
-	int bisect_find_all = 0;
 	int use_bitmap_index = 0;
+	unsigned bisect_flags = 0;
 	const char *show_progress = NULL;
 
 	if (argc == 2 && !strcmp(argv[1], "-h"))
@@ -426,7 +426,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 		}
 		if (!strcmp(arg, "--bisect-all")) {
 			bisect_list = 1;
-			bisect_find_all = 1;
+			bisect_flags |= BISECT_FIND_ALL;
 			info.flags |= BISECT_SHOW_ALL;
 			revs.show_decorations = 1;
 			continue;
@@ -538,7 +538,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	if (bisect_list) {
 		int reaches, all;
 
-		find_bisection(&revs.commits, &reaches, &all, bisect_find_all);
+		find_bisection(&revs.commits, &reaches, &all, bisect_flags);
 
 		if (bisect_show_vars)
 			return show_bisect_vars(&info, reaches, all);
-- 
2.14.3 (Apple Git-98)


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4] bisect: create 'bisect_flags' parameter in find_bisection()
  2018-04-18 21:05 ` [PATCH v4] " Harald Nordgren
@ 2018-04-20 12:23   ` Johannes Schindelin
  0 siblings, 0 replies; 9+ messages in thread
From: Johannes Schindelin @ 2018-04-20 12:23 UTC (permalink / raw)
  To: Harald Nordgren; +Cc: git, tiago, christian.couder, sbeller

Hi Harald,

On Wed, 18 Apr 2018, Harald Nordgren wrote:

> Make it possible to implement bisecting only on first parents or on
> merge commits by passing flags to find_bisection(), instead of just
> a 'find_all' boolean.
> 
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
> 
> Notes:
>     Use unsigned type and cache flag value

This version looks good to me.

Thanks,
Johannes

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-15  8:58 [PATCH] Create 'bisect_flags' parameter in find_bisection() in preparation of passing multiple options Harald Nordgren
2018-04-15  9:55 ` Christian Couder
2018-04-15 10:44 ` [PATCH v2] Make it possible to implement bisecting only on first parents or on merge commits by passing flags to find_bisection(), instead of just a find_all boolean Harald Nordgren
2018-04-15 13:35   ` Christian Couder
2018-04-15 13:49 ` [PATCH v3] bisect: create 'bisect_flags' parameter in find_bisection() Harald Nordgren
2018-04-17  2:15   ` Junio C Hamano
2018-04-18 20:31   ` Johannes Schindelin
2018-04-18 21:05 ` [PATCH v4] " Harald Nordgren
2018-04-20 12:23   ` Johannes Schindelin

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox