mirror of
https://github.com/git/git
synced 2024-08-27 11:39:22 +00:00
![Jeff King](/assets/img/avatar_default.png)
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>
31 lines
835 B
C
31 lines
835 B
C
#ifndef RERERE_H
|
|
#define RERERE_H
|
|
|
|
#include "string-list.h"
|
|
|
|
struct pathspec;
|
|
|
|
#define RERERE_AUTOUPDATE 01
|
|
#define RERERE_NOAUTOUPDATE 02
|
|
#define RERERE_READONLY 04
|
|
|
|
/*
|
|
* Marks paths that have been hand-resolved and added to the
|
|
* index. Set in the util field of such paths after calling
|
|
* rerere_remaining.
|
|
*/
|
|
extern void *RERERE_RESOLVED;
|
|
|
|
extern int setup_rerere(struct string_list *, int);
|
|
extern int rerere(int);
|
|
extern const char *rerere_path(const char *hex, const char *file);
|
|
extern int rerere_forget(struct pathspec *);
|
|
extern int rerere_remaining(struct string_list *);
|
|
extern void rerere_clear(struct string_list *);
|
|
extern void rerere_gc(struct string_list *);
|
|
|
|
#define OPT_RERERE_AUTOUPDATE(v) OPT_UYN(0, "rerere-autoupdate", (v), \
|
|
N_("update the index with reused conflict resolution if possible"))
|
|
|
|
#endif
|