* [PATCH 0/4] code clean-up for rerere
@ 2023-08-24 20:54 Junio C Hamano
2023-08-24 20:54 ` [PATCH 1/4] rerere: simplify check_one_conflict() helper function Junio C Hamano
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Junio C Hamano @ 2023-08-24 20:54 UTC (permalink / raw)
To: git
These are to clean up some odd code and in-code comment that invited
"Huh?" while I was reviewing the code paths involved in "git rerere"
recently. None of them is intended to change any behaviour.
Junio C Hamano (4):
rerere: simplify check_one_conflict() helper function
rerere: fix comment on handle_file() helper
rerere: try_merge() should use LL_MERGE_ERROR when it means an error
rerere: modernize use of empty strbuf
rerere.c | 46 +++++++++++++++++++---------------------------
1 file changed, 19 insertions(+), 27 deletions(-)
--
2.42.0-29-gcd9da15a85
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/4] rerere: simplify check_one_conflict() helper function
2023-08-24 20:54 [PATCH 0/4] code clean-up for rerere Junio C Hamano
@ 2023-08-24 20:54 ` Junio C Hamano
2023-08-24 20:54 ` [PATCH 2/4] rerere: fix comment on handle_file() helper Junio C Hamano
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2023-08-24 20:54 UTC (permalink / raw)
To: git
The helper function is responsible for inspecting the index and
deciding if the path is merged, is conflicted in a way that we would
want to handle, or is conflicted in a way that we cannot handle.
Currently, only conflicts with both stage #2 and stage #3 are
handled, but eventually we would want to be able to deal with
delete-modify conflicts (i.e. only one of stages #2 and #3 exist).
Streamline the implementation of the function to make it easier to
extend.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
rerere.c | 34 +++++++++++++++-------------------
1 file changed, 15 insertions(+), 19 deletions(-)
diff --git a/rerere.c b/rerere.c
index b525dd9230..73cdc8392f 100644
--- a/rerere.c
+++ b/rerere.c
@@ -499,9 +499,11 @@ static int handle_file(struct index_state *istate,
}
/*
- * Look at a cache entry at "i" and see if it is not conflicting,
- * conflicting and we are willing to handle, or conflicting and
- * we are unable to handle, and return the determination in *type.
+ * Look at a cache entry at "i" and see if it is not conflicting
+ * (RESOLVED), conflicting and we are willing to handle (THREE_STAGED),
+ * or conflicting and we are unable to handle (PUNTED), and return the
+ * determination in *type.
+ *
* Return the cache index to be looked at next, by skipping the
* stages we have already looked at in this invocation of this
* function.
@@ -509,6 +511,7 @@ static int handle_file(struct index_state *istate,
static int check_one_conflict(struct index_state *istate, int i, int *type)
{
const struct cache_entry *e = istate->cache[i];
+ unsigned int seen_stages = 0;
if (!ce_stage(e)) {
*type = RESOLVED;
@@ -516,24 +519,17 @@ static int check_one_conflict(struct index_state *istate, int i, int *type)
}
*type = PUNTED;
- while (i < istate->cache_nr && ce_stage(istate->cache[i]) == 1)
- i++;
-
- /* Only handle regular files with both stages #2 and #3 */
- if (i + 1 < istate->cache_nr) {
- const struct cache_entry *e2 = istate->cache[i];
- const struct cache_entry *e3 = istate->cache[i + 1];
- if (ce_stage(e2) == 2 &&
- ce_stage(e3) == 3 &&
- ce_same_name(e, e3) &&
- S_ISREG(e2->ce_mode) &&
- S_ISREG(e3->ce_mode))
- *type = THREE_STAGED;
+ for (; i < istate->cache_nr; i++) {
+ const struct cache_entry *n = istate->cache[i];
+ if (!ce_same_name(n, e))
+ break;
+ if (S_ISREG(n->ce_mode))
+ seen_stages |= 1u << (ce_stage(n) - 1);
}
- /* Skip the entries with the same name */
- while (i < istate->cache_nr && ce_same_name(e, istate->cache[i]))
- i++;
+ if ((seen_stages & 6) == 6)
+ *type = THREE_STAGED; /* has both stages #2 and #3 */
+
return i;
}
--
2.42.0-29-gcd9da15a85
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/4] rerere: fix comment on handle_file() helper
2023-08-24 20:54 [PATCH 0/4] code clean-up for rerere Junio C Hamano
2023-08-24 20:54 ` [PATCH 1/4] rerere: simplify check_one_conflict() helper function Junio C Hamano
@ 2023-08-24 20:54 ` Junio C Hamano
2023-08-24 20:54 ` [PATCH 3/4] rerere: try_merge() should use LL_MERGE_ERROR when it means an error Junio C Hamano
2023-08-24 20:54 ` [PATCH 4/4] rerere: modernize use of empty strbuf Junio C Hamano
3 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2023-08-24 20:54 UTC (permalink / raw)
To: git; +Cc: Thomas Gummerer
The return value from handle_path() is returned to the caller of
handle_file() in the normal cases, and it is not the number of
hunks. It is just a normal C Boolean, "do we (!=0) or do we not (0)
have conflict?" plus "a negative return value signals an error".
And all the callers of handle_file() understands its return value as
such. Update the comment to match the reality after 221444f5
(rerere: only return whether a path has conflicts or not,
2018-08-05), which apparently forgot to update this comment when it
turned the returned value of this function from the number of
conflict hunks to a boolean plus error.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
cc: Thomas Gummerer <t.gummerer@gmail.com>
---
rerere.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/rerere.c b/rerere.c
index 73cdc8392f..4ce1270a94 100644
--- a/rerere.c
+++ b/rerere.c
@@ -454,7 +454,7 @@ static int handle_path(unsigned char *hash, struct rerere_io *io, int marker_siz
/*
* Scan the path for conflicts, do the "handle_path()" thing above, and
- * return the number of conflict hunks found.
+ * return true iff conflict hunks were found.
*/
static int handle_file(struct index_state *istate,
const char *path, unsigned char *hash, const char *output)
--
2.42.0-29-gcd9da15a85
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/4] rerere: try_merge() should use LL_MERGE_ERROR when it means an error
2023-08-24 20:54 [PATCH 0/4] code clean-up for rerere Junio C Hamano
2023-08-24 20:54 ` [PATCH 1/4] rerere: simplify check_one_conflict() helper function Junio C Hamano
2023-08-24 20:54 ` [PATCH 2/4] rerere: fix comment on handle_file() helper Junio C Hamano
@ 2023-08-24 20:54 ` Junio C Hamano
2023-08-24 20:54 ` [PATCH 4/4] rerere: modernize use of empty strbuf Junio C Hamano
3 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2023-08-24 20:54 UTC (permalink / raw)
To: git
When the preimage or the postimage files cannot be read, the
try_merge() helper function returns LL_MERGE_CONFLICT. To all of
its callers, this does not make them do wrong things per-se, as they
are only checking if the result is 0 and LL_MERGE_CONFLICT is not 0.
But it is an error if we fail to read the input we expect to be able
to read; return LL_MERGE_ERROR instead. This does not change any
behaviour---it just makes the code use the "correct" constant to
signal an error.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
rerere.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/rerere.c b/rerere.c
index 4ce1270a94..6bc3c54d3b 100644
--- a/rerere.c
+++ b/rerere.c
@@ -617,7 +617,7 @@ static int try_merge(struct index_state *istate,
if (read_mmfile(&base, rerere_path(id, "preimage")) ||
read_mmfile(&other, rerere_path(id, "postimage"))) {
- ret = LL_MERGE_CONFLICT;
+ ret = LL_MERGE_ERROR;
} else {
/*
* A three-way merge. Note that this honors user-customizable
--
2.42.0-29-gcd9da15a85
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 4/4] rerere: modernize use of empty strbuf
2023-08-24 20:54 [PATCH 0/4] code clean-up for rerere Junio C Hamano
` (2 preceding siblings ...)
2023-08-24 20:54 ` [PATCH 3/4] rerere: try_merge() should use LL_MERGE_ERROR when it means an error Junio C Hamano
@ 2023-08-24 20:54 ` Junio C Hamano
3 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2023-08-24 20:54 UTC (permalink / raw)
To: git
Back when the code in the handle_conflict() helper function that
hashes the contents stored in the strbuf "one" and "two", including
its terminating NUL, was written, a freshly initialized strbuf had
NULL in its .buf member, so it was an error to say
update(one.buf, one.len + 1)
which was corrected by b4833a2c (rerere: Fix use of an empty
strbuf.buf, 2007-09-26).
But soon after that, b315c5c0 (strbuf change: be sure ->buf is never
ever NULL., 2007-09-27) introduced strbuf_slopbuf mechanism that
ensures that .buf member of a strbuf is *never* NULL. A freshly
initialized and empty strbuf uses a static piece of memory that has
NUL in it, with its .len member set to 0, so we can always safely
use from offset 0 of .buf[] array for (one.len + 1) bytes.
Simplify the code by essentially reverting the b4833a2c, whose fix
is no longer necessary in the modern world order.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
rerere.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/rerere.c b/rerere.c
index 6bc3c54d3b..69a61aac93 100644
--- a/rerere.c
+++ b/rerere.c
@@ -390,12 +390,8 @@ static int handle_conflict(struct strbuf *out, struct rerere_io *io,
strbuf_addbuf(out, &two);
rerere_strbuf_putconflict(out, '>', marker_size);
if (ctx) {
- the_hash_algo->update_fn(ctx, one.buf ?
- one.buf : "",
- one.len + 1);
- the_hash_algo->update_fn(ctx, two.buf ?
- two.buf : "",
- two.len + 1);
+ the_hash_algo->update_fn(ctx, one.buf, one.len + 1);
+ the_hash_algo->update_fn(ctx, two.buf, two.len + 1);
}
break;
} else if (hunk == RR_SIDE_1)
--
2.42.0-29-gcd9da15a85
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-08-24 20:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-24 20:54 [PATCH 0/4] code clean-up for rerere Junio C Hamano
2023-08-24 20:54 ` [PATCH 1/4] rerere: simplify check_one_conflict() helper function Junio C Hamano
2023-08-24 20:54 ` [PATCH 2/4] rerere: fix comment on handle_file() helper Junio C Hamano
2023-08-24 20:54 ` [PATCH 3/4] rerere: try_merge() should use LL_MERGE_ERROR when it means an error Junio C Hamano
2023-08-24 20:54 ` [PATCH 4/4] rerere: modernize use of empty strbuf Junio C Hamano
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).