From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 14 Mar 2018 19:01:03 +0000 (-0700)
Subject: Merge branch 'ma/roll-back-lockfiles'
X-Git-Tag: v2.17.0-rc0~16
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/787aa97f217eb97827205271daba6d95c80b9049?hp=-c

Merge branch 'ma/roll-back-lockfiles'

Some codepaths used to take a lockfile and did not roll it back;
they are automatically rolled back at program exit, so there is no
real "breakage", but it still is a good practice to roll back when
you are done with a lockfile.

* ma/roll-back-lockfiles:
sequencer: do not roll back lockfile unnecessarily
merge: always roll back lock in `checkout_fast_forward()`
merge-recursive: always roll back lock in `merge_recursive_generic()`
sequencer: always roll back lock in `do_recursive_merge()`
sequencer: make lockfiles non-static
---

787aa97f217eb97827205271daba6d95c80b9049
diff --combined sequencer.c
index 969755b7e0,548ad997b5..091bd6bda5
--- a/sequencer.c
+++ b/sequencer.c
@@@ -339,7 -339,7 +339,7 @@@ static void print_advice(int show_hint
  static int write_message(const void *buf, size_t len, const char *filename,
  			 int append_eol)
  {
- 	static struct lock_file msg_file;
+ 	struct lock_file msg_file = LOCK_INIT;
  
  	int msg_fd = hold_lock_file_for_update(&msg_file, filename, 0);
  	if (msg_fd < 0)
@@@ -352,10 -352,8 +352,8 @@@
  		rollback_lock_file(&msg_file);
  		return error_errno(_("could not write eol to '%s'"), filename);
  	}
- 	if (commit_lock_file(&msg_file) < 0) {
- 		rollback_lock_file(&msg_file);
- 		return error(_("failed to finalize '%s'."), filename);
- 	}
+ 	if (commit_lock_file(&msg_file) < 0)
+ 		return error(_("failed to finalize '%s'"), filename);
  
  	return 0;
  }
@@@ -485,7 -483,7 +483,7 @@@ static int do_recursive_merge(struct co
  	struct tree *result, *next_tree, *base_tree, *head_tree;
  	int clean;
  	char **xopt;
- 	static struct lock_file index_lock;
+ 	struct lock_file index_lock = LOCK_INIT;
  
  	if (hold_locked_index(&index_lock, LOCK_REPORT_ON_ERROR) < 0)
  		return -1;
@@@ -514,8 -512,10 +512,10 @@@
  		fputs(o.obuf.buf, stdout);
  	strbuf_release(&o.obuf);
  	diff_warn_rename_limit("merge.renamelimit", o.needed_rename_limit, 0);
- 	if (clean < 0)
+ 	if (clean < 0) {
+ 		rollback_lock_file(&index_lock);
  		return clean;
+ 	}
  
  	if (active_cache_changed &&
  	    write_locked_index(&the_index, &index_lock, COMMIT_LOCK))
@@@ -1705,7 -1705,7 +1705,7 @@@ static int prepare_revs(struct replay_o
  
  static int read_and_refresh_cache(struct replay_opts *opts)
  {
- 	static struct lock_file index_lock;
+ 	struct lock_file index_lock = LOCK_INIT;
  	int index_fd = hold_locked_index(&index_lock, 0);
  	if (read_index_preload(&the_index, NULL) < 0) {
  		rollback_lock_file(&index_lock);
@@@ -1869,31 -1869,22 +1869,31 @@@ static int count_commands(struct todo_l
  	return count;
  }
  
 +static ssize_t strbuf_read_file_or_whine(struct strbuf *sb, const char *path)
 +{
 +	int fd;
 +	ssize_t len;
 +
 +	fd = open(path, O_RDONLY);
 +	if (fd < 0)
 +		return error_errno(_("could not open '%s'"), path);
 +	len = strbuf_read(sb, fd, 0);
 +	close(fd);
 +	if (len < 0)
 +		return error(_("could not read '%s'."), path);
 +	return len;
 +}
 +
  static int read_populate_todo(struct todo_list *todo_list,
  			struct replay_opts *opts)
  {
  	struct stat st;
  	const char *todo_file = get_todo_path(opts);
 -	int fd, res;
 +	int res;
  
  	strbuf_reset(&todo_list->buf);
 -	fd = open(todo_file, O_RDONLY);
 -	if (fd < 0)
 -		return error_errno(_("could not open '%s'"), todo_file);
 -	if (strbuf_read(&todo_list->buf, fd, 0) < 0) {
 -		close(fd);
 -		return error(_("could not read '%s'."), todo_file);
 -	}
 -	close(fd);
 +	if (strbuf_read_file_or_whine(&todo_list->buf, todo_file) < 0)
 +		return -1;
  
  	res = stat(todo_file, &st);
  	if (res)
@@@ -2108,16 -2099,14 +2108,14 @@@ static int create_seq_dir(void
  
  static int save_head(const char *head)
  {
- 	static struct lock_file head_lock;
+ 	struct lock_file head_lock = LOCK_INIT;
  	struct strbuf buf = STRBUF_INIT;
  	int fd;
  	ssize_t written;
  
  	fd = hold_lock_file_for_update(&head_lock, git_path_head_file(), 0);
- 	if (fd < 0) {
- 		rollback_lock_file(&head_lock);
+ 	if (fd < 0)
  		return error_errno(_("could not lock HEAD"));
- 	}
  	strbuf_addf(&buf, "%s\n", head);
  	written = write_in_full(fd, buf.buf, buf.len);
  	strbuf_release(&buf);
@@@ -2126,10 -2115,8 +2124,8 @@@
  		return error_errno(_("could not write to '%s'"),
  				   git_path_head_file());
  	}
- 	if (commit_lock_file(&head_lock) < 0) {
- 		rollback_lock_file(&head_lock);
- 		return error(_("failed to finalize '%s'."), git_path_head_file());
- 	}
+ 	if (commit_lock_file(&head_lock) < 0)
+ 		return error(_("failed to finalize '%s'"), git_path_head_file());
  	return 0;
  }
  
@@@ -2233,7 -2220,7 +2229,7 @@@ fail
  
  static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)
  {
- 	static struct lock_file todo_lock;
+ 	struct lock_file todo_lock = LOCK_INIT;
  	const char *todo_path = get_todo_path(opts);
  	int next = todo_list->current, offset, fd;
  
@@@ -2253,7 -2240,7 +2249,7 @@@
  			todo_list->buf.len - offset) < 0)
  		return error_errno(_("could not write to '%s'"), todo_path);
  	if (commit_lock_file(&todo_lock) < 0)
- 		return error(_("failed to finalize '%s'."), todo_path);
+ 		return error(_("failed to finalize '%s'"), todo_path);
  
  	if (is_rebase_i(opts)) {
  		const char *done_path = rebase_path_done();
@@@ -2323,9 -2310,6 +2319,9 @@@ static int make_patch(struct commit *co
  	p = short_commit_name(commit);
  	if (write_message(p, strlen(p), rebase_path_stopped_sha(), 1) < 0)
  		return -1;
 +	if (update_ref("rebase", "REBASE_HEAD", &commit->object.oid,
 +		       NULL, REF_NO_DEREF, UPDATE_REFS_MSG_ON_ERR))
 +		res |= error(_("could not update %s"), "REBASE_HEAD");
  
  	strbuf_addf(&buf, "%s/patch", get_dir(opts));
  	memset(&log_tree_opt, 0, sizeof(log_tree_opt));
@@@ -2577,7 -2561,6 +2573,7 @@@ static int pick_commits(struct todo_lis
  			unlink(rebase_path_author_script());
  			unlink(rebase_path_stopped_sha());
  			unlink(rebase_path_amend());
 +			delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
  		}
  		if (item->command <= TODO_SQUASH) {
  			if (is_rebase_i(opts))
@@@ -2883,7 -2866,7 +2879,7 @@@ int sequencer_pick_revisions(struct rep
  			if (!lookup_commit_reference_gently(&oid, 1)) {
  				enum object_type type = sha1_object_info(oid.hash, NULL);
  				return error(_("%s: can't cherry-pick a %s"),
 -					name, typename(type));
 +					name, type_name(type));
  			}
  		} else
  			return error(_("%s: bad revision"), name);
@@@ -3164,13 -3147,20 +3160,13 @@@ int check_todo_list(void
  	struct strbuf todo_file = STRBUF_INIT;
  	struct todo_list todo_list = TODO_LIST_INIT;
  	struct strbuf missing = STRBUF_INIT;
 -	int advise_to_edit_todo = 0, res = 0, fd, i;
 +	int advise_to_edit_todo = 0, res = 0, i;
  
  	strbuf_addstr(&todo_file, rebase_path_todo());
 -	fd = open(todo_file.buf, O_RDONLY);
 -	if (fd < 0) {
 -		res = error_errno(_("could not open '%s'"), todo_file.buf);
 -		goto leave_check;
 -	}
 -	if (strbuf_read(&todo_list.buf, fd, 0) < 0) {
 -		close(fd);
 -		res = error(_("could not read '%s'."), todo_file.buf);
 +	if (strbuf_read_file_or_whine(&todo_list.buf, todo_file.buf) < 0) {
 +		res = -1;
  		goto leave_check;
  	}
 -	close(fd);
  	advise_to_edit_todo = res =
  		parse_insn_buffer(todo_list.buf.buf, &todo_list);
  
@@@ -3186,10 -3176,17 +3182,10 @@@
  
  	todo_list_release(&todo_list);
  	strbuf_addstr(&todo_file, ".backup");
 -	fd = open(todo_file.buf, O_RDONLY);
 -	if (fd < 0) {
 -		res = error_errno(_("could not open '%s'"), todo_file.buf);
 -		goto leave_check;
 -	}
 -	if (strbuf_read(&todo_list.buf, fd, 0) < 0) {
 -		close(fd);
 -		res = error(_("could not read '%s'."), todo_file.buf);
 +	if (strbuf_read_file_or_whine(&todo_list.buf, todo_file.buf) < 0) {
 +		res = -1;
  		goto leave_check;
  	}
 -	close(fd);
  	strbuf_release(&todo_file);
  	res = !!parse_insn_buffer(todo_list.buf.buf, &todo_list);
  
@@@ -3270,8 -3267,15 +3266,8 @@@ int skip_unnecessary_picks(void
  	}
  	strbuf_release(&buf);
  
 -	fd = open(todo_file, O_RDONLY);
 -	if (fd < 0) {
 -		return error_errno(_("could not open '%s'"), todo_file);
 -	}
 -	if (strbuf_read(&todo_list.buf, fd, 0) < 0) {
 -		close(fd);
 -		return error(_("could not read '%s'."), todo_file);
 -	}
 -	close(fd);
 +	if (strbuf_read_file_or_whine(&todo_list.buf, todo_file) < 0)
 +		return -1;
  	if (parse_insn_buffer(todo_list.buf.buf, &todo_list) < 0) {
  		todo_list_release(&todo_list);
  		return -1;
@@@ -3362,11 -3366,17 +3358,11 @@@ int rearrange_squash(void
  	const char *todo_file = rebase_path_todo();
  	struct todo_list todo_list = TODO_LIST_INIT;
  	struct hashmap subject2item;
 -	int res = 0, rearranged = 0, *next, *tail, fd, i;
 +	int res = 0, rearranged = 0, *next, *tail, i;
  	char **subjects;
  
 -	fd = open(todo_file, O_RDONLY);
 -	if (fd < 0)
 -		return error_errno(_("could not open '%s'"), todo_file);
 -	if (strbuf_read(&todo_list.buf, fd, 0) < 0) {
 -		close(fd);
 -		return error(_("could not read '%s'."), todo_file);
 -	}
 -	close(fd);
 +	if (strbuf_read_file_or_whine(&todo_list.buf, todo_file) < 0)
 +		return -1;
  	if (parse_insn_buffer(todo_list.buf.buf, &todo_list) < 0) {
  		todo_list_release(&todo_list);
  		return -1;