mirror of
https://github.com/git/git.git
synced 2024-11-01 14:57:52 +01:00
9dd330e6ca
There's a bug in builtin/am.c in which we take a lock on
MERGE_RR recursively. But rather than fix am.c, this patch
fixes the confusing interface from rerere.c that caused the
bug. Read on for the gory details.
The setup_rerere() function both reads the existing MERGE_RR
file, and takes MERGE_RR.lock. In the rerere() and
rerere_forget() functions, we end up in write_rr(), which
will then commit the lock file.
But for functions like rerere_clear() that do not write to
MERGE_RR, we expect the caller to have handled
setup_rerere(). That caller would then need to release the
lockfile, but it can't; the lock struct is local to
rerere.c.
For builtin/rerere.c, this is OK. We run a single rerere
operation and then exit immediately, which has the side
effect of rolling back the lockfile.
But in builtin/am.c, this is actively wrong. If we run "git
am -3 --skip", we call setup-rerere twice without releasing
the lock:
1. The "--skip" causes us to call am_rerere_clear(), which
calls setup_rerere(), but never drops the lock.
2. We then proceed to the next patch.
3. The "--3way" may cause us to call rerere() to handle
conflicts in that patch, but we are already holding the
lock. The lockfile code dies with:
BUG: prepare_tempfile_object called for active object
We could fix this by having rerere_clear() call
rollback_lock_file(). But it feels a bit odd for it to roll
back a lockfile that it did not itself take. So let's
simplify the interface further, and handle setup_rerere in
the function itself, taking away the question from the
caller over whether they need to do so.
We can give rerere_gc() the same treatment, as well (even
though it doesn't have any callers besides builtin/rerere.c
at this point). Note that these functions don't take flags
from their callers to pass along to setup_rerere; that's OK,
because the flags would not be meaningful for what they are
doing.
Both of those functions need to hold the lock because even
though they do not write to MERGE_RR, they are still writing
and should be protected from a simultaneous "rerere" run.
But rerere_remaining(), "rerere diff", and "rerere status"
are all read-only operations. They want to setup_rerere(),
but do not care about taking the lock in the first place.
Since our update of MERGE_RR is the usual atomic rename done
by commit_lock_file, they can just do a lockless read. For
that, we teach setup_rerere a READONLY flag to avoid the
lock.
As a bonus, this pushes builtin/rerere.c's setup_rerere call
closer to the functions that use it. Which means that "git
rerere totally-bogus-command" will no longer silently
exit(0) in a repository without rerere enabled.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
114 lines
2.9 KiB
C
114 lines
2.9 KiB
C
#include "builtin.h"
|
|
#include "cache.h"
|
|
#include "dir.h"
|
|
#include "parse-options.h"
|
|
#include "string-list.h"
|
|
#include "rerere.h"
|
|
#include "xdiff/xdiff.h"
|
|
#include "xdiff-interface.h"
|
|
#include "pathspec.h"
|
|
|
|
static const char * const rerere_usage[] = {
|
|
N_("git rerere [clear | forget <path>... | status | remaining | diff | gc]"),
|
|
NULL,
|
|
};
|
|
|
|
static int outf(void *dummy, mmbuffer_t *ptr, int nbuf)
|
|
{
|
|
int i;
|
|
for (i = 0; i < nbuf; i++)
|
|
if (write_in_full(1, ptr[i].ptr, ptr[i].size) != ptr[i].size)
|
|
return -1;
|
|
return 0;
|
|
}
|
|
|
|
static int diff_two(const char *file1, const char *label1,
|
|
const char *file2, const char *label2)
|
|
{
|
|
xpparam_t xpp;
|
|
xdemitconf_t xecfg;
|
|
xdemitcb_t ecb;
|
|
mmfile_t minus, plus;
|
|
|
|
if (read_mmfile(&minus, file1) || read_mmfile(&plus, file2))
|
|
return 1;
|
|
|
|
printf("--- a/%s\n+++ b/%s\n", label1, label2);
|
|
fflush(stdout);
|
|
memset(&xpp, 0, sizeof(xpp));
|
|
xpp.flags = 0;
|
|
memset(&xecfg, 0, sizeof(xecfg));
|
|
xecfg.ctxlen = 3;
|
|
ecb.outf = outf;
|
|
xdi_diff(&minus, &plus, &xpp, &xecfg, &ecb);
|
|
|
|
free(minus.ptr);
|
|
free(plus.ptr);
|
|
return 0;
|
|
}
|
|
|
|
int cmd_rerere(int argc, const char **argv, const char *prefix)
|
|
{
|
|
struct string_list merge_rr = STRING_LIST_INIT_DUP;
|
|
int i, autoupdate = -1, flags = 0;
|
|
|
|
struct option options[] = {
|
|
OPT_SET_INT(0, "rerere-autoupdate", &autoupdate,
|
|
N_("register clean resolutions in index"), 1),
|
|
OPT_END(),
|
|
};
|
|
|
|
argc = parse_options(argc, argv, prefix, options, rerere_usage, 0);
|
|
|
|
git_config(git_xmerge_config, NULL);
|
|
|
|
if (autoupdate == 1)
|
|
flags = RERERE_AUTOUPDATE;
|
|
if (autoupdate == 0)
|
|
flags = RERERE_NOAUTOUPDATE;
|
|
|
|
if (argc < 1)
|
|
return rerere(flags);
|
|
|
|
if (!strcmp(argv[0], "forget")) {
|
|
struct pathspec pathspec;
|
|
if (argc < 2)
|
|
warning("'git rerere forget' without paths is deprecated");
|
|
parse_pathspec(&pathspec, 0, PATHSPEC_PREFER_CWD,
|
|
prefix, argv + 1);
|
|
return rerere_forget(&pathspec);
|
|
}
|
|
|
|
if (!strcmp(argv[0], "clear")) {
|
|
rerere_clear(&merge_rr);
|
|
} else if (!strcmp(argv[0], "gc"))
|
|
rerere_gc(&merge_rr);
|
|
else if (!strcmp(argv[0], "status")) {
|
|
if (setup_rerere(&merge_rr, flags | RERERE_READONLY) < 0)
|
|
return 0;
|
|
for (i = 0; i < merge_rr.nr; i++)
|
|
printf("%s\n", merge_rr.items[i].string);
|
|
} else if (!strcmp(argv[0], "remaining")) {
|
|
rerere_remaining(&merge_rr);
|
|
for (i = 0; i < merge_rr.nr; i++) {
|
|
if (merge_rr.items[i].util != RERERE_RESOLVED)
|
|
printf("%s\n", merge_rr.items[i].string);
|
|
else
|
|
/* prepare for later call to
|
|
* string_list_clear() */
|
|
merge_rr.items[i].util = NULL;
|
|
}
|
|
} else if (!strcmp(argv[0], "diff")) {
|
|
if (setup_rerere(&merge_rr, flags | RERERE_READONLY) < 0)
|
|
return 0;
|
|
for (i = 0; i < merge_rr.nr; i++) {
|
|
const char *path = merge_rr.items[i].string;
|
|
const char *name = (const char *)merge_rr.items[i].util;
|
|
diff_two(rerere_path(name, "preimage"), path, path, path);
|
|
}
|
|
} else
|
|
usage_with_options(rerere_usage, options);
|
|
|
|
string_list_clear(&merge_rr, 1);
|
|
return 0;
|
|
}
|