git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Subject: [PATCH v2 10/13] rerere: explain the primary codepath
Date: Tue, 30 Jun 2015 23:04:56 -0700	[thread overview]
Message-ID: <1435730699-9124-11-git-send-email-gitster@pobox.com> (raw)
In-Reply-To: <1435730699-9124-1-git-send-email-gitster@pobox.com>

Explain the internals of rerere as in-code comments, while
sprinkling "NEEDSWORK" comment to highlight iffy bits and
questionable assumptions.

This one covers the codepath reached from rerere(), the primary
interface to the subsystem.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 rerere.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 82 insertions(+), 13 deletions(-)

diff --git a/rerere.c b/rerere.c
index d54bdb2..3d9c33b 100644
--- a/rerere.c
+++ b/rerere.c
@@ -199,6 +199,21 @@ static int is_cmarker(char *buf, int marker_char, int marker_size)
 	return isspace(*buf);
 }
 
+/*
+ * Read contents a file with conflicts, normalize the conflicts
+ * by (1) discarding the common ancestor version in diff3-style,
+ * (2) reordering our side and their side so that whichever sorts
+ * alphabetically earlier comes before the other one, while
+ * computing the "conflict ID", which is just an SHA-1 hash of
+ * one side of the conflict, NUL, the other side of the conflict,
+ * and NUL concatenated together.
+ *
+ * Return the number of conflict hunks found.
+ *
+ * NEEDSWORK: the logic and theory of operation behind this conflict
+ * normalization may deserve to be documented somewhere, perhaps in
+ * Documentation/technical/rerere.txt.
+ */
 static int handle_path(unsigned char *sha1, struct rerere_io *io, int marker_size)
 {
 	git_SHA_CTX ctx;
@@ -269,6 +284,10 @@ static int handle_path(unsigned char *sha1, struct rerere_io *io, int marker_siz
 	return hunk_no;
 }
 
+/*
+ * Scan the path for conflicts, do the "handle_path()" thing above, and
+ * return the number of conflict hunks found.
+ */
 static int handle_file(const char *path, unsigned char *sha1, const char *output)
 {
 	int hunk_no = 0;
@@ -506,29 +525,54 @@ int rerere_remaining(struct string_list *merge_rr)
 	return 0;
 }
 
+/*
+ * Find the conflict identified by "name"; the change between its
+ * "preimage" (i.e. a previous contents with conflict markers) and its
+ * "postimage" (i.e. the corresponding contents with conflicts
+ * resolved) may apply cleanly to the contents stored in "path", i.e.
+ * the conflict this time around.
+ *
+ * Returns 0 for successful replay of recorded resolution, or non-zero
+ * for failure.
+ */
 static int merge(const char *name, const char *path)
 {
 	int ret;
 	mmfile_t cur = {NULL, 0}, base = {NULL, 0}, other = {NULL, 0};
 	mmbuffer_t result = {NULL, 0};
 
+	/*
+	 * Normalize the conflicts in path and write it out to
+	 * "thisimage" temporary file.
+	 */
 	if (handle_file(path, NULL, rerere_path(name, "thisimage")) < 0)
 		return 1;
 
 	if (read_mmfile(&cur, rerere_path(name, "thisimage")) ||
-			read_mmfile(&base, rerere_path(name, "preimage")) ||
-			read_mmfile(&other, rerere_path(name, "postimage"))) {
+	    read_mmfile(&base, rerere_path(name, "preimage")) ||
+	    read_mmfile(&other, rerere_path(name, "postimage"))) {
 		ret = 1;
 		goto out;
 	}
+
+	/*
+	 * A three-way merge. Note that this honors user-customizable
+	 * low-level merge driver settings.
+	 */
 	ret = ll_merge(&result, path, &base, NULL, &cur, "", &other, "", NULL);
 	if (!ret) {
 		FILE *f;
 
+		/*
+		 * A successful replay of recorded resolution.
+		 * Mark that "postimage" was used to help gc.
+		 */
 		if (utime(rerere_path(name, "postimage"), NULL) < 0)
 			warning("failed utime() on %s: %s",
 					rerere_path(name, "postimage"),
 					strerror(errno));
+
+		/* Update "path" with the resolution */
 		f = fopen(path, "w");
 		if (!f)
 			return error("Could not open %s: %s", path,
@@ -581,41 +625,61 @@ static int do_plain_rerere(struct string_list *rr, int fd)
 	find_conflict(&conflict);
 
 	/*
-	 * MERGE_RR records paths with conflicts immediately after merge
-	 * failed.  Some of the conflicted paths might have been hand resolved
-	 * in the working tree since then, but the initial run would catch all
-	 * and register their preimages.
+	 * MERGE_RR records paths with conflicts immediately after
+	 * merge failed.  Some of the conflicted paths might have been
+	 * hand resolved in the working tree since then, but the
+	 * initial run would catch all and register their preimages.
 	 */
-
 	for (i = 0; i < conflict.nr; i++) {
 		const char *path = conflict.items[i].string;
 		if (!string_list_has_string(rr, path)) {
 			unsigned char sha1[20];
 			char *hex;
 			int ret;
+
+			/*
+			 * Ask handle_file() to scan and assign a
+			 * conflict ID.  No need to write anything out
+			 * yet.
+			 */
 			ret = handle_file(path, sha1, NULL);
 			if (ret < 1)
 				continue;
 			hex = xstrdup(sha1_to_hex(sha1));
 			string_list_insert(rr, path)->util = hex;
+
+			/*
+			 * If the directory does not exist, create
+			 * it.  mkdir_in_gitdir() will fail with
+			 * EEXIST if there already is one.
+			 *
+			 * NEEDSWORK: make sure "gc" does not remove
+			 * preimage without removing the directory.
+			 */
 			if (mkdir_in_gitdir(git_path("rr-cache/%s", hex)))
 				continue;
+
+			/*
+			 * We are the first to encounter this
+			 * conflict.  Ask handle_file() to write the
+			 * normalized contents to the "preimage" file.
+			 */
 			handle_file(path, NULL, rerere_path(hex, "preimage"));
 			fprintf(stderr, "Recorded preimage for '%s'\n", path);
 		}
 	}
 
 	/*
-	 * Now some of the paths that had conflicts earlier might have been
-	 * hand resolved.  Others may be similar to a conflict already that
-	 * was resolved before.
+	 * Some of the paths that had conflicts earlier might have
+	 * been resolved by the user.  Others may be similar to a
+	 * conflict already that was resolved before.
 	 */
-
 	for (i = 0; i < rr->nr; i++) {
 		int ret;
 		const char *path = rr->items[i].string;
 		const char *name = (const char *)rr->items[i].util;
 
+		/* Is there a recorded resolution we could attempt to apply? */
 		if (has_rerere_resolution(name)) {
 			if (merge(name, path))
 				continue;
@@ -629,13 +693,13 @@ static int do_plain_rerere(struct string_list *rr, int fd)
 			goto mark_resolved;
 		}
 
-		/* Let's see if we have resolved it. */
+		/* Let's see if the user has resolved it. */
 		ret = handle_file(path, NULL, NULL);
 		if (ret)
 			continue;
 
-		fprintf(stderr, "Recorded resolution for '%s'.\n", path);
 		copy_file(rerere_path(name, "postimage"), path, 0666);
+		fprintf(stderr, "Recorded resolution for '%s'.\n", path);
 	mark_resolved:
 		free(rr->items[i].util);
 		rr->items[i].util = NULL;
@@ -689,6 +753,11 @@ int setup_rerere(struct string_list *merge_rr, int flags)
 	return fd;
 }
 
+/*
+ * The main entry point that is called internally from codepaths that
+ * perform mergy operations, possibly leaving conflicted index entries
+ * and working tree files.
+ */
 int rerere(int flags)
 {
 	struct string_list merge_rr = STRING_LIST_INIT_DUP;
-- 
2.5.0-rc0-209-g5e1f148

  parent reply	other threads:[~2015-07-01  6:06 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-01  6:04 [PATCH v2 00/13] "rerere" minor clean-up Junio C Hamano
2015-07-01  6:04 ` [PATCH v2 01/13] rerere: fix an off-by-one non-bug Junio C Hamano
2015-07-01  6:04 ` [PATCH v2 02/13] rerere: plug conflict ID leaks Junio C Hamano
2015-07-01  6:04 ` [PATCH v2 03/13] rerere: lift PATH_MAX limitation Junio C Hamano
2015-07-01  6:04 ` [PATCH v2 04/13] rerere: write out each record of MERGE_RR in one go Junio C Hamano
2015-07-01  6:04 ` [PATCH v2 05/13] rerere: report autoupdated paths only after actually updating them Junio C Hamano
2015-07-01  6:04 ` [PATCH v2 06/13] rerere: drop want_sp parameter from is_cmarker() Junio C Hamano
2015-07-01  6:04 ` [PATCH v2 07/13] rerere: stop looping unnecessarily Junio C Hamano
2015-07-01  6:04 ` [PATCH v2 08/13] rerere: explain the rerere I/O abstraction Junio C Hamano
2015-07-01  6:04 ` [PATCH v2 09/13] rerere: explain MERGE_RR management helpers Junio C Hamano
2015-07-01  6:04 ` Junio C Hamano [this message]
2015-07-01  6:04 ` [PATCH v2 11/13] rerere: explain "rerere forget" codepath Junio C Hamano
2015-07-01  6:04 ` [PATCH v2 12/13] rerere: explain the remainder Junio C Hamano
2015-07-01  6:04 ` [PATCH v2 13/13] rerere: refactor "replay" part of do_plain_rerere() Junio C Hamano
2015-07-17 22:24 ` [PATCH v3 00/18] "rerere" preparatory clean-up Junio C Hamano
2015-07-17 22:24   ` [PATCH v3 01/18] rerere: fix an off-by-one non-bug Junio C Hamano
2015-07-24 19:46     ` Junio C Hamano
2015-07-17 22:24   ` [PATCH v3 02/18] rerere: plug conflict ID leaks Junio C Hamano
2015-07-17 22:24   ` [PATCH v3 03/18] rerere: lift PATH_MAX limitation Junio C Hamano
2015-07-17 22:24   ` [PATCH v3 04/18] rerere: write out each record of MERGE_RR in one go Junio C Hamano
2015-07-17 22:24   ` [PATCH v3 05/18] rerere: report autoupdated paths only after actually updating them Junio C Hamano
2015-07-17 22:24   ` [PATCH v3 06/18] rerere: drop want_sp parameter from is_cmarker() Junio C Hamano
2015-07-18  8:24     ` Philip Oakley
2015-07-18  8:47       ` Eric Sunshine
2015-07-17 22:24   ` [PATCH v3 07/18] rerere: stop looping unnecessarily Junio C Hamano
2015-07-24 20:06     ` Junio C Hamano
2015-07-17 22:24   ` [PATCH v3 08/18] rerere: explain the rerere I/O abstraction Junio C Hamano
2015-07-24 20:42     ` Junio C Hamano
2015-07-17 22:24   ` [PATCH v3 09/18] rerere: explain MERGE_RR management helpers Junio C Hamano
2015-07-17 22:24   ` [PATCH v3 10/18] rerere: explain the primary codepath Junio C Hamano
2015-07-17 22:24   ` [PATCH v3 11/18] rerere: explain "rerere forget" codepath Junio C Hamano
2015-07-17 22:24   ` [PATCH v3 12/18] rerere: explain the remainder Junio C Hamano
2015-07-17 22:24   ` [PATCH v3 13/18] rerere: refactor "replay" part of do_plain_rerere() Junio C Hamano
2015-07-17 22:24   ` [PATCH v3 14/18] rerere: further de-dent do_plain_rerere() Junio C Hamano
2015-07-17 22:24   ` [PATCH v3 15/18] rerere: further clarify do_rerere_one_path() Junio C Hamano
2015-07-17 22:24   ` [PATCH v3 16/18] rerere: call conflict-ids IDs Junio C Hamano
2015-07-17 22:24   ` [PATCH v3 17/18] rerere: use "struct rerere_id" instead of "char *" for conflict ID Junio C Hamano
2015-07-18  8:47     ` Eric Sunshine
2015-07-17 22:24   ` [PATCH v3 18/18] rerere: un-nest merge() further Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1435730699-9124-11-git-send-email-gitster@pobox.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).