Git development
 help / color / mirror / Atom feed
* [PATCH 1/1] Preserve topological ordering after merges This modifies the algorithm of topopological ordering. The problem with the current algoritm is that it displays the graph below as follows
From: Sergey Rudyshin via GitGitGadget @ 2020-01-07 10:30 UTC (permalink / raw)
  To: git; +Cc: Sergey Rudyshin, Junio C Hamano, Sergey Rudyshin
In-Reply-To: <pull.514.git.1578393057.gitgitgadget@gmail.com>

From: Sergey Rudyshin <540555@gmail.com>

* E
|\
| * D
| |\
| |/
|/|
* | C
| * B
|/
* A

commit B is placed between A and C, which is wrong
because E stays that D and B comes after C

the only correct solution here is

* E
|\
| * D
| |\
| |/
|/|
| * B
* | C
|/
* A

while it seems that it contradicts to
D stating that C should be between D and B
The new algorithm solves this issue

Here is an example:
* walk to to the root via "first" parents;
* go E -> C -> A;
* print A because it has no parents;
* step back to C;
* print C because it has no other parents;
* then step back to E;
* go D -> B -> A;
* do not print A because A is already printed;
* step back to B;
* print B;
* so on.

This change makes option "--topo-order" obsolete, because
there is only one way to order parents of a single commit.
"--date-order" and "--author-date-order" are preserved and make sense
only for the case when multiple commits are given
to be able to sort those commits.

Signed-off-by: Sergey Rudyshin <540555@gmail.com>
---
 Documentation/git-rev-list.txt             |   4 +-
 Documentation/rev-list-options.txt         |  41 +++---
 commit.c                                   | 149 ++++++++++++++-------
 commit.h                                   |   4 +-
 t/t4202-log.sh                             |  15 +--
 t/t4215-log-skewed-merges.sh               |  44 +++---
 t/t6003-rev-list-topo-order.sh             |  54 ++++----
 t/t6012-rev-list-simplify.sh               |   8 +-
 t/t6016-rev-list-graph-simplify-history.sh |  41 +++---
 t/t6600-test-reach.sh                      |   6 +-
 10 files changed, 214 insertions(+), 152 deletions(-)

diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index 025c911436..ad1fda7a54 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -3,7 +3,7 @@ git-rev-list(1)
 
 NAME
 ----
-git-rev-list - Lists commit objects in reverse chronological order
+git-rev-list - Lists commit objects in reverse topological order
 
 
 SYNOPSIS
@@ -17,7 +17,7 @@ DESCRIPTION
 List commits that are reachable by following the `parent` links from the
 given commit(s), but exclude commits that are reachable from the one(s)
 given with a '{caret}' in front of them.  The output is given in reverse
-chronological order by default.
+topological order by default.
 
 You can think of this as a set operation.  Commits given on the command
 line form a set of commits that are reachable from any of them, and then
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index bfd02ade99..7ab2f451ae 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -643,39 +643,42 @@ ifndef::git-shortlog[]
 Commit Ordering
 ~~~~~~~~~~~~~~~
 
-By default, the commits are shown in reverse chronological order.
+By default, the commits are shown in reverse topological order.
++
+Meaning that no parents are shown before all of its children and
+merges do not change the previous order.
++
+E.g., if commit M0 produces topologically ordered list of 
+commits L0 then commit M1 created by merging any commit into M0
+produces list L1 which starts with L0
++
+In case if multiple commits are given via the command line
+the following parameters determine the order among them:
 
 --date-order::
-	Show no parents before all of its children are shown, but
-	otherwise show commits in the commit timestamp order.
+	show commits in the commit timestamp order.
 
 --author-date-order::
-	Show no parents before all of its children are shown, but
-	otherwise show commits in the author timestamp order.
+	show commits in the author timestamp order.
 
 --topo-order::
-	Show no parents before all of its children are shown, and
-	avoid showing commits on multiple lines of history
-	intermixed.
+	show commits in the commit timestamp order.
+	this is deprecated and will be
+	removed in the future.
 +
 For example, in a commit history like this:
 +
 ----------------------------------------------------------------
-
-    ---1----2----4----7
-	\	       \
-	 3----5----6----8---
-
+   1----2-----------5
+    \    \         /
+     -----\---3---4---6
+           \     /
+            ----
 ----------------------------------------------------------------
 +
 where the numbers denote the order of commit timestamps, `git
 rev-list` and friends with `--date-order` show the commits in the
-timestamp order: 8 7 6 5 4 3 2 1.
-+
-With `--topo-order`, they would show 8 6 5 3 7 4 2 1 (or 8 7 4 2 6 5
-3 1); some older commits are shown before newer ones in order to
-avoid showing the commits from two parallel development track mixed
-together.
+following order: 1 2 3 4 5 6.
 
 --reverse::
 	Output the commits chosen to be shown (see Commit Limiting
diff --git a/commit.c b/commit.c
index 434ec030d6..01e9d07db9 100644
--- a/commit.c
+++ b/commit.c
@@ -738,6 +738,12 @@ int compare_commits_by_author_date(const void *a_, const void *b_,
 	return 0;
 }
 
+static int compare_commits_by_author_date_rev(const void *a_, const void *b_,
+				   void *cb_data)
+{
+	return compare_commits_by_author_date(b_, a_, cb_data);
+}
+
 int compare_commits_by_gen_then_commit_date(const void *a_, const void *b_, void *unused)
 {
 	const struct commit *a = a_, *b = b_;
@@ -767,36 +773,83 @@ int compare_commits_by_commit_date(const void *a_, const void *b_, void *unused)
 	return 0;
 }
 
+static int compare_commits_by_commit_date_rev(const void *a_, const void *b_, void *unused)
+{
+	return compare_commits_by_commit_date (b_, a_, unused);
+}
+
+struct maze {
+	struct commit *ret;
+	struct commit *out;
+};
+
+define_commit_slab(maze_slab, struct maze *);
+
+/*
+ * returns the next commit while exploring the maze
+ * current commit has the information:
+ * 	- what was the last commit we went for (OUT)
+ * 	- from which commit we came in (RET)
+ * trying to find a parent next to OUT
+ * if it does not exists returns RET
+ * otherwise returns the found parent
+ */
+static struct commit *get_next(struct commit *commit, struct maze *mz, struct indegree_slab *indegree)
+{
+	struct commit_list *parents = commit->parents;
+	struct commit *next = NULL;
+	int found = 0;
+
+	while (parents) {
+		struct commit *parent = parents->item;
+
+		if (*indegree_slab_at(indegree, parent)) {
+
+			if (!mz->out || found) {
+				next = parent;
+				break;
+			} else if  (mz->out == parent) {
+				found = 1;
+			}
+		}
+		parents = parents->next;
+	}
+
+	if (!next) next = mz->ret;
+
+	return next;
+}
+
 /*
  * Performs an in-place topological sort on the list supplied.
  */
 void sort_in_topological_order(struct commit_list **list, enum rev_sort_order sort_order)
 {
 	struct commit_list *next, *orig = *list;
-	struct commit_list **pptr;
+	struct commit_list **pptr = list;
 	struct indegree_slab indegree;
-	struct prio_queue queue;
+	struct prio_queue queue_tips;
 	struct commit *commit;
 	struct author_date_slab author_date;
+	struct maze_slab maze;
+	struct maze *mz;
 
 	if (!orig)
 		return;
 	*list = NULL;
 
 	init_indegree_slab(&indegree);
-	memset(&queue, '\0', sizeof(queue));
+	init_maze_slab(&maze);
+	memset(&queue_tips, '\0', sizeof(queue_tips));
 
 	switch (sort_order) {
-	default: /* REV_SORT_IN_GRAPH_ORDER */
-		queue.compare = NULL;
-		break;
-	case REV_SORT_BY_COMMIT_DATE:
-		queue.compare = compare_commits_by_commit_date;
+	default: 
+		queue_tips.compare = compare_commits_by_commit_date_rev;
 		break;
 	case REV_SORT_BY_AUTHOR_DATE:
 		init_author_date_slab(&author_date);
-		queue.compare = compare_commits_by_author_date;
-		queue.cb_data = &author_date;
+		queue_tips.compare = compare_commits_by_author_date_rev;
+		queue_tips.cb_data = &author_date;
 		break;
 	}
 
@@ -804,9 +857,10 @@ void sort_in_topological_order(struct commit_list **list, enum rev_sort_order so
 	for (next = orig; next; next = next->next) {
 		struct commit *commit = next->item;
 		*(indegree_slab_at(&indegree, commit)) = 1;
-		/* also record the author dates, if needed */
-		if (sort_order == REV_SORT_BY_AUTHOR_DATE)
-			record_author_date(&author_date, commit);
+		mz = xmalloc(sizeof(struct maze));
+		mz->ret = NULL;
+		mz->out = NULL;
+		*(maze_slab_at(&maze, commit)) = mz;
 	}
 
 	/* update the indegree */
@@ -832,51 +886,56 @@ void sort_in_topological_order(struct commit_list **list, enum rev_sort_order so
 	for (next = orig; next; next = next->next) {
 		struct commit *commit = next->item;
 
-		if (*(indegree_slab_at(&indegree, commit)) == 1)
-			prio_queue_put(&queue, commit);
+		if (*(indegree_slab_at(&indegree, commit)) == 1) {
+			/* also record the author dates, if needed */
+			if (sort_order == REV_SORT_BY_AUTHOR_DATE)
+				record_author_date(&author_date, commit);
+			prio_queue_put(&queue_tips, commit);
+		}
 	}
 
-	/*
-	 * This is unfortunate; the initial tips need to be shown
-	 * in the order given from the revision traversal machinery.
-	 */
-	if (sort_order == REV_SORT_IN_GRAPH_ORDER)
-		prio_queue_reverse(&queue);
-
 	/* We no longer need the commit list */
 	free_commit_list(orig);
 
 	pptr = list;
 	*list = NULL;
-	while ((commit = prio_queue_get(&queue)) != NULL) {
-		struct commit_list *parents;
-
-		for (parents = commit->parents; parents ; parents = parents->next) {
-			struct commit *parent = parents->item;
-			int *pi = indegree_slab_at(&indegree, parent);
 
-			if (!*pi)
+	while ((commit = prio_queue_get(&queue_tips)) != NULL) {
+		struct commit *prev_commit = NULL;
+		while (commit) {
+			mz = *(maze_slab_at(&maze, commit));
+			if (!prev_commit) {
+				/* either a new tip or 
+				 * after entering an already visited commit
+				 */
+			}
+			else if (!mz->out) {
+				/* happens only once for every commit
+				 * note that for tips RET is set to NULL
+				 */
+				mz->ret = prev_commit;
+			} 
+			else if (prev_commit != mz->out) {
+				/* entered an already visited commit
+				 * step back
+				 */
+				commit = prev_commit;
+				prev_commit = NULL;
 				continue;
-
-			/*
-			 * parents are only enqueued for emission
-			 * when all their children have been emitted thereby
-			 * guaranteeing topological order.
-			 */
-			if (--(*pi) == 1)
-				prio_queue_put(&queue, parent);
+			}
+			mz->out = get_next(commit, mz, &indegree);
+			if (mz->out == mz->ret) {
+				/* upon leaving a commit emit it */
+				*pptr = commit_list_insert(commit, pptr);
+			}
+			prev_commit = commit;
+			commit = mz->out;
 		}
-		/*
-		 * all children of commit have already been
-		 * emitted. we can emit it now.
-		 */
-		*(indegree_slab_at(&indegree, commit)) = 0;
-
-		pptr = &commit_list_insert(commit, pptr)->next;
 	}
 
 	clear_indegree_slab(&indegree);
-	clear_prio_queue(&queue);
+	clear_maze_slab(&maze);
+	clear_prio_queue(&queue_tips);
 	if (sort_order == REV_SORT_BY_AUTHOR_DATE)
 		clear_author_date_slab(&author_date);
 }
diff --git a/commit.h b/commit.h
index 221cdaa34b..1fe3cc240e 100644
--- a/commit.h
+++ b/commit.h
@@ -209,7 +209,7 @@ struct commit *pop_commit(struct commit_list **stack);
 void clear_commit_marks(struct commit *commit, unsigned int mark);
 void clear_commit_marks_many(int nr, struct commit **commit, unsigned int mark);
 
-
+/* TODO get rid of rev_sort_order in a favour of positional parameters */
 enum rev_sort_order {
 	REV_SORT_IN_GRAPH_ORDER = 0,
 	REV_SORT_BY_COMMIT_DATE,
@@ -222,8 +222,6 @@ enum rev_sort_order {
  *   invariant of resulting list is:
  *      a reachable from b => ord(b) < ord(a)
  *   sort_order further specifies:
- *   REV_SORT_IN_GRAPH_ORDER: try to show a commit on a single-parent
- *                            chain together.
  *   REV_SORT_BY_COMMIT_DATE: show eligible commits in committer-date order.
  */
 void sort_in_topological_order(struct commit_list **, enum rev_sort_order);
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 2c9489484a..46f04b36a3 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -635,17 +635,16 @@ test_expect_success 'set up more tangled history' '
 cat > expect <<\EOF
 *   Merge tag 'reach'
 |\
-| \
+| * reach
+| |
 |  \
 *-. \   Merge tags 'octopus-a' and 'octopus-b'
 |\ \ \
-* | | | seventh
 | | * | octopus-b
-| |/ /
-|/| |
-| * | octopus-a
-|/ /
-| * reach
+| | |/
+| * / octopus-a
+| |/
+* / seventh
 |/
 *   Merge branch 'tangle'
 |\
@@ -673,7 +672,7 @@ cat > expect <<\EOF
 * initial
 EOF
 
-test_expect_success 'log --graph with merge' '
+test_expect_success 'log --graph with merge tag reach' '
 	git log --graph --date-order --pretty=tformat:%s |
 		sed "s/ *\$//" >actual &&
 	test_cmp expect actual
diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
index 18709a723e..91630c0cae 100755
--- a/t/t4215-log-skewed-merges.sh
+++ b/t/t4215-log-skewed-merges.sh
@@ -5,7 +5,7 @@ test_description='git log --graph of skewed merges'
 . ./test-lib.sh
 
 check_graph () {
-	cat >expect &&
+	sed "s/ *$//" >expect &&
 	git log --graph --pretty=tformat:%s "$@" >actual.raw &&
 	sed "s/ *$//" actual.raw >actual &&
 	test_cmp expect actual
@@ -185,20 +185,21 @@ test_expect_success 'log --graph with right-skewed merge following a left-skewed
 
 	check_graph --date-order <<-\EOF
 	*   4_H
-	|\
+	|\  
 	| *   4_G
-	| |\
+	| |\  
+	| | * 4_C
 	| * | 4_F
-	|/| |
+	|/| | 
 	| * |   4_E
-	| |\ \
-	| | * | 4_D
-	| |/ /
-	|/| |
-	| | * 4_C
-	| |/
+	| |\ \  
+	| | |/  
+	| |/|   
+	| | * 4_D
+	| |/  
+	|/|   
 	| * 4_B
-	|/
+	|/  
 	* 4_A
 	EOF
 '
@@ -221,21 +222,20 @@ test_expect_success 'log --graph with octopus merge with column joining its penu
 
 	check_graph <<-\EOF
 	*   5_H
-	|\
+	|\  
 	| *-.   5_G
-	| |\ \
+	| |\ \  
 	| | | * 5_F
 	| | * |   5_E
-	| |/|\ \
-	| |_|/ /
-	|/| | /
-	| | |/
-	* | | 5_D
+	| |/|\ \  
+	| |_|/ /  
+	|/| | /   
+	| | |/    
 	| | * 5_C
-	| |/
-	|/|
-	| * 5_B
-	|/
+	| * | 5_B
+	| |/  
+	* / 5_D
+	|/  
 	* 5_A
 	EOF
 '
diff --git a/t/t6003-rev-list-topo-order.sh b/t/t6003-rev-list-topo-order.sh
index 24d1836f41..19cb79bc29 100755
--- a/t/t6003-rev-list-topo-order.sh
+++ b/t/t6003-rev-list-topo-order.sh
@@ -87,12 +87,12 @@ c3
 c2
 c1
 b4
-a3
-a2
-a1
 b3
 b2
 b1
+a3
+a2
+a1
 a0
 l2
 l1
@@ -105,15 +105,15 @@ l5
 l4
 l3
 a4
-b4
-a3
-a2
 c3
 c2
+c1
+b4
 b3
 b2
-c1
 b1
+a3
+a2
 a1
 a0
 l2
@@ -127,12 +127,12 @@ l5
 l4
 l3
 a4
-b4
 c3
 c2
+c1
+b4
 b3
 b2
-c1
 b1
 a3
 a2
@@ -205,12 +205,12 @@ c3
 c2
 c1
 b4
-a3
-a2
-a1
 b3
 b2
 b1
+a3
+a2
+a1
 a0
 l2
 EOF
@@ -224,12 +224,12 @@ c3
 c2
 c1
 b4
-a3
-a2
-a1
 b3
 b2
 b1
+a3
+a2
+a1
 a0
 l2
 EOF
@@ -237,10 +237,10 @@ EOF
 test_output_expect_success 'prune near topo' 'git rev-list --topo-order a4 ^c3' <<EOF
 a4
 b4
+b3
 a3
 a2
 a1
-b3
 EOF
 
 test_output_expect_success "head has no parent" 'git rev-list --topo-order  root' <<EOF
@@ -297,8 +297,8 @@ c3
 c2
 c1
 b4
-a3
-a2
+b3
+b2
 EOF
 
 test_output_expect_success "max-count 10 - non topo order" 'git rev-list --max-count=10 l5' <<EOF
@@ -376,12 +376,12 @@ c3
 c2
 c1
 b4
-a3
-a2
-a1
 b3
 b2
 b1
+a3
+a2
+a1
 a0
 l2
 l1
@@ -399,12 +399,12 @@ c3
 c2
 c1
 b4
-a3
-a2
-a1
 b3
 b2
 b1
+a3
+a2
+a1
 a0
 l2
 l1
@@ -428,12 +428,12 @@ c3
 c2
 c1
 b4
-a3
-a2
-a1
 b3
 b2
 b1
+a3
+a2
+a1
 a0
 l2
 l1
diff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh
index a10f0df02b..9f687b6b22 100755
--- a/t/t6012-rev-list-simplify.sh
+++ b/t/t6012-rev-list-simplify.sh
@@ -122,16 +122,16 @@ check_result () {
 
 check_result 'L K J I H F E D C G B A' --full-history --topo-order
 check_result 'L K I H G F E D C B J A' --full-history
-check_result 'L K I H G F E D C B J A' --full-history --date-order
-check_result 'L K I H G F E D B C J A' --full-history --author-date-order
+check_result 'L K J I H F E D C G B A' --full-history --date-order
+check_result 'L K J I H F E D C G B A' --full-history --author-date-order
 check_result 'K I H E C B A' --full-history -- file
 check_result 'K I H E C B A' --full-history --topo-order -- file
 check_result 'K I H E C B A' --full-history --date-order -- file
-check_result 'K I H E B C A' --full-history --author-date-order -- file
+check_result 'K I H E C B A' --full-history --author-date-order -- file
 check_result 'I E C B A' --simplify-merges -- file
 check_result 'I E C B A' --simplify-merges --topo-order -- file
 check_result 'I E C B A' --simplify-merges --date-order -- file
-check_result 'I E B C A' --simplify-merges --author-date-order -- file
+check_result 'I E C B A' --simplify-merges --author-date-order -- file
 check_result 'I B A' -- file
 check_result 'I B A' --topo-order -- file
 check_result 'I B A' --date-order -- file
diff --git a/t/t6016-rev-list-graph-simplify-history.sh b/t/t6016-rev-list-graph-simplify-history.sh
index f5e6e92f5b..5750c6f0fd 100755
--- a/t/t6016-rev-list-graph-simplify-history.sh
+++ b/t/t6016-rev-list-graph-simplify-history.sh
@@ -235,27 +235,28 @@ test_expect_success '--graph ^C3' '
 # I don't think the ordering of the boundary commits is really
 # that important, but this test depends on it.  If the ordering ever changes
 # in the code, we'll need to update this test.
-test_expect_success '--graph --boundary ^C3' '
+test_expect_success '--graph --boundary --all ^C3' '
 	rm -f expected &&
-	echo "* $A7" >> expected &&
-	echo "*   $A6" >> expected &&
-	echo "|\\  " >> expected &&
-	echo "| * $C4" >> expected &&
-	echo "* | $A5" >> expected &&
-	echo "| |     " >> expected &&
-	echo "|  \\    " >> expected &&
-	echo "*-. \\   $A4" >> expected &&
-	echo "|\\ \\ \\  " >> expected &&
-	echo "| * | | $B2" >> expected &&
-	echo "| * | | $B1" >> expected &&
-	echo "* | | | $A3" >> expected &&
-	echo "o | | | $A2" >> expected &&
-	echo "|/ / /  " >> expected &&
-	echo "o / / $A1" >> expected &&
-	echo " / /  " >> expected &&
-	echo "| o $C3" >> expected &&
-	echo "|/  " >> expected &&
-	echo "o $C2" >> expected &&
+	cat >> expected <<-TESTDATA &&
+	* $A7
+	*   $A6
+	|\  
+	| * $C4
+	* | $A5
+	| |     
+	|  \    
+	*-. \   $A4
+	|\ \ \  
+	| * | | $B2
+	| * | | $B1
+	* | | | $A3
+	| | | o $C3
+	| | |/  
+	| | o $C2
+	o | $A2
+	|/  
+	o $A1
+	TESTDATA
 	git rev-list --graph --boundary --all ^C3 > actual &&
 	test_cmp expected actual
 	'
diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh
index b24d850036..a6d389c4fd 100755
--- a/t/t6600-test-reach.sh
+++ b/t/t6600-test-reach.sh
@@ -339,6 +339,8 @@ test_expect_success 'rev-list: ancestry-path topo-order' '
 	run_three_modes git rev-list --topo-order --ancestry-path commit-3-3..commit-6-6
 '
 
+# TODO get rid of the "sort" below 
+# if commitGraph is enabled then sort_in_topological_order is not envoked
 test_expect_success 'rev-list: symmetric difference topo-order' '
 	git rev-parse \
 		commit-6-6 commit-5-6 commit-4-6 \
@@ -349,8 +351,8 @@ test_expect_success 'rev-list: symmetric difference topo-order' '
 		commit-6-1 commit-5-1 commit-4-1 \
 		commit-3-8 commit-2-8 commit-1-8 \
 		commit-3-7 commit-2-7 commit-1-7 \
-	>expect &&
-	run_three_modes git rev-list --topo-order commit-3-8...commit-6-6
+	| sort >expect &&
+	run_three_modes git rev-list --topo-order commit-3-8...commit-6-6 | sort
 '
 
 test_expect_success 'get_reachable_subset:all' '
-- 
gitgitgadget

^ permalink raw reply related

* [PATCH 0/1] Preserve topological ordering after merges
From: Sergey Rudyshin via GitGitGadget @ 2020-01-07 10:30 UTC (permalink / raw)
  To: git; +Cc: Sergey Rudyshin, Junio C Hamano

This modifies the algoritm of topopological ordering. The problem with the
current algoritm is that it displays the graph below as following

* E
|\
| * D
| |\
| |/
|/|
* | C
| * B
|/
* A

commit B is placed between A and C, which is wrong because E stays that D
and B comes after C

the only correct solution here is

* E
|\
| * D
| |\
| |/
|/|
| * B
* | C
|/
* A

while it seems that it contradicts to D staiting that C shoud be between D
and B The new algorithm solves this issue

Here is an example: walk to to the root via "first" parents go E -> C -> A
print A because it has no parents step back to C print C because it has no
other parents then step back to E go D -> B -> A do not print A becase A is
already printed step back to B print B so on

This change makes option "--topo-order" obsolete, because for a single
commit there is only one way to order parents. "--date-order" and
"--author-date-order" are preserved and make sence only for the case when
multiple commits are given to be able to sort those commits.

Sergey Rudyshin (1):
  Preserve topological ordering after merges This modifies the algorithm
    of topopological ordering. The problem with the current algoritm is
    that it displays the graph below as follows

 Documentation/git-rev-list.txt             |   4 +-
 Documentation/rev-list-options.txt         |  41 +++---
 commit.c                                   | 149 ++++++++++++++-------
 commit.h                                   |   4 +-
 t/t4202-log.sh                             |  15 +--
 t/t4215-log-skewed-merges.sh               |  44 +++---
 t/t6003-rev-list-topo-order.sh             |  54 ++++----
 t/t6012-rev-list-simplify.sh               |   8 +-
 t/t6016-rev-list-graph-simplify-history.sh |  41 +++---
 t/t6600-test-reach.sh                      |   6 +-
 10 files changed, 214 insertions(+), 152 deletions(-)


base-commit: 8679ef24ed64018bb62170c43ce73e0261c0600a
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-514%2FSergeyRudyshin%2Ftopo_order_fix-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-514/SergeyRudyshin/topo_order_fix-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/514
-- 
gitgitgadget

^ permalink raw reply

* [PATCH 1/1] doc: fix a typo in gitcore-tutorial.txt
From: Heba Waly via GitGitGadget @ 2020-01-07 10:05 UTC (permalink / raw)
  To: git; +Cc: Heba Waly, Junio C Hamano, Heba Waly
In-Reply-To: <pull.515.git.1578391553.gitgitgadget@gmail.com>

From: Heba Waly <heba.waly@gmail.com>

Signed-off-by: Heba Waly <heba.waly@gmail.com>
---
 Documentation/gitcore-tutorial.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/gitcore-tutorial.txt b/Documentation/gitcore-tutorial.txt
index f880d21dfb..c0b95256cc 100644
--- a/Documentation/gitcore-tutorial.txt
+++ b/Documentation/gitcore-tutorial.txt
@@ -751,7 +751,7 @@ to it.
 ================================================
 If you make the decision to start your new branch at some
 other point in the history than the current `HEAD`, you can do so by
-just telling 'git checkout' what the base of the checkout would be.
+just telling 'git switch' what the base of the checkout would be.
 In other words, if you have an earlier tag or branch, you'd just do
 
 ------------
-- 
gitgitgadget

^ permalink raw reply related

* [PATCH 0/1] [Outreachy] doc: fix a typo in gitcore-tutorial.txt
From: Heba Waly via GitGitGadget @ 2020-01-07 10:05 UTC (permalink / raw)
  To: git; +Cc: Heba Waly, Junio C Hamano

Fix a typo.

Heba Waly (1):
  doc: fix a typo in gitcore-tutorial.txt

 Documentation/gitcore-tutorial.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: 8679ef24ed64018bb62170c43ce73e0261c0600a
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-515%2FHebaWaly%2Fgitcore_tutorial_typo-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-515/HebaWaly/gitcore_tutorial_typo-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/515
-- 
gitgitgadget

^ permalink raw reply

* [PATCH 1/1] Update imap-send.c, fix incompatibilities with OpenSSL 1.1.x
From: Liam Huang via GitGitGadget @ 2020-01-07 10:02 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Liam Huang
In-Reply-To: <pull.516.git.1578391376.gitgitgadget@gmail.com>

From: Liam Huang <Liam0205@users.noreply.github.com>

Some APIs have been changed since OpenSSL 1.1.0, so fix incompatibilities with OpenSSL 1.1.x.

See:

* <https://www.openssl.org/docs/man1.1.0/man3/SSLv23_method.html>
* <https://wiki.openssl.org/index.php/Library_Initialization>

Signed-off-by: Liam Huang <liamhuang0205@gmail.com>
---
 imap-send.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/imap-send.c b/imap-send.c
index 6c54d8c29d..446fd5532b 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -249,15 +249,28 @@ static int verify_hostname(X509 *cert, const char *hostname)
 	/* try the DNS subjectAltNames */
 	found = 0;
 	if ((subj_alt_names = X509_get_ext_d2i(cert, NID_subject_alt_name, NULL, NULL))) {
+#if (OPENSSL_VERSION_NUMBER >= 0x10100000L)
+		int num_subj_alt_names = OPENSSL_sk_num(subj_alt_names);
+#else
 		int num_subj_alt_names = sk_GENERAL_NAME_num(subj_alt_names);
+#endif
 		for (i = 0; !found && i < num_subj_alt_names; i++) {
+			
+#if (OPENSSL_VERSION_NUMBER >= 0x10100000L)
+			GENERAL_NAME *subj_alt_name = OPENSSL_sk_value(subj_alt_names, i);
+#else
 			GENERAL_NAME *subj_alt_name = sk_GENERAL_NAME_value(subj_alt_names, i);
+#endif
 			if (subj_alt_name->type == GEN_DNS &&
 			    strlen((const char *)subj_alt_name->d.ia5->data) == (size_t)subj_alt_name->d.ia5->length &&
 			    host_matches(hostname, (const char *)(subj_alt_name->d.ia5->data)))
 				found = 1;
 		}
+#if (OPENSSL_VERSION_NUMBER >= 0x10100000L)
+		OPENSSL_sk_pop_free(subj_alt_names, GENERAL_NAME_free);
+#else
 		sk_GENERAL_NAME_pop_free(subj_alt_names, GENERAL_NAME_free);
+#endif
 	}
 	if (found)
 		return 0;
@@ -284,12 +297,22 @@ static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int ve
 	int ret;
 	X509 *cert;
 
+#if (OPENSSL_VERSION_NUMBER >= 0x10100000L)
+        OPENSSL_init_ssl(0, NULL);
+
+	meth = TLS_method();
+#else
 	SSL_library_init();
 	SSL_load_error_strings();
 
 	meth = SSLv23_method();
+#endif
 	if (!meth) {
+#if (OPENSSL_VERSION_NUMBER >= 0x10100000L)
+		ssl_socket_perror("TLS_method");
+#else
 		ssl_socket_perror("SSLv23_method");
+#endif
 		return -1;
 	}
 
-- 
gitgitgadget

^ permalink raw reply related

* [PATCH 0/1] Update imap-send.c, fix incompatibilities with OpenSSL 1.1.x
From: Liam Huang via GitGitGadget @ 2020-01-07 10:02 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

Some APIs have been changed since OpenSSL 1.1.0, so fix incompatibilities
with OpenSSL 1.1.x.

See:

 * https://www.openssl.org/docs/man1.1.0/man3/SSLv23_method.html
 * https://wiki.openssl.org/index.php/Library_Initialization

Liam Huang (1):
  Update imap-send.c, fix incompatibilities with OpenSSL 1.1.x

 imap-send.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)


base-commit: 042ed3e048af08014487d19196984347e3be7d1c
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-516%2FLiam0205%2Fpatch-1-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-516/Liam0205/patch-1-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/516
-- 
gitgitgadget

^ permalink raw reply

* [PATCH v2 1/1] unpack-trees: exit check_updates() early if updates are not wanted
From: Elijah Newren via GitGitGadget @ 2020-01-07  6:57 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Elijah Newren
In-Reply-To: <pull.686.v2.git.git.1578380277.gitgitgadget@gmail.com>

From: Elijah Newren <newren@gmail.com>

check_updates() has a lot of code that repeatedly checks whether
o->update or o->dry_run are set.  (Note that o->dry_run is a
near-synonym for !o->update, but not quite as per commit 2c9078d05bf2
("unpack-trees: add the dry_run flag to unpack_trees_options",
2011-05-25).)  In fact, this function almost turns into a no-op whenever
the condition
   !o->update || o->dry_run
is met.  Simplify the code by checking this condition at the beginning
of the function, and when it is true, do the few things that are
relevant and return early.

There are a few things that make the conversion not quite obvious:
  * The fact that check_updates() does not actually turn into a no-op
    when updates are not wanted may be slightly surprising.  However,
    commit 33ecf7eb61 (Discard "deleted" cache entries after using them
    to update the working tree, 2008-02-07) put the discarding of
    unused cache entries in check_updates() so we still need to keep
    the call to remove_marked_cache_entries().  It's possible this
    call belongs in another function, but it is certainly needed as
    tests will fail if it is removed.
  * The original called remove_scheduled_dirs() unconditionally.
    Technically, commit 7847892716 (unlink_entry(): introduce
    schedule_dir_for_removal(), 2009-02-09) should have made that call
    conditional, but it didn't matter in practice because
    remove_scheduled_dirs() becomes a no-op when all the calls to
    unlink_entry() are skipped.  As such, we do not need to call it.
  * When (o->dry_run && o->update), the original would have two calls
    to git_attr_set_direction() surrounding a bunch of skipped updates.
    These two calls to git_attr_set_direction() cancel each other out
    and thus can be omitted when o->dry_run is true just as they
    already are when !o->update.
  * The code would previously call setup_collided_checkout_detection()
    and report_collided_checkout() even when o->dry_run.  However, this
    was just an expensive no-op because
    setup_collided_checkout_detection() merely cleared the CE_MATCHED
    flag for each cache entry, and report_collided_checkout() reported
    which ones had it set.  Since a dry-run would skip all the
    checkout_entry() calls, CE_MATCHED would never get set and thus
    no collisions would be reported.  Since we can't detect the
    collisions anyway without doing updates, skipping the collisions
    detection setup and reporting is an optimization.
  * The code previously would call get_progress() and
    display_progress() even when (!o->update || o->dry_run).  This
    served to show how long it took to skip all the updates, which is
    somewhat useless.  Since we are skipping the updates, we can skip
    showing how long it takes to skip them.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 unpack-trees.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index 2399b6818b..4c68dbdb43 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -372,15 +372,20 @@ static int check_updates(struct unpack_trees_options *o)
 	state.refresh_cache = 1;
 	state.istate = index;
 
+	if (!o->update || o->dry_run) {
+		remove_marked_cache_entries(index, 0);
+		trace_performance_leave("check_updates");
+		return 0;
+	}
+
 	if (o->clone)
 		setup_collided_checkout_detection(&state, index);
 
 	progress = get_progress(o);
 
-	if (o->update)
-		git_attr_set_direction(GIT_ATTR_CHECKOUT);
+	git_attr_set_direction(GIT_ATTR_CHECKOUT);
 
-	if (should_update_submodules() && o->update && !o->dry_run)
+	if (should_update_submodules())
 		load_gitmodules_file(index, NULL);
 
 	for (i = 0; i < index->cache_nr; i++) {
@@ -388,18 +393,18 @@ static int check_updates(struct unpack_trees_options *o)
 
 		if (ce->ce_flags & CE_WT_REMOVE) {
 			display_progress(progress, ++cnt);
-			if (o->update && !o->dry_run)
-				unlink_entry(ce);
+			unlink_entry(ce);
 		}
 	}
+
 	remove_marked_cache_entries(index, 0);
 	remove_scheduled_dirs();
 
-	if (should_update_submodules() && o->update && !o->dry_run)
+	if (should_update_submodules())
 		load_gitmodules_file(index, &state);
 
 	enable_delayed_checkout(&state);
-	if (has_promisor_remote() && o->update && !o->dry_run) {
+	if (has_promisor_remote()) {
 		/*
 		 * Prefetch the objects that are to be checked out in the loop
 		 * below.
@@ -431,15 +436,12 @@ static int check_updates(struct unpack_trees_options *o)
 				    ce->name);
 			display_progress(progress, ++cnt);
 			ce->ce_flags &= ~CE_UPDATE;
-			if (o->update && !o->dry_run) {
-				errs |= checkout_entry(ce, &state, NULL, NULL);
-			}
+			errs |= checkout_entry(ce, &state, NULL, NULL);
 		}
 	}
 	stop_progress(&progress);
 	errs |= finish_delayed_checkout(&state, NULL);
-	if (o->update)
-		git_attr_set_direction(GIT_ATTR_CHECKIN);
+	git_attr_set_direction(GIT_ATTR_CHECKIN);
 
 	if (o->clone)
 		report_collided_checkout(index);
-- 
gitgitgadget

^ permalink raw reply related

* [PATCH v2 0/1] unpack-trees: exit check_updates() early if updates are not wanted
From: Elijah Newren via GitGitGadget @ 2020-01-07  6:57 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano
In-Reply-To: <pull.686.git.git.1578087730.gitgitgadget@gmail.com>

In trying to understand check_updates(), I found I was able to simplify the
function by making it exit early when updates are not wanted.

Changes since v1:

 * Added two bullet points to the end of the commit message to explain the
   questions Junio brought up about function calls that are skipped by the
   early return.

Elijah Newren (1):
  unpack-trees: exit check_updates() early if updates are not wanted

 unpack-trees.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)


base-commit: 8679ef24ed64018bb62170c43ce73e0261c0600a
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-686%2Fnewren%2Fsimplify-check-updates-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-686/newren/simplify-check-updates-v2
Pull-Request: https://github.com/git/git/pull/686

Range-diff vs v1:

 1:  de0f381284 ! 1:  dd27727332 unpack-trees: exit check_updates() early if updates are not wanted
     @@ -33,6 +33,21 @@
              These two calls to git_attr_set_direction() cancel each other out
              and thus can be omitted when o->dry_run is true just as they
              already are when !o->update.
     +      * The code would previously call setup_collided_checkout_detection()
     +        and report_collided_checkout() even when o->dry_run.  However, this
     +        was just an expensive no-op because
     +        setup_collided_checkout_detection() merely cleared the CE_MATCHED
     +        flag for each cache entry, and report_collided_checkout() reported
     +        which ones had it set.  Since a dry-run would skip all the
     +        checkout_entry() calls, CE_MATCHED would never get set and thus
     +        no collisions would be reported.  Since we can't detect the
     +        collisions anyway without doing updates, skipping the collisions
     +        detection setup and reporting is an optimization.
     +      * The code previously would call get_progress() and
     +        display_progress() even when (!o->update || o->dry_run).  This
     +        served to show how long it took to skip all the updates, which is
     +        somewhat useless.  Since we are skipping the updates, we can skip
     +        showing how long it takes to skip them.
      
          Signed-off-by: Elijah Newren <newren@gmail.com>
      

-- 
gitgitgadget

^ permalink raw reply

* Re: [PATCH] RFC: commit: add a commit.all-ignore-submodules config option
From: Marc-André Lureau @ 2020-01-07  5:15 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git
In-Reply-To: <20200107000551.GE92456@google.com>

Hi

On Tue, Jan 7, 2020 at 4:05 AM Jonathan Nieder <jrnieder@gmail.com> wrote:
>
> Marc-André Lureau wrote:
> > On Sat, Jan 4, 2020 at 4:45 AM Jonathan Nieder <jrnieder@gmail.com> wrote:
> >> Marc-André Lureau wrote:
>
> >>> One of my most frequent mistake is to commit undesired submodules
> >>> changes when doing "commit -a", and I have seen a number of people doing
> >>> the same mistake in various projects. I wish there would be a config to
> >>> change this default behaviour.
> >>
> >> Can you say more about the overall workflow this is part of?  What
> >> causes the submodules to change state in the first place here?
> >
> > The most common case is, I guess, when you work on different branches
> > that have different (compatible) versions of the submodules.
>
> Ah!  This is because "git checkout" defaults to --no-recurse-submodules,
> which is a terrible default.

Thanks for the hint, I'll give it a try for a while and let you know.

> Does "git config submodule.recurse true" help?  If so, we can look
> into which it would take to flip that default.
>
> Thanks,
> Jonathan
>


^ permalink raw reply

* [PATCH v2 11/16] t3415: increase granularity of test_auto_{fixup,squash}()
From: Denton Liu @ 2020-01-07  4:53 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Eric Sunshine, Jakub Narebski, Johannes Sixt, Junio C Hamano
In-Reply-To: <cover.1578372682.git.liu.denton@gmail.com>

We are using `test_must_fail test_auto_{fixup,squash}` which would
ensure that the function failed. However, this is a little ham-fisted as
there is no way of ensuring that the expected part of the function
actually fails.

Increase the granularity by accepting an optional `!` first argument
which will check that the rebase does not squash in any commits by
ensuring that there are still 4 commits. If `!` is not provided, the old
logic is run.

This patch may be better reviewed with `--ignore-all-space`.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3415-rebase-autosquash.sh | 64 +++++++++++++++++++++++++-----------
 1 file changed, 44 insertions(+), 20 deletions(-)

diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index b0add36f82..093de9005b 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -25,6 +25,13 @@ test_expect_success setup '
 '
 
 test_auto_fixup () {
+	no_squash= &&
+	if test "x$1" = 'x!'
+	then
+		no_squash=true
+		shift
+	fi &&
+
 	git reset --hard base &&
 	echo 1 >file1 &&
 	git add -u &&
@@ -35,14 +42,19 @@ test_auto_fixup () {
 	test_tick &&
 	git rebase $2 -i HEAD^^^ &&
 	git log --oneline >actual &&
-	test_line_count = 3 actual &&
-	git diff --exit-code $1 &&
-	echo 1 >expect &&
-	git cat-file blob HEAD^:file1 >actual &&
-	test_cmp expect actual &&
-	git cat-file commit HEAD^ >commit &&
-	grep first commit >actual &&
-	test_line_count = 1 actual
+	if test -n "$no_squash"
+	then
+		test_line_count = 4 actual
+	else
+		test_line_count = 3 actual &&
+		git diff --exit-code $1 &&
+		echo 1 >expect &&
+		git cat-file blob HEAD^:file1 >actual &&
+		test_cmp expect actual &&
+		git cat-file commit HEAD^ >commit &&
+		grep first commit >actual &&
+		test_line_count = 1 actual
+	fi
 }
 
 test_expect_success 'auto fixup (option)' '
@@ -52,12 +64,19 @@ test_expect_success 'auto fixup (option)' '
 test_expect_success 'auto fixup (config)' '
 	git config rebase.autosquash true &&
 	test_auto_fixup final-fixup-config-true &&
-	test_must_fail test_auto_fixup fixup-config-true-no --no-autosquash &&
+	test_auto_fixup ! fixup-config-true-no --no-autosquash &&
 	git config rebase.autosquash false &&
-	test_must_fail test_auto_fixup final-fixup-config-false
+	test_auto_fixup ! final-fixup-config-false
 '
 
 test_auto_squash () {
+	no_squash= &&
+	if test "x$1" = 'x!'
+	then
+		no_squash=true
+		shift
+	fi &&
+
 	git reset --hard base &&
 	echo 1 >file1 &&
 	git add -u &&
@@ -68,14 +87,19 @@ test_auto_squash () {
 	test_tick &&
 	git rebase $2 -i HEAD^^^ &&
 	git log --oneline >actual &&
-	test_line_count = 3 actual &&
-	git diff --exit-code $1 &&
-	echo 1 >expect &&
-	git cat-file blob HEAD^:file1 >actual &&
-	test_cmp expect actual &&
-	git cat-file commit HEAD^ >commit &&
-	grep first commit >actual &&
-	test_line_count = 2 actual
+	if test -n "$no_squash"
+	then
+		test_line_count = 4 actual
+	else
+		test_line_count = 3 actual &&
+		git diff --exit-code $1 &&
+		echo 1 >expect &&
+		git cat-file blob HEAD^:file1 >actual &&
+		test_cmp expect actual &&
+		git cat-file commit HEAD^ >commit &&
+		grep first commit >actual &&
+		test_line_count = 2 actual
+	fi
 }
 
 test_expect_success 'auto squash (option)' '
@@ -85,9 +109,9 @@ test_expect_success 'auto squash (option)' '
 test_expect_success 'auto squash (config)' '
 	git config rebase.autosquash true &&
 	test_auto_squash final-squash-config-true &&
-	test_must_fail test_auto_squash squash-config-true-no --no-autosquash &&
+	test_auto_squash ! squash-config-true-no --no-autosquash &&
 	git config rebase.autosquash false &&
-	test_must_fail test_auto_squash final-squash-config-false
+	test_auto_squash ! final-squash-config-false
 '
 
 test_expect_success 'misspelled auto squash' '
-- 
2.25.0.rc1.180.g49a268d3eb


^ permalink raw reply related

* [PATCH v2 08/16] t3030: use test_path_is_missing()
From: Denton Liu @ 2020-01-07  4:53 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Eric Sunshine, Jakub Narebski, Johannes Sixt, Junio C Hamano
In-Reply-To: <cover.1578372682.git.liu.denton@gmail.com>

We use `test_must_fail test -d` to ensure that the directory is removed.
However, test_must_fail() should only be used for git commands. Use
test_path_is_missing() instead to check that the directory has been
removed.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3030-merge-recursive.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh
index 2170758e38..d48d211a95 100755
--- a/t/t3030-merge-recursive.sh
+++ b/t/t3030-merge-recursive.sh
@@ -604,7 +604,7 @@ test_expect_success 'merge removes empty directories' '
 	git commit -mremoved-d/e &&
 	git checkout master &&
 	git merge -s recursive rm &&
-	test_must_fail test -d d
+	test_path_is_missing d
 '
 
 test_expect_success 'merge-recursive simple w/submodule' '
-- 
2.25.0.rc1.180.g49a268d3eb


^ permalink raw reply related

* [PATCH v2 15/16] t3507: use test_path_is_missing()
From: Denton Liu @ 2020-01-07  4:53 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Eric Sunshine, Jakub Narebski, Johannes Sixt, Junio C Hamano
In-Reply-To: <cover.1578372682.git.liu.denton@gmail.com>

The test_must_fail() function should only be used for git commands since
we should assume that external commands work sanely. Replace
`test_must_fail test_path_exists` with `test_path_is_missing` since we
expect these paths to not exist.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3507-cherry-pick-conflict.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index 2a0d119c8a..9bd482ce3b 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -168,7 +168,7 @@ test_expect_success 'successful final commit clears cherry-pick state' '
 	echo resolved >foo &&
 	test_path_is_file .git/sequencer/todo &&
 	git commit -a &&
-	test_must_fail test_path_exists .git/sequencer
+	test_path_is_missing .git/sequencer
 '
 
 test_expect_success 'reset after final pick clears cherry-pick state' '
@@ -178,7 +178,7 @@ test_expect_success 'reset after final pick clears cherry-pick state' '
 	echo resolved >foo &&
 	test_path_is_file .git/sequencer/todo &&
 	git reset &&
-	test_must_fail test_path_exists .git/sequencer
+	test_path_is_missing .git/sequencer
 '
 
 test_expect_success 'failed cherry-pick produces dirty index' '
@@ -387,7 +387,7 @@ test_expect_success 'successful final commit clears revert state' '
 	echo resolved >foo &&
 	test_path_is_file .git/sequencer/todo &&
 	git commit -a &&
-	test_must_fail test_path_exists .git/sequencer
+	test_path_is_missing .git/sequencer
 '
 
 test_expect_success 'reset after final pick clears revert state' '
@@ -397,7 +397,7 @@ test_expect_success 'reset after final pick clears revert state' '
 	echo resolved >foo &&
 	test_path_is_file .git/sequencer/todo &&
 	git reset &&
-	test_must_fail test_path_exists .git/sequencer
+	test_path_is_missing .git/sequencer
 '
 
 test_expect_success 'revert conflict, diff3 -m style' '
-- 
2.25.0.rc1.180.g49a268d3eb


^ permalink raw reply related

* [PATCH v2 12/16] t3419: stop losing return code of git command
From: Denton Liu @ 2020-01-07  4:53 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Eric Sunshine, Jakub Narebski, Johannes Sixt, Junio C Hamano
In-Reply-To: <cover.1578372682.git.liu.denton@gmail.com>

Fix invocation of git command so its exit codes is not lost within
a non-assignment command substitution.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3419-rebase-patch-id.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/t3419-rebase-patch-id.sh b/t/t3419-rebase-patch-id.sh
index 49f548cdb9..94552669ae 100755
--- a/t/t3419-rebase-patch-id.sh
+++ b/t/t3419-rebase-patch-id.sh
@@ -80,7 +80,8 @@ do_tests () {
 		git commit -q -m "change big file again" &&
 		git checkout -q other^{} &&
 		git rebase master &&
-		test_must_fail test -n "$(git rev-list master...HEAD~)"
+		git rev-list master...HEAD~ >revs &&
+		test_must_be_empty revs
 	'
 
 	test_expect_success $pr 'do not drop patch' '
-- 
2.25.0.rc1.180.g49a268d3eb


^ permalink raw reply related

* [PATCH v2 13/16] t3504: do check for conflict marker after failed cherry-pick
From: Denton Liu @ 2020-01-07  4:53 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Eric Sunshine, Jakub Narebski, Johannes Sixt, Junio C Hamano
In-Reply-To: <cover.1578372682.git.liu.denton@gmail.com>

From: Johannes Sixt <j6t@kdbg.org>

The test with disabled rerere should make sure that the cherry-picked
result does not have the conflict replaced with a recorded resolution.

It attempts to do so by ensuring that the file content is _not_ equal
to some other file. That by itself is a very dubious check because just
about every random result of an incomplete cherry-pick would satisfy
the condition.

In this case, the intent was to check that the conflicting file does
_not_ contain the resolved content. But the check actually uses the
wrong reference file, but not the resolved content. Needless to say
that the non-equality is satisfied. And, on top of it, it uses a commit
that does not even touch the file that is checked.

Do check for the expected result, which is content from both sides of
the merge and merge conflicts. (The latter we check for just the
middle separator for brevity.)

As a side-effect, this also removes an incorrect use of test_must_fail.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3504-cherry-pick-rerere.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/t3504-cherry-pick-rerere.sh b/t/t3504-cherry-pick-rerere.sh
index a267b2d144..80a0d08706 100755
--- a/t/t3504-cherry-pick-rerere.sh
+++ b/t/t3504-cherry-pick-rerere.sh
@@ -94,8 +94,10 @@ test_expect_success 'cherry-pick --rerere-autoupdate more than once' '
 
 test_expect_success 'cherry-pick conflict without rerere' '
 	test_config rerere.enabled false &&
-	test_must_fail git cherry-pick master &&
-	test_must_fail test_cmp expect foo
+	test_must_fail git cherry-pick foo-master &&
+	grep ===== foo &&
+	grep foo-dev foo &&
+	grep foo-master foo
 '
 
 test_done
-- 
2.25.0.rc1.180.g49a268d3eb


^ permalink raw reply related

* [PATCH v2 16/16] t4124: only mark git command with test_must_fail
From: Denton Liu @ 2020-01-07  4:53 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Eric Sunshine, Jakub Narebski, Johannes Sixt, Junio C Hamano
In-Reply-To: <cover.1578372682.git.liu.denton@gmail.com>

The test_must_fail function should only be used for git commands since
we should assume that external commands work sanely. Since apply_patch
wraps a sed and git invocation, rewrite it to accept an `!` argument
which would cause only the git command to be prefixed with
`test_must_fail`.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t4124-apply-ws-rule.sh | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh
index ff51e9e789..971a5a7512 100755
--- a/t/t4124-apply-ws-rule.sh
+++ b/t/t4124-apply-ws-rule.sh
@@ -35,9 +35,15 @@ prepare_test_file () {
 }
 
 apply_patch () {
+	cmd_prefix= &&
+	if test "x$1" = 'x!'
+	then
+		cmd_prefix=test_must_fail &&
+		shift
+	fi &&
 	>target &&
 	sed -e "s|\([ab]\)/file|\1/target|" <patch |
-	git apply "$@"
+	$cmd_prefix git apply "$@"
 }
 
 test_fix () {
@@ -99,7 +105,7 @@ test_expect_success 'whitespace=warn, default rule' '
 
 test_expect_success 'whitespace=error-all, default rule' '
 
-	test_must_fail apply_patch --whitespace=error-all &&
+	apply_patch ! --whitespace=error-all &&
 	test_must_be_empty target
 
 '
-- 
2.25.0.rc1.180.g49a268d3eb


^ permalink raw reply related

* [PATCH v2 14/16] t3507: fix indentation
From: Denton Liu @ 2020-01-07  4:53 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Eric Sunshine, Jakub Narebski, Johannes Sixt, Junio C Hamano
In-Reply-To: <cover.1578372682.git.liu.denton@gmail.com>

We have some test cases which are indented 7-spaces instead of a tab.
Reindent with a tab instead.

This patch should appear empty with `--ignore-all-space`.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3507-cherry-pick-conflict.sh | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index 9b9b4ca8d4..2a0d119c8a 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -381,23 +381,23 @@ test_expect_success 'failed commit does not clear REVERT_HEAD' '
 '
 
 test_expect_success 'successful final commit clears revert state' '
-       pristine_detach picked-signed &&
+	pristine_detach picked-signed &&
 
-       test_must_fail git revert picked-signed base &&
-       echo resolved >foo &&
-       test_path_is_file .git/sequencer/todo &&
-       git commit -a &&
-       test_must_fail test_path_exists .git/sequencer
+	test_must_fail git revert picked-signed base &&
+	echo resolved >foo &&
+	test_path_is_file .git/sequencer/todo &&
+	git commit -a &&
+	test_must_fail test_path_exists .git/sequencer
 '
 
 test_expect_success 'reset after final pick clears revert state' '
-       pristine_detach picked-signed &&
+	pristine_detach picked-signed &&
 
-       test_must_fail git revert picked-signed base &&
-       echo resolved >foo &&
-       test_path_is_file .git/sequencer/todo &&
-       git reset &&
-       test_must_fail test_path_exists .git/sequencer
+	test_must_fail git revert picked-signed base &&
+	echo resolved >foo &&
+	test_path_is_file .git/sequencer/todo &&
+	git reset &&
+	test_must_fail test_path_exists .git/sequencer
 '
 
 test_expect_success 'revert conflict, diff3 -m style' '
-- 
2.25.0.rc1.180.g49a268d3eb


^ permalink raw reply related

* [PATCH v2 10/16] t3415: stop losing return codes of git commands
From: Denton Liu @ 2020-01-07  4:53 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Eric Sunshine, Jakub Narebski, Johannes Sixt, Junio C Hamano
In-Reply-To: <cover.1578372682.git.liu.denton@gmail.com>

Fix invocations of git commands so their exit codes are not lost
within non-assignment command substitutions.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3415-rebase-autosquash.sh | 113 +++++++++++++++++++++++++----------
 1 file changed, 82 insertions(+), 31 deletions(-)

diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
index 22d218698e..b0add36f82 100755
--- a/t/t3415-rebase-autosquash.sh
+++ b/t/t3415-rebase-autosquash.sh
@@ -37,8 +37,12 @@ test_auto_fixup () {
 	git log --oneline >actual &&
 	test_line_count = 3 actual &&
 	git diff --exit-code $1 &&
-	test 1 = "$(git cat-file blob HEAD^:file1)" &&
-	test 1 = $(git cat-file commit HEAD^ | grep first | wc -l)
+	echo 1 >expect &&
+	git cat-file blob HEAD^:file1 >actual &&
+	test_cmp expect actual &&
+	git cat-file commit HEAD^ >commit &&
+	grep first commit >actual &&
+	test_line_count = 1 actual
 }
 
 test_expect_success 'auto fixup (option)' '
@@ -66,8 +70,12 @@ test_auto_squash () {
 	git log --oneline >actual &&
 	test_line_count = 3 actual &&
 	git diff --exit-code $1 &&
-	test 1 = "$(git cat-file blob HEAD^:file1)" &&
-	test 2 = $(git cat-file commit HEAD^ | grep first | wc -l)
+	echo 1 >expect &&
+	git cat-file blob HEAD^:file1 >actual &&
+	test_cmp expect actual &&
+	git cat-file commit HEAD^ >commit &&
+	grep first commit >actual &&
+	test_line_count = 2 actual
 }
 
 test_expect_success 'auto squash (option)' '
@@ -94,7 +102,8 @@ test_expect_success 'misspelled auto squash' '
 	git log --oneline >actual &&
 	test_line_count = 4 actual &&
 	git diff --exit-code final-missquash &&
-	test 0 = $(git rev-list final-missquash...HEAD | wc -l)
+	git rev-list final-missquash...HEAD >list &&
+	test_must_be_empty list
 '
 
 test_expect_success 'auto squash that matches 2 commits' '
@@ -113,9 +122,15 @@ test_expect_success 'auto squash that matches 2 commits' '
 	git log --oneline >actual &&
 	test_line_count = 4 actual &&
 	git diff --exit-code final-multisquash &&
-	test 1 = "$(git cat-file blob HEAD^^:file1)" &&
-	test 2 = $(git cat-file commit HEAD^^ | grep first | wc -l) &&
-	test 1 = $(git cat-file commit HEAD | grep first | wc -l)
+	echo 1 >expect &&
+	git cat-file blob HEAD^^:file1 >actual &&
+	test_cmp expect actual &&
+	git cat-file commit HEAD^^ >commit &&
+	grep first commit >actual &&
+	test_line_count = 2 actual &&
+	git cat-file commit HEAD >commit &&
+	grep first commit >actual &&
+	test_line_count = 1 actual
 '
 
 test_expect_success 'auto squash that matches a commit after the squash' '
@@ -134,25 +149,38 @@ test_expect_success 'auto squash that matches a commit after the squash' '
 	git log --oneline >actual &&
 	test_line_count = 5 actual &&
 	git diff --exit-code final-presquash &&
-	test 0 = "$(git cat-file blob HEAD^^:file1)" &&
-	test 1 = "$(git cat-file blob HEAD^:file1)" &&
-	test 1 = $(git cat-file commit HEAD | grep third | wc -l) &&
-	test 1 = $(git cat-file commit HEAD^ | grep third | wc -l)
+	echo 0 >expect &&
+	git cat-file blob HEAD^^:file1 >actual &&
+	test_cmp expect actual &&
+	echo 1 >expect &&
+	git cat-file blob HEAD^:file1 >actual &&
+	test_cmp expect actual &&
+	git cat-file commit HEAD >commit &&
+	grep third commit >actual &&
+	test_line_count = 1 actual &&
+	git cat-file commit HEAD^ >commit &&
+	grep third commit >actual &&
+	test_line_count = 1 actual
 '
 test_expect_success 'auto squash that matches a sha1' '
 	git reset --hard base &&
 	echo 1 >file1 &&
 	git add -u &&
 	test_tick &&
-	git commit -m "squash! $(git rev-parse --short HEAD^)" &&
+	oid=$(git rev-parse --short HEAD^) &&
+	git commit -m "squash! $oid" &&
 	git tag final-shasquash &&
 	test_tick &&
 	git rebase --autosquash -i HEAD^^^ &&
 	git log --oneline >actual &&
 	test_line_count = 3 actual &&
 	git diff --exit-code final-shasquash &&
-	test 1 = "$(git cat-file blob HEAD^:file1)" &&
-	test 1 = $(git cat-file commit HEAD^ | grep squash | wc -l)
+	echo 1 >expect &&
+	git cat-file blob HEAD^:file1 >actual &&
+	test_cmp expect actual &&
+	git cat-file commit HEAD^ >commit &&
+	grep squash commit >actual &&
+	test_line_count = 1 actual
 '
 
 test_expect_success 'auto squash that matches longer sha1' '
@@ -160,15 +188,20 @@ test_expect_success 'auto squash that matches longer sha1' '
 	echo 1 >file1 &&
 	git add -u &&
 	test_tick &&
-	git commit -m "squash! $(git rev-parse --short=11 HEAD^)" &&
+	oid=$(git rev-parse --short=11 HEAD^) &&
+	git commit -m "squash! $oid" &&
 	git tag final-longshasquash &&
 	test_tick &&
 	git rebase --autosquash -i HEAD^^^ &&
 	git log --oneline >actual &&
 	test_line_count = 3 actual &&
 	git diff --exit-code final-longshasquash &&
-	test 1 = "$(git cat-file blob HEAD^:file1)" &&
-	test 1 = $(git cat-file commit HEAD^ | grep squash | wc -l)
+	echo 1 >expect &&
+	git cat-file blob HEAD^:file1 >actual &&
+	test_cmp expect actual &&
+	git cat-file commit HEAD^ >commit &&
+	grep squash commit >actual &&
+	test_line_count = 1 actual
 '
 
 test_auto_commit_flags () {
@@ -183,8 +216,12 @@ test_auto_commit_flags () {
 	git log --oneline >actual &&
 	test_line_count = 3 actual &&
 	git diff --exit-code final-commit-$1 &&
-	test 1 = "$(git cat-file blob HEAD^:file1)" &&
-	test $2 = $(git cat-file commit HEAD^ | grep first | wc -l)
+	echo 1 >expect &&
+	git cat-file blob HEAD^:file1 >actual &&
+	test_cmp expect actual &&
+	git cat-file commit HEAD^ >commit &&
+	grep first commit >actual &&
+	test_line_count = $2 actual
 }
 
 test_expect_success 'use commit --fixup' '
@@ -210,11 +247,15 @@ test_auto_fixup_fixup () {
 	(
 		set_cat_todo_editor &&
 		test_must_fail git rebase --autosquash -i HEAD^^^^ >actual &&
+		head=$(git rev-parse --short HEAD) &&
+		parent1=$(git rev-parse --short HEAD^) &&
+		parent2=$(git rev-parse --short HEAD^^) &&
+		parent3=$(git rev-parse --short HEAD^^^) &&
 		cat >expected <<-EOF &&
-		pick $(git rev-parse --short HEAD^^^) first commit
-		$1 $(git rev-parse --short HEAD^) $1! first
-		$1 $(git rev-parse --short HEAD) $1! $2! first
-		pick $(git rev-parse --short HEAD^^) second commit
+		pick $parent3 first commit
+		$1 $parent1 $1! first
+		$1 $head $1! $2! first
+		pick $parent2 second commit
 		EOF
 		test_cmp expected actual
 	) &&
@@ -222,13 +263,17 @@ test_auto_fixup_fixup () {
 	git log --oneline >actual &&
 	test_line_count = 3 actual
 	git diff --exit-code "final-$1-$2" &&
-	test 2 = "$(git cat-file blob HEAD^:file1)" &&
+	echo 2 >expect &&
+	git cat-file blob HEAD^:file1 >actual &&
+	test_cmp expect actual &&
+	git cat-file commit HEAD^ >commit &&
+	grep first commit >actual &&
 	if test "$1" = "fixup"
 	then
-		test 1 = $(git cat-file commit HEAD^ | grep first | wc -l)
+		test_line_count = 1 actual
 	elif test "$1" = "squash"
 	then
-		test 3 = $(git cat-file commit HEAD^ | grep first | wc -l)
+		test_line_count = 3 actual
 	else
 		false
 	fi
@@ -256,19 +301,25 @@ test_expect_success C_LOCALE_OUTPUT 'autosquash with custom inst format' '
 	echo 2 >file1 &&
 	git add -u &&
 	test_tick &&
-	git commit -m "squash! $(git rev-parse --short HEAD^)" &&
+	oid=$(git rev-parse --short HEAD^) &&
+	git commit -m "squash! $oid" &&
 	echo 1 >file1 &&
 	git add -u &&
 	test_tick &&
-	git commit -m "squash! $(git log -n 1 --format=%s HEAD~2)" &&
+	subject=$(git log -n 1 --format=%s HEAD~2) &&
+	git commit -m "squash! $subject" &&
 	git tag final-squash-instFmt &&
 	test_tick &&
 	git rebase --autosquash -i HEAD~4 &&
 	git log --oneline >actual &&
 	test_line_count = 3 actual &&
 	git diff --exit-code final-squash-instFmt &&
-	test 1 = "$(git cat-file blob HEAD^:file1)" &&
-	test 2 = $(git cat-file commit HEAD^ | grep squash | wc -l)
+	echo 1 >expect &&
+	git cat-file blob HEAD^:file1 >actual &&
+	test_cmp expect actual &&
+	git cat-file commit HEAD^ >commit &&
+	grep squash commit >actual &&
+	test_line_count = 2 actual
 '
 
 test_expect_success 'autosquash with empty custom instructionFormat' '
-- 
2.25.0.rc1.180.g49a268d3eb


^ permalink raw reply related

* [PATCH v2 09/16] t3310: extract common notes_merge_files_gone()
From: Denton Liu @ 2020-01-07  4:53 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Eric Sunshine, Jakub Narebski, Johannes Sixt, Junio C Hamano
In-Reply-To: <cover.1578372682.git.liu.denton@gmail.com>

We have many statements which are duplicated. Extract and replace these
duplicate statements with notes_merge_files_gone().

While we're at it, replace the test_might_fail(), which should only be
used on git commands.

Also, remove the redirection from stderr to /dev/null. This is because
the test scripts automatically suppress output already. Otherwise, if a
developer asks for verbose output via the `-v` flag, the stderr output
may be useful for debugging.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t3310-notes-merge-manual-resolve.sh | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh
index 2dea846e25..d746f4ba55 100755
--- a/t/t3310-notes-merge-manual-resolve.sh
+++ b/t/t3310-notes-merge-manual-resolve.sh
@@ -32,6 +32,12 @@ verify_notes () {
 	test_cmp "expect_log_$notes_ref" "output_log_$notes_ref"
 }
 
+notes_merge_files_gone () {
+	# No .git/NOTES_MERGE_* files left
+	{ ls .git/NOTES_MERGE_* >output || :; } &&
+	test_must_be_empty output
+}
+
 cat <<EOF | sort >expect_notes_x
 6e8e3febca3c2bb896704335cc4d0c34cb2f8715 $commit_sha4
 e5388c10860456ee60673025345fe2e153eb8cf8 $commit_sha3
@@ -335,9 +341,7 @@ EOF
 y and z notes on 4th commit
 EOF
 	git notes merge --commit &&
-	# No .git/NOTES_MERGE_* files left
-	test_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
-	test_must_be_empty output &&
+	notes_merge_files_gone &&
 	# Merge commit has pre-merge y and pre-merge z as parents
 	test "$(git rev-parse refs/notes/m^1)" = "$(cat pre_merge_y)" &&
 	test "$(git rev-parse refs/notes/m^2)" = "$(cat pre_merge_z)" &&
@@ -397,9 +401,7 @@ test_expect_success 'redo merge of z into m (== y) with default ("manual") resol
 
 test_expect_success 'abort notes merge' '
 	git notes merge --abort &&
-	# No .git/NOTES_MERGE_* files left
-	test_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
-	test_must_be_empty output &&
+	notes_merge_files_gone &&
 	# m has not moved (still == y)
 	test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)" &&
 	# Verify that other notes refs has not changed (w, x, y and z)
@@ -464,9 +466,7 @@ EOF
 	echo "new note on 5th commit" > .git/NOTES_MERGE_WORKTREE/$commit_sha5 &&
 	# Finalize merge
 	git notes merge --commit &&
-	# No .git/NOTES_MERGE_* files left
-	test_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
-	test_must_be_empty output &&
+	notes_merge_files_gone &&
 	# Merge commit has pre-merge y and pre-merge z as parents
 	test "$(git rev-parse refs/notes/m^1)" = "$(cat pre_merge_y)" &&
 	test "$(git rev-parse refs/notes/m^2)" = "$(cat pre_merge_z)" &&
@@ -553,9 +553,7 @@ EOF
 
 test_expect_success 'resolve situation by aborting the notes merge' '
 	git notes merge --abort &&
-	# No .git/NOTES_MERGE_* files left
-	test_might_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
-	test_must_be_empty output &&
+	notes_merge_files_gone &&
 	# m has not moved (still == w)
 	test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)" &&
 	# Verify that other notes refs has not changed (w, x, y and z)
-- 
2.25.0.rc1.180.g49a268d3eb


^ permalink raw reply related

* [PATCH v2 07/16] t2018: replace "sha" with "oid"
From: Denton Liu @ 2020-01-07  4:53 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Eric Sunshine, Jakub Narebski, Johannes Sixt, Junio C Hamano
In-Reply-To: <cover.1578372682.git.liu.denton@gmail.com>

As part of the effort to become more hash-agnostic, replace all
instances of "sha" with "oid".

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t2018-checkout-branch.sh | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index caf43571b1..bbca7ef8da 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -4,12 +4,12 @@ test_description='checkout'
 
 . ./test-lib.sh
 
-# Arguments: [!] <branch> <sha> [<checkout options>]
+# Arguments: [!] <branch> <oid> [<checkout options>]
 #
 # Runs "git checkout" to switch to <branch>, testing that
 #
 #   1) we are on the specified branch, <branch>;
-#   2) HEAD is <sha>; if <sha> is not specified, the old HEAD is used.
+#   2) HEAD is <oid>; if <oid> is not specified, the old HEAD is used.
 #
 # If <checkout options> is not specified, "git checkout" is run with -b.
 #
@@ -25,8 +25,8 @@ do_checkout () {
 	exp_branch=$1 &&
 	exp_ref="refs/heads/$exp_branch" &&
 
-	# if <sha> is not specified, use HEAD.
-	exp_sha=${2:-$(git rev-parse --verify HEAD)} &&
+	# if <oid> is not specified, use HEAD.
+	exp_oid=${2:-$(git rev-parse --verify HEAD)} &&
 
 	# default options for git checkout: -b
 	if test -z "$3"
@@ -38,15 +38,15 @@ do_checkout () {
 
 	if test -n "$should_fail"
 	then
-		test_must_fail git checkout $opts $exp_branch $exp_sha
+		test_must_fail git checkout $opts $exp_branch $exp_oid
 	else
-		git checkout $opts $exp_branch $exp_sha &&
+		git checkout $opts $exp_branch $exp_oid &&
 		echo "$exp_ref" >ref.expect &&
 		git rev-parse --symbolic-full-name HEAD >ref.actual &&
 		test_cmp ref.expect ref.actual &&
-		echo "$exp_sha" >sha.expect &&
-		git rev-parse --verify HEAD >sha.actual &&
-		test_cmp sha.expect sha.actual
+		echo "$exp_oid" >oid.expect &&
+		git rev-parse --verify HEAD >oid.actual &&
+		test_cmp oid.expect oid.actual
 	fi
 }
 
-- 
2.25.0.rc1.180.g49a268d3eb


^ permalink raw reply related

* [PATCH v2 06/16] t2018: don't lose return code of git commands
From: Denton Liu @ 2020-01-07  4:53 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Eric Sunshine, Jakub Narebski, Johannes Sixt, Junio C Hamano
In-Reply-To: <cover.1578372682.git.liu.denton@gmail.com>

Fix invocations of git commands so their exit codes are not lost
within non-assignment command substitutions.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t2018-checkout-branch.sh | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index 687ab6713c..caf43571b1 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -41,8 +41,12 @@ do_checkout () {
 		test_must_fail git checkout $opts $exp_branch $exp_sha
 	else
 		git checkout $opts $exp_branch $exp_sha &&
-		test $exp_ref = $(git rev-parse --symbolic-full-name HEAD) &&
-		test $exp_sha = $(git rev-parse --verify HEAD)
+		echo "$exp_ref" >ref.expect &&
+		git rev-parse --symbolic-full-name HEAD >ref.actual &&
+		test_cmp ref.expect ref.actual &&
+		echo "$exp_sha" >sha.expect &&
+		git rev-parse --verify HEAD >sha.actual &&
+		test_cmp sha.expect sha.actual
 	fi
 }
 
@@ -162,7 +166,8 @@ test_expect_success 'checkout -B to a merge base' '
 '
 
 test_expect_success 'checkout -B to an existing branch from detached HEAD resets branch to HEAD' '
-	git checkout $(git rev-parse --verify HEAD) &&
+	head=$(git rev-parse --verify HEAD) &&
+	git checkout "$head" &&
 
 	do_checkout branch2 "" -B
 '
-- 
2.25.0.rc1.180.g49a268d3eb


^ permalink raw reply related

* [PATCH v2 05/16] t2018: teach do_checkout() to accept `!` arg
From: Denton Liu @ 2020-01-07  4:53 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Eric Sunshine, Jakub Narebski, Johannes Sixt, Junio C Hamano
In-Reply-To: <cover.1578372682.git.liu.denton@gmail.com>

We are running `test_must_fail do_checkout`. However,
`test_must_fail` should only be used on git commands. Teach
do_checkout() to accept `!` as a potential first argument which will
cause the function to expect the "git checkout" to fail.

This increases the granularity of the test as, instead of blindly
checking that do_checkout() failed, we check that only the specific
expected invocation of git fails.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t2018-checkout-branch.sh | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index 7ca55efc6b..687ab6713c 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -4,7 +4,7 @@ test_description='checkout'
 
 . ./test-lib.sh
 
-# Arguments: <branch> <sha> [<checkout options>]
+# Arguments: [!] <branch> <sha> [<checkout options>]
 #
 # Runs "git checkout" to switch to <branch>, testing that
 #
@@ -12,7 +12,16 @@ test_description='checkout'
 #   2) HEAD is <sha>; if <sha> is not specified, the old HEAD is used.
 #
 # If <checkout options> is not specified, "git checkout" is run with -b.
+#
+# If the first argument is `!`, "git checkout" is expected to fail when
+# it is run.
 do_checkout () {
+	should_fail= &&
+	if test "x$1" = "x!"
+	then
+		should_fail=yes &&
+		shift
+	fi &&
 	exp_branch=$1 &&
 	exp_ref="refs/heads/$exp_branch" &&
 
@@ -27,10 +36,14 @@ do_checkout () {
 		opts="$3"
 	fi
 
-	git checkout $opts $exp_branch $exp_sha &&
-
-	test $exp_ref = $(git rev-parse --symbolic-full-name HEAD) &&
-	test $exp_sha = $(git rev-parse --verify HEAD)
+	if test -n "$should_fail"
+	then
+		test_must_fail git checkout $opts $exp_branch $exp_sha
+	else
+		git checkout $opts $exp_branch $exp_sha &&
+		test $exp_ref = $(git rev-parse --symbolic-full-name HEAD) &&
+		test $exp_sha = $(git rev-parse --verify HEAD)
+	fi
 }
 
 test_dirty_unmergeable () {
@@ -91,7 +104,7 @@ test_expect_success 'checkout -b to a new branch, set to an explicit ref' '
 
 test_expect_success 'checkout -b to a new branch with unmergeable changes fails' '
 	setup_dirty_unmergeable &&
-	test_must_fail do_checkout branch2 $HEAD1 &&
+	do_checkout ! branch2 $HEAD1 &&
 	test_dirty_unmergeable
 '
 
@@ -125,7 +138,7 @@ test_expect_success 'checkout -f -b to a new branch with mergeable changes disca
 
 test_expect_success 'checkout -b to an existing branch fails' '
 	test_when_finished git reset --hard HEAD &&
-	test_must_fail do_checkout branch2 $HEAD2
+	do_checkout ! branch2 $HEAD2
 '
 
 test_expect_success 'checkout -b to @{-1} fails with the right branch name' '
@@ -164,7 +177,7 @@ test_expect_success 'checkout -B to an existing branch with unmergeable changes
 	git checkout branch1 &&
 
 	setup_dirty_unmergeable &&
-	test_must_fail do_checkout branch2 $HEAD1 -B &&
+	do_checkout ! branch2 $HEAD1 -B &&
 	test_dirty_unmergeable
 '
 
-- 
2.25.0.rc1.180.g49a268d3eb


^ permalink raw reply related

* [PATCH v2 04/16] t2018: use test_expect_code for failing git commands
From: Denton Liu @ 2020-01-07  4:53 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Eric Sunshine, Jakub Narebski, Johannes Sixt, Junio C Hamano
In-Reply-To: <cover.1578372682.git.liu.denton@gmail.com>

When we are expecting `git diff` to fail, we negate its status with
`!`. However, if git ever exits unexpectedly, say due to a segfault, we
would not be able to tell the difference between that and a controlled
failure. Use `test_expect_code 1 git diff` instead so that if an
unexpected failure occurs, we can catch it.

We have some instances of `test_must_fail test_dirty_{un,}mergable`.
However, `test_must_fail` should only be used on git commands. Convert
these instances to use the `test_dirty_{un,}mergeable_discards_changes`
helper functions so that we remove the double-negation.

While we're at it, remove redirections to /dev/null since output is
silenced when running without `-v` anyway.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t2018-checkout-branch.sh | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index 61206a90fb..7ca55efc6b 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -34,7 +34,11 @@ do_checkout () {
 }
 
 test_dirty_unmergeable () {
-	! git diff --exit-code >/dev/null
+	test_expect_code 1 git diff --exit-code
+}
+
+test_dirty_unmergeable_discards_changes () {
+	git diff --exit-code
 }
 
 setup_dirty_unmergeable () {
@@ -42,7 +46,11 @@ setup_dirty_unmergeable () {
 }
 
 test_dirty_mergeable () {
-	! git diff --cached --exit-code >/dev/null
+	test_expect_code 1 git diff --cached --exit-code
+}
+
+test_dirty_mergeable_discards_changes () {
+	git diff --cached --exit-code
 }
 
 setup_dirty_mergeable () {
@@ -94,7 +102,7 @@ test_expect_success 'checkout -f -b to a new branch with unmergeable changes dis
 
 	# still dirty and on branch1
 	do_checkout branch2 $HEAD1 "-f -b" &&
-	test_must_fail test_dirty_unmergeable
+	test_dirty_unmergeable_discards_changes
 '
 
 test_expect_success 'checkout -b to a new branch preserves mergeable changes' '
@@ -112,7 +120,7 @@ test_expect_success 'checkout -f -b to a new branch with mergeable changes disca
 	test_when_finished git reset --hard HEAD &&
 	setup_dirty_mergeable &&
 	do_checkout branch2 $HEAD1 "-f -b" &&
-	test_must_fail test_dirty_mergeable
+	test_dirty_mergeable_discards_changes
 '
 
 test_expect_success 'checkout -b to an existing branch fails' '
@@ -163,7 +171,7 @@ test_expect_success 'checkout -B to an existing branch with unmergeable changes
 test_expect_success 'checkout -f -B to an existing branch with unmergeable changes discards changes' '
 	# still dirty and on branch1
 	do_checkout branch2 $HEAD1 "-f -B" &&
-	test_must_fail test_dirty_unmergeable
+	test_dirty_unmergeable_discards_changes
 '
 
 test_expect_success 'checkout -B to an existing branch preserves mergeable changes' '
@@ -180,7 +188,7 @@ test_expect_success 'checkout -f -B to an existing branch with mergeable changes
 
 	setup_dirty_mergeable &&
 	do_checkout branch2 $HEAD1 "-f -B" &&
-	test_must_fail test_dirty_mergeable
+	test_dirty_mergeable_discards_changes
 '
 
 test_expect_success 'checkout -b <describe>' '
-- 
2.25.0.rc1.180.g49a268d3eb


^ permalink raw reply related

* [PATCH v2 03/16] t2018: improve style of if-statement
From: Denton Liu @ 2020-01-07  4:53 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Eric Sunshine, Jakub Narebski, Johannes Sixt, Junio C Hamano
In-Reply-To: <cover.1578372682.git.liu.denton@gmail.com>

Convert `[]` to `test` and break if-then into separate lines, both of
which bring the style in line with Git's coding guidelines.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t2018-checkout-branch.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index 79b16e4677..61206a90fb 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -20,7 +20,8 @@ do_checkout () {
 	exp_sha=${2:-$(git rev-parse --verify HEAD)} &&
 
 	# default options for git checkout: -b
-	if [ -z "$3" ]; then
+	if test -z "$3"
+	then
 		opts="-b"
 	else
 		opts="$3"
-- 
2.25.0.rc1.180.g49a268d3eb


^ permalink raw reply related

* [PATCH v2 02/16] t2018: add space between function name and ()
From: Denton Liu @ 2020-01-07  4:52 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Eric Sunshine, Jakub Narebski, Johannes Sixt, Junio C Hamano
In-Reply-To: <cover.1578372682.git.liu.denton@gmail.com>

Add a space between the function name and () which brings the style in
line with Git's coding guidelines.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t2018-checkout-branch.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index e18abce3d2..79b16e4677 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -12,7 +12,7 @@ test_description='checkout'
 #   2) HEAD is <sha>; if <sha> is not specified, the old HEAD is used.
 #
 # If <checkout options> is not specified, "git checkout" is run with -b.
-do_checkout() {
+do_checkout () {
 	exp_branch=$1 &&
 	exp_ref="refs/heads/$exp_branch" &&
 
@@ -32,19 +32,19 @@ do_checkout() {
 	test $exp_sha = $(git rev-parse --verify HEAD)
 }
 
-test_dirty_unmergeable() {
+test_dirty_unmergeable () {
 	! git diff --exit-code >/dev/null
 }
 
-setup_dirty_unmergeable() {
+setup_dirty_unmergeable () {
 	echo >>file1 change2
 }
 
-test_dirty_mergeable() {
+test_dirty_mergeable () {
 	! git diff --cached --exit-code >/dev/null
 }
 
-setup_dirty_mergeable() {
+setup_dirty_mergeable () {
 	echo >file2 file2 &&
 	git add file2
 }
-- 
2.25.0.rc1.180.g49a268d3eb


^ permalink raw reply related

* [PATCH v2 01/16] t2018: remove trailing space from test description
From: Denton Liu @ 2020-01-07  4:52 UTC (permalink / raw)
  To: Git Mailing List
  Cc: Eric Sunshine, Jakub Narebski, Johannes Sixt, Junio C Hamano
In-Reply-To: <cover.1578372682.git.liu.denton@gmail.com>

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 t/t2018-checkout-branch.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index 822381dd9d..e18abce3d2 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='checkout '
+test_description='checkout'
 
 . ./test-lib.sh
 
-- 
2.25.0.rc1.180.g49a268d3eb


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox