Merge branch 'ma/lockfile-cleanup'

gitster · gitster · commit 2f76ebc93ca6 · 2018-05-30T14:04:05.000+09:00
Code clean-up to adjust to a more recent lockfile API convention that
allows lockfile instances kept on the stack.

* ma/lockfile-cleanup:
  lock_file: move static locks into functions
  lock_file: make function-local locks non-static
  refs.c: do not die if locking fails in `delete_pseudoref()`
  refs.c: do not die if locking fails in `write_pseudoref()`
  t/helper/test-write-cache: clean up lock-handling
diff --git a/apply.c b/apply.c
@@ -4058,7 +4058,7 @@ static int build_fake_ancestor(struct apply_state *state, struct patch *list)
 {
 	struct patch *patch;
 	struct index_state result = { NULL };
-	static struct lock_file lock;
+	struct lock_file lock = LOCK_INIT;
 	int res;
 
 	/* Once we start supporting the reverse patch, it may be
diff --git a/builtin/add.c b/builtin/add.c
@@ -265,8 +265,6 @@ static int edit_patch(int argc, const char **argv, const char *prefix)
 	return 0;
 }
 
-static struct lock_file lock_file;
-
 static const char ignore_error[] =
 N_("The following paths are ignored by one of your .gitignore files:\n");
 
@@ -393,6 +391,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	int add_new_files;
 	int require_pathspec;
 	char *seen = NULL;
+	struct lock_file lock_file = LOCK_INIT;
 
 	git_config(add_config, NULL);
 
diff --git a/builtin/describe.c b/builtin/describe.c
@@ -612,7 +612,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 				suffix = broken;
 			}
 		} else if (dirty) {
-			static struct lock_file index_lock;
+			struct lock_file index_lock = LOCK_INIT;
 			struct rev_info revs;
 			struct argv_array args = ARGV_ARRAY_INIT;
 			int fd, result;
diff --git a/builtin/difftool.c b/builtin/difftool.c
@@ -610,7 +610,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
 			continue;
 
 		if (!indices_loaded) {
-			static struct lock_file lock;
+			struct lock_file lock = LOCK_INIT;
 			strbuf_reset(&buf);
 			strbuf_addf(&buf, "%s/wtindex", tmpdir);
 			if (hold_lock_file_for_update(&lock, buf.buf, 0) < 0 ||
diff --git a/builtin/gc.c b/builtin/gc.c
@@ -373,7 +373,7 @@ static int need_to_gc(void)
 /* return NULL on success, else hostname running the gc */
 static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
 {
-	static struct lock_file lock;
+	struct lock_file lock = LOCK_INIT;
 	char my_host[HOST_NAME_MAX + 1];
 	struct strbuf sb = STRBUF_INIT;
 	struct stat st;
diff --git a/builtin/merge.c b/builtin/merge.c
@@ -647,7 +647,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 			      struct commit_list *remoteheads,
 			      struct commit *head)
 {
-	static struct lock_file lock;
+	struct lock_file lock = LOCK_INIT;
 	const char *head_arg = "HEAD";
 
 	hold_locked_index(&lock, LOCK_DIE_ON_ERROR);
@@ -805,7 +805,7 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
 {
 	struct object_id result_tree, result_commit;
 	struct commit_list *parents, **pptr = &parents;
-	static struct lock_file lock;
+	struct lock_file lock = LOCK_INIT;
 
 	hold_locked_index(&lock, LOCK_DIE_ON_ERROR);
 	refresh_cache(REFRESH_QUIET);
diff --git a/builtin/mv.c b/builtin/mv.c
@@ -72,7 +72,6 @@ static const char *add_slash(const char *path)
 	return path;
 }
 
-static struct lock_file lock_file;
 #define SUBMODULE_WITH_GITDIR ((const char *)1)
 
 static void prepare_move_submodule(const char *src, int first,
@@ -131,6 +130,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 	enum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;
 	struct stat st;
 	struct string_list src_for_dst = STRING_LIST_INIT_NODUP;
+	struct lock_file lock_file = LOCK_INIT;
 
 	git_config(git_default_config, NULL);
 
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
@@ -107,15 +107,14 @@ static int git_read_tree_config(const char *var, const char *value, void *cb)
 	return git_default_config(var, value, cb);
 }
 
-static struct lock_file lock_file;
-
 int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 {
 	int i, stage = 0;
 	struct object_id oid;
 	struct tree_desc t[MAX_UNPACK_TREES];
 	struct unpack_trees_options opts;
 	int prefix_set = 0;
+	struct lock_file lock_file = LOCK_INIT;
 	const struct option read_tree_options[] = {
 		{ OPTION_CALLBACK, 0, "index-output", NULL, N_("file"),
 		  N_("write resulting index to <file>"),
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
@@ -876,7 +876,7 @@ static void refuse_unconfigured_deny_delete_current(void)
 static int command_singleton_iterator(void *cb_data, struct object_id *oid);
 static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
 {
-	static struct lock_file shallow_lock;
+	struct lock_file shallow_lock = LOCK_INIT;
 	struct oid_array extra = OID_ARRAY_INIT;
 	struct check_connected_options opt = CHECK_CONNECTED_INIT;
 	uint32_t mask = 1 << (cmd->index % 32);
diff --git a/builtin/rm.c b/builtin/rm.c
@@ -233,8 +233,6 @@ static int check_local_mod(struct object_id *head, int index_only)
 	return errs;
 }
 
-static struct lock_file lock_file;
-
 static int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;
 static int ignore_unmatch = 0;
 
@@ -251,6 +249,7 @@ static struct option builtin_rm_options[] = {
 
 int cmd_rm(int argc, const char **argv, const char *prefix)
 {
+	struct lock_file lock_file = LOCK_INIT;
 	int i;
 	struct pathspec pathspec;
 	char *seen;
diff --git a/bundle.c b/bundle.c
@@ -409,7 +409,7 @@ static int write_bundle_refs(int bundle_fd, struct rev_info *revs)
 int create_bundle(struct bundle_header *header, const char *path,
 		  int argc, const char **argv)
 {
-	static struct lock_file lock;
+	struct lock_file lock = LOCK_INIT;
 	int bundle_fd = -1;
 	int bundle_to_stdout;
 	int ref_count = 0;
diff --git a/fast-import.c b/fast-import.c
@@ -1817,7 +1817,7 @@ static void dump_marks_helper(FILE *f,
 
 static void dump_marks(void)
 {
-	static struct lock_file mark_lock;
+	struct lock_file mark_lock = LOCK_INIT;
 	FILE *f;
 
 	if (!export_marks_file || (import_marks_file && !import_marks_file_done))
diff --git a/refs.c b/refs.c
@@ -660,7 +660,7 @@ static int write_pseudoref(const char *pseudoref, const struct object_id *oid,
 {
 	const char *filename;
 	int fd;
-	static struct lock_file lock;
+	struct lock_file lock = LOCK_INIT;
 	struct strbuf buf = STRBUF_INIT;
 	int ret = -1;
 
@@ -670,8 +670,7 @@ static int write_pseudoref(const char *pseudoref, const struct object_id *oid,
 	strbuf_addf(&buf, "%s\n", oid_to_hex(oid));
 
 	filename = git_path("%s", pseudoref);
-	fd = hold_lock_file_for_update_timeout(&lock, filename,
-					       LOCK_DIE_ON_ERROR,
+	fd = hold_lock_file_for_update_timeout(&lock, filename, 0,
 					       get_files_ref_lock_timeout_ms());
 	if (fd < 0) {
 		strbuf_addf(err, "could not open '%s' for writing: %s",
@@ -706,20 +705,23 @@ static int write_pseudoref(const char *pseudoref, const struct object_id *oid,
 
 static int delete_pseudoref(const char *pseudoref, const struct object_id *old_oid)
 {
-	static struct lock_file lock;
 	const char *filename;
 
 	filename = git_path("%s", pseudoref);
 
 	if (old_oid && !is_null_oid(old_oid)) {
+		struct lock_file lock = LOCK_INIT;
 		int fd;
 		struct object_id actual_old_oid;
 
 		fd = hold_lock_file_for_update_timeout(
-				&lock, filename, LOCK_DIE_ON_ERROR,
+				&lock, filename, 0,
 				get_files_ref_lock_timeout_ms());
-		if (fd < 0)
-			die_errno(_("Could not open '%s' for writing"), filename);
+		if (fd < 0) {
+			error_errno(_("could not open '%s' for writing"),
+				    filename);
+			return -1;
+		}
 		if (read_ref(pseudoref, &actual_old_oid))
 			die("could not read ref '%s'", pseudoref);
 		if (oidcmp(&actual_old_oid, old_oid)) {
diff --git a/refs/files-backend.c b/refs/files-backend.c
@@ -2991,7 +2991,7 @@ static int files_reflog_expire(struct ref_store *ref_store,
 {
 	struct files_ref_store *refs =
 		files_downcast(ref_store, REF_STORE_WRITE, "reflog_expire");
-	static struct lock_file reflog_lock;
+	struct lock_file reflog_lock = LOCK_INIT;
 	struct expire_reflog_cb cb;
 	struct ref_lock *lock;
 	struct strbuf log_file_sb = STRBUF_INIT;
diff --git a/rerere.c b/rerere.c
@@ -703,10 +703,9 @@ static int merge(const struct rerere_id *id, const char *path)
 	return ret;
 }
 
-static struct lock_file index_lock;
-
 static void update_paths(struct string_list *update)
 {
+	struct lock_file index_lock = LOCK_INIT;
 	int i;
 
 	hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);
diff --git a/shallow.c b/shallow.c
@@ -353,7 +353,7 @@ void advertise_shallow_grafts(int fd)
  */
 void prune_shallow(int show_only)
 {
-	static struct lock_file shallow_lock;
+	struct lock_file shallow_lock = LOCK_INIT;
 	struct strbuf sb = STRBUF_INIT;
 	int fd;
 
diff --git a/t/helper/test-scrap-cache-tree.c b/t/helper/test-scrap-cache-tree.c
@@ -4,10 +4,10 @@
 #include "tree.h"
 #include "cache-tree.h"
 
-static struct lock_file index_lock;
-
 int cmd__scrap_cache_tree(int ac, const char **av)
 {
+	struct lock_file index_lock = LOCK_INIT;
+
 	setup_git_directory();
 	hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);
 	if (read_cache() < 0)
diff --git a/t/helper/test-write-cache.c b/t/helper/test-write-cache.c
@@ -2,22 +2,18 @@
 #include "cache.h"
 #include "lockfile.h"
 
-static struct lock_file index_lock;
-
 int cmd__write_cache(int argc, const char **argv)
 {
-	int i, cnt = 1, lockfd;
+	struct lock_file index_lock = LOCK_INIT;
+	int i, cnt = 1;
 	if (argc == 2)
 		cnt = strtol(argv[1], NULL, 0);
 	setup_git_directory();
 	read_cache();
 	for (i = 0; i < cnt; i++) {
-		lockfd = hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);
-		if (0 <= lockfd) {
-			write_locked_index(&the_index, &index_lock, COMMIT_LOCK);
-		} else {
-			rollback_lock_file(&index_lock);
-		}
+		hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);
+		if (write_locked_index(&the_index, &index_lock, COMMIT_LOCK))
+			die("unable to write index file");
 	}
 
 	return 0;

Original file line number	Diff line number	Diff line change
`@@ -4058,7 +4058,7 @@ static int build_fake_ancestor(struct apply_state state, struct patch list)`
`4058`	`4058`	`{`
`4059`	`4059`	`struct patch *patch;`
`4060`	`4060`	`struct index_state result = { NULL };`
`4061`		`- static struct lock_file lock;`
	`4061`	`+ struct lock_file lock = LOCK_INIT;`
`4062`	`4062`	`int res;`
`4063`	`4063`
`4064`	`4064`	`/* Once we start supporting the reverse patch, it may be`
Original file line number	Diff line number	Diff line change
`@@ -612,7 +612,7 @@ int cmd_describe(int argc, const char *argv, const char prefix)`
`612`	`612`	`suffix = broken;`
`613`	`613`	`}`
`614`	`614`	`} else if (dirty) {`
`615`		`- static struct lock_file index_lock;`
	`615`	`+ struct lock_file index_lock = LOCK_INIT;`
`616`	`616`	`struct rev_info revs;`
`617`	`617`	`struct argv_array args = ARGV_ARRAY_INIT;`
`618`	`618`	`int fd, result;`
Original file line number	Diff line number	Diff line change
`@@ -373,7 +373,7 @@ static int need_to_gc(void)`
`373`	`373`	`/* return NULL on success, else hostname running the gc */`
`374`	`374`	`static const char lock_repo_for_gc(int force, pid_t ret_pid)`
`375`	`375`	`{`
`376`		`- static struct lock_file lock;`
	`376`	`+ struct lock_file lock = LOCK_INIT;`
`377`	`377`	`char my_host[HOST_NAME_MAX + 1];`
`378`	`378`	`struct strbuf sb = STRBUF_INIT;`
`379`	`379`	`struct stat st;`
Original file line number	Diff line number	Diff line change
`@@ -876,7 +876,7 @@ static void refuse_unconfigured_deny_delete_current(void)`
`876`	`876`	`static int command_singleton_iterator(void cb_data, struct object_id oid);`
`877`	`877`	`static int update_shallow_ref(struct command cmd, struct shallow_info si)`
`878`	`878`	`{`
`879`		`- static struct lock_file shallow_lock;`
	`879`	`+ struct lock_file shallow_lock = LOCK_INIT;`
`880`	`880`	`struct oid_array extra = OID_ARRAY_INIT;`
`881`	`881`	`struct check_connected_options opt = CHECK_CONNECTED_INIT;`
`882`	`882`	`uint32_t mask = 1 << (cmd->index % 32);`
Original file line number	Diff line number	Diff line change
`@@ -233,8 +233,6 @@ static int check_local_mod(struct object_id *head, int index_only)`
`233`	`233`	`return errs;`
`234`	`234`	`}`
`235`	`235`
`236`		`-static struct lock_file lock_file;`
`237`		`-`
`238`	`236`	`static int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;`
`239`	`237`	`static int ignore_unmatch = 0;`
`240`	`238`
`@@ -251,6 +249,7 @@ static struct option builtin_rm_options[] = {`
`251`	`249`
`252`	`250`	`int cmd_rm(int argc, const char *argv, const char prefix)`
`253`	`251`	`{`
	`252`	`+ struct lock_file lock_file = LOCK_INIT;`
`254`	`253`	`int i;`
`255`	`254`	`struct pathspec pathspec;`
`256`	`255`	`char *seen;`