2007-07-15 01:14:45 +02:00
|
|
|
#include "builtin.h"
|
2006-12-20 17:39:41 +01:00
|
|
|
#include "cache.h"
|
2017-06-14 20:07:36 +02:00
|
|
|
#include "config.h"
|
2009-01-10 13:07:50 +01:00
|
|
|
#include "dir.h"
|
2010-08-05 13:28:37 +02:00
|
|
|
#include "parse-options.h"
|
2008-07-21 20:03:49 +02:00
|
|
|
#include "string-list.h"
|
2008-07-09 14:58:57 +02:00
|
|
|
#include "rerere.h"
|
2006-12-20 17:39:41 +01:00
|
|
|
#include "xdiff/xdiff.h"
|
|
|
|
|
#include "xdiff-interface.h"
|
2013-07-14 10:35:40 +02:00
|
|
|
#include "pathspec.h"
|
2006-12-20 17:39:41 +01:00
|
|
|
|
2010-08-05 13:28:37 +02:00
|
|
|
static const char * const rerere_usage[] = {
|
2015-01-13 08:44:47 +01:00
|
|
|
N_("git rerere [clear | forget <path>... | status | remaining | diff | gc]"),
|
2010-08-05 13:28:37 +02:00
|
|
|
NULL,
|
|
|
|
|
};
|
2006-12-20 17:39:41 +01:00
|
|
|
|
|
|
|
|
static int outf(void *dummy, mmbuffer_t *ptr, int nbuf)
|
|
|
|
|
{
|
|
|
|
|
int i;
|
|
|
|
|
for (i = 0; i < nbuf; i++)
|
2007-01-08 16:58:23 +01:00
|
|
|
if (write_in_full(1, ptr[i].ptr, ptr[i].size) != ptr[i].size)
|
|
|
|
|
return -1;
|
2006-12-20 17:39:41 +01:00
|
|
|
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;
|
react to errors in xdi_diff
When we call into xdiff to perform a diff, we generally lose
the return code completely. Typically by ignoring the return
of our xdi_diff wrapper, but sometimes we even propagate
that return value up and then ignore it later. This can
lead to us silently producing incorrect diffs (e.g., "git
log" might produce no output at all, not even a diff header,
for a content-level diff).
In practice this does not happen very often, because the
typical reason for xdiff to report failure is that it
malloc() failed (it uses straight malloc, and not our
xmalloc wrapper). But it could also happen when xdiff
triggers one our callbacks, which returns an error (e.g.,
outf() in builtin/rerere.c tries to report a write failure
in this way). And the next patch also plans to add more
failure modes.
Let's notice an error return from xdiff and react
appropriately. In most of the diff.c code, we can simply
die(), which matches the surrounding code (e.g., that is
what we do if we fail to load a file for diffing in the
first place). This is not that elegant, but we are probably
better off dying to let the user know there was a problem,
rather than simply generating bogus output.
We could also just die() directly in xdi_diff, but the
callers typically have a bit more context, and can provide a
better message (and if we do later decide to pass errors up,
we're one step closer to doing so).
There is one interesting case, which is in diff_grep(). Here
if we cannot generate the diff, there is nothing to match,
and we silently return "no hits". This is actually what the
existing code does already, but we make it a little more
explicit.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-25 01:12:23 +02:00
|
|
|
int ret;
|
2006-12-20 17:39:41 +01:00
|
|
|
|
|
|
|
|
if (read_mmfile(&minus, file1) || read_mmfile(&plus, file2))
|
react to errors in xdi_diff
When we call into xdiff to perform a diff, we generally lose
the return code completely. Typically by ignoring the return
of our xdi_diff wrapper, but sometimes we even propagate
that return value up and then ignore it later. This can
lead to us silently producing incorrect diffs (e.g., "git
log" might produce no output at all, not even a diff header,
for a content-level diff).
In practice this does not happen very often, because the
typical reason for xdiff to report failure is that it
malloc() failed (it uses straight malloc, and not our
xmalloc wrapper). But it could also happen when xdiff
triggers one our callbacks, which returns an error (e.g.,
outf() in builtin/rerere.c tries to report a write failure
in this way). And the next patch also plans to add more
failure modes.
Let's notice an error return from xdiff and react
appropriately. In most of the diff.c code, we can simply
die(), which matches the surrounding code (e.g., that is
what we do if we fail to load a file for diffing in the
first place). This is not that elegant, but we are probably
better off dying to let the user know there was a problem,
rather than simply generating bogus output.
We could also just die() directly in xdi_diff, but the
callers typically have a bit more context, and can provide a
better message (and if we do later decide to pass errors up,
we're one step closer to doing so).
There is one interesting case, which is in diff_grep(). Here
if we cannot generate the diff, there is nothing to match,
and we silently return "no hits". This is actually what the
existing code does already, but we make it a little more
explicit.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-25 01:12:23 +02:00
|
|
|
return -1;
|
2006-12-20 17:39:41 +01:00
|
|
|
|
|
|
|
|
printf("--- a/%s\n+++ b/%s\n", label1, label2);
|
|
|
|
|
fflush(stdout);
|
2008-10-25 15:30:37 +02:00
|
|
|
memset(&xpp, 0, sizeof(xpp));
|
2010-05-02 15:04:41 +02:00
|
|
|
xpp.flags = 0;
|
2007-07-04 20:05:46 +02:00
|
|
|
memset(&xecfg, 0, sizeof(xecfg));
|
2006-12-20 17:39:41 +01:00
|
|
|
xecfg.ctxlen = 3;
|
|
|
|
|
ecb.outf = outf;
|
react to errors in xdi_diff
When we call into xdiff to perform a diff, we generally lose
the return code completely. Typically by ignoring the return
of our xdi_diff wrapper, but sometimes we even propagate
that return value up and then ignore it later. This can
lead to us silently producing incorrect diffs (e.g., "git
log" might produce no output at all, not even a diff header,
for a content-level diff).
In practice this does not happen very often, because the
typical reason for xdiff to report failure is that it
malloc() failed (it uses straight malloc, and not our
xmalloc wrapper). But it could also happen when xdiff
triggers one our callbacks, which returns an error (e.g.,
outf() in builtin/rerere.c tries to report a write failure
in this way). And the next patch also plans to add more
failure modes.
Let's notice an error return from xdiff and react
appropriately. In most of the diff.c code, we can simply
die(), which matches the surrounding code (e.g., that is
what we do if we fail to load a file for diffing in the
first place). This is not that elegant, but we are probably
better off dying to let the user know there was a problem,
rather than simply generating bogus output.
We could also just die() directly in xdi_diff, but the
callers typically have a bit more context, and can provide a
better message (and if we do later decide to pass errors up,
we're one step closer to doing so).
There is one interesting case, which is in diff_grep(). Here
if we cannot generate the diff, there is nothing to match,
and we silently return "no hits". This is actually what the
existing code does already, but we make it a little more
explicit.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-25 01:12:23 +02:00
|
|
|
ret = xdi_diff(&minus, &plus, &xpp, &xecfg, &ecb);
|
2006-12-20 17:39:41 +01:00
|
|
|
|
|
|
|
|
free(minus.ptr);
|
|
|
|
|
free(plus.ptr);
|
react to errors in xdi_diff
When we call into xdiff to perform a diff, we generally lose
the return code completely. Typically by ignoring the return
of our xdi_diff wrapper, but sometimes we even propagate
that return value up and then ignore it later. This can
lead to us silently producing incorrect diffs (e.g., "git
log" might produce no output at all, not even a diff header,
for a content-level diff).
In practice this does not happen very often, because the
typical reason for xdiff to report failure is that it
malloc() failed (it uses straight malloc, and not our
xmalloc wrapper). But it could also happen when xdiff
triggers one our callbacks, which returns an error (e.g.,
outf() in builtin/rerere.c tries to report a write failure
in this way). And the next patch also plans to add more
failure modes.
Let's notice an error return from xdiff and react
appropriately. In most of the diff.c code, we can simply
die(), which matches the surrounding code (e.g., that is
what we do if we fail to load a file for diffing in the
first place). This is not that elegant, but we are probably
better off dying to let the user know there was a problem,
rather than simply generating bogus output.
We could also just die() directly in xdi_diff, but the
callers typically have a bit more context, and can provide a
better message (and if we do later decide to pass errors up,
we're one step closer to doing so).
There is one interesting case, which is in diff_grep(). Here
if we cannot generate the diff, there is nothing to match,
and we silently return "no hits". This is actually what the
existing code does already, but we make it a little more
explicit.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-25 01:12:23 +02:00
|
|
|
return ret;
|
2006-12-20 17:39:41 +01:00
|
|
|
}
|
|
|
|
|
|
2007-09-18 02:06:47 +02:00
|
|
|
int cmd_rerere(int argc, const char **argv, const char *prefix)
|
|
|
|
|
{
|
2010-07-04 21:46:19 +02:00
|
|
|
struct string_list merge_rr = STRING_LIST_INIT_DUP;
|
rerere: release lockfile in non-writing functions
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>
2015-09-02 00:14:09 +02:00
|
|
|
int i, autoupdate = -1, flags = 0;
|
2010-08-05 13:28:37 +02:00
|
|
|
|
|
|
|
|
struct option options[] = {
|
|
|
|
|
OPT_SET_INT(0, "rerere-autoupdate", &autoupdate,
|
2012-08-20 14:32:38 +02:00
|
|
|
N_("register clean resolutions in index"), 1),
|
2010-08-05 13:28:37 +02:00
|
|
|
OPT_END(),
|
|
|
|
|
};
|
|
|
|
|
|
|
|
|
|
argc = parse_options(argc, argv, prefix, options, rerere_usage, 0);
|
|
|
|
|
|
2014-04-30 06:08:29 +02:00
|
|
|
git_config(git_xmerge_config, NULL);
|
|
|
|
|
|
2010-08-05 13:28:37 +02:00
|
|
|
if (autoupdate == 1)
|
|
|
|
|
flags = RERERE_AUTOUPDATE;
|
|
|
|
|
if (autoupdate == 0)
|
|
|
|
|
flags = RERERE_NOAUTOUPDATE;
|
|
|
|
|
|
|
|
|
|
if (argc < 1)
|
2009-12-04 09:20:48 +01:00
|
|
|
return rerere(flags);
|
2009-11-09 16:05:01 +01:00
|
|
|
|
2010-08-05 13:28:37 +02:00
|
|
|
if (!strcmp(argv[0], "forget")) {
|
2013-07-14 10:35:40 +02:00
|
|
|
struct pathspec pathspec;
|
2011-03-01 14:21:05 +01:00
|
|
|
if (argc < 2)
|
|
|
|
|
warning("'git rerere forget' without paths is deprecated");
|
2013-07-14 10:35:40 +02:00
|
|
|
parse_pathspec(&pathspec, 0, PATHSPEC_PREFER_CWD,
|
|
|
|
|
prefix, argv + 1);
|
|
|
|
|
return rerere_forget(&pathspec);
|
2010-01-21 09:23:48 +01:00
|
|
|
}
|
2009-11-09 16:05:01 +01:00
|
|
|
|
2010-08-05 13:28:37 +02:00
|
|
|
if (!strcmp(argv[0], "clear")) {
|
2011-05-08 21:55:34 +02:00
|
|
|
rerere_clear(&merge_rr);
|
2010-08-05 13:28:37 +02:00
|
|
|
} else if (!strcmp(argv[0], "gc"))
|
2011-05-08 21:55:34 +02:00
|
|
|
rerere_gc(&merge_rr);
|
rerere: release lockfile in non-writing functions
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>
2015-09-02 00:14:09 +02:00
|
|
|
else if (!strcmp(argv[0], "status")) {
|
|
|
|
|
if (setup_rerere(&merge_rr, flags | RERERE_READONLY) < 0)
|
|
|
|
|
return 0;
|
2006-12-20 17:39:41 +01:00
|
|
|
for (i = 0; i < merge_rr.nr; i++)
|
2008-07-21 20:03:49 +02:00
|
|
|
printf("%s\n", merge_rr.items[i].string);
|
rerere: release lockfile in non-writing functions
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>
2015-09-02 00:14:09 +02:00
|
|
|
} else if (!strcmp(argv[0], "remaining")) {
|
2011-02-16 11:47:44 +01:00
|
|
|
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;
|
|
|
|
|
}
|
rerere: release lockfile in non-writing functions
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>
2015-09-02 00:14:09 +02:00
|
|
|
} else if (!strcmp(argv[0], "diff")) {
|
|
|
|
|
if (setup_rerere(&merge_rr, flags | RERERE_READONLY) < 0)
|
|
|
|
|
return 0;
|
2006-12-20 17:39:41 +01:00
|
|
|
for (i = 0; i < merge_rr.nr; i++) {
|
2008-07-21 20:03:49 +02:00
|
|
|
const char *path = merge_rr.items[i].string;
|
2015-07-05 02:38:34 +02:00
|
|
|
const struct rerere_id *id = merge_rr.items[i].util;
|
2015-10-05 21:46:27 +02:00
|
|
|
if (diff_two(rerere_path(id, "preimage"), path, path, path))
|
|
|
|
|
die("unable to generate diff for %s", rerere_path(id, NULL));
|
2006-12-20 17:39:41 +01:00
|
|
|
}
|
rerere: release lockfile in non-writing functions
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>
2015-09-02 00:14:09 +02:00
|
|
|
} else
|
2010-08-05 13:28:37 +02:00
|
|
|
usage_with_options(rerere_usage, options);
|
2006-12-20 17:39:41 +01:00
|
|
|
|
2008-07-21 20:03:49 +02:00
|
|
|
string_list_clear(&merge_rr, 1);
|
2006-12-20 17:39:41 +01:00
|
|
|
return 0;
|
|
|
|
|
}
|
