From: Junio C Hamano <gitster@pobox.com>
Date: Sun, 18 Nov 2018 09:23:56 +0000 (+0900)
Subject: Merge branch 'js/rebase-r-and-merge-head'
X-Git-Tag: v2.20.0-rc0~14
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/0de3a73eb6b36d5b8df82e62dd5679462f19edef?hp=-c

Merge branch 'js/rebase-r-and-merge-head'

Bugfix for the recently graduated "git rebase --rebase-merges".

* js/rebase-r-and-merge-head:
status: rebase and merge can be in progress at the same time
built-in rebase --skip/--abort: clean up stale .git/<name> files
rebase -i: include MERGE_HEAD into files to clean up
rebase -r: do not write MERGE_HEAD unless needed
rebase -r: demonstrate bug with conflicting merges
---

0de3a73eb6b36d5b8df82e62dd5679462f19edef
diff --combined builtin/rebase.c
index 337c5c7f99,017dce1b50..59e6f7852f
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@@ -23,6 -23,7 +23,7 @@@
  #include "revision.h"
  #include "commit-reach.h"
  #include "rerere.h"
+ #include "branch.h"
  
  static char const * const builtin_rebase_usage[] = {
  	N_("git rebase [-i] [options] [--exec <cmd>] [--onto <newbase>] "
@@@ -522,17 -523,12 +523,17 @@@ finished_rebase
  
  #define GIT_REFLOG_ACTION_ENVIRONMENT "GIT_REFLOG_ACTION"
  
 +#define RESET_HEAD_DETACH (1<<0)
 +#define RESET_HEAD_HARD (1<<1)
 +
  static int reset_head(struct object_id *oid, const char *action,
 -		      const char *switch_to_branch, int detach_head,
 +		      const char *switch_to_branch, unsigned flags,
  		      const char *reflog_orig_head, const char *reflog_head)
  {
 +	unsigned detach_head = flags & RESET_HEAD_DETACH;
 +	unsigned reset_hard = flags & RESET_HEAD_HARD;
  	struct object_id head_oid;
 -	struct tree_desc desc;
 +	struct tree_desc desc[2] = { { NULL }, { NULL } };
  	struct lock_file lock = LOCK_INIT;
  	struct unpack_trees_options unpack_tree_opts;
  	struct tree *tree;
@@@ -541,62 -537,60 +542,62 @@@
  	size_t prefix_len;
  	struct object_id *orig = NULL, oid_orig,
  		*old_orig = NULL, oid_old_orig;
 -	int ret = 0;
 +	int ret = 0, nr = 0;
  
  	if (switch_to_branch && !starts_with(switch_to_branch, "refs/"))
  		BUG("Not a fully qualified branch: '%s'", switch_to_branch);
  
 -	if (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0)
 -		return -1;
 +	if (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0) {
 +		ret = -1;
 +		goto leave_reset_head;
 +	}
  
 -	if (!oid) {
 -		if (get_oid("HEAD", &head_oid)) {
 -			rollback_lock_file(&lock);
 -			return error(_("could not determine HEAD revision"));
 -		}
 -		oid = &head_oid;
 +	if ((!oid || !reset_hard) && get_oid("HEAD", &head_oid)) {
 +		ret = error(_("could not determine HEAD revision"));
 +		goto leave_reset_head;
  	}
  
 +	if (!oid)
 +		oid = &head_oid;
 +
  	memset(&unpack_tree_opts, 0, sizeof(unpack_tree_opts));
  	setup_unpack_trees_porcelain(&unpack_tree_opts, action);
  	unpack_tree_opts.head_idx = 1;
  	unpack_tree_opts.src_index = the_repository->index;
  	unpack_tree_opts.dst_index = the_repository->index;
 -	unpack_tree_opts.fn = oneway_merge;
 +	unpack_tree_opts.fn = reset_hard ? oneway_merge : twoway_merge;
  	unpack_tree_opts.update = 1;
  	unpack_tree_opts.merge = 1;
  	if (!detach_head)
  		unpack_tree_opts.reset = 1;
  
  	if (read_index_unmerged(the_repository->index) < 0) {
 -		rollback_lock_file(&lock);
 -		return error(_("could not read index"));
 +		ret = error(_("could not read index"));
 +		goto leave_reset_head;
  	}
  
 -	if (!fill_tree_descriptor(&desc, oid)) {
 -		error(_("failed to find tree of %s"), oid_to_hex(oid));
 -		rollback_lock_file(&lock);
 -		free((void *)desc.buffer);
 -		return -1;
 +	if (!reset_hard && !fill_tree_descriptor(&desc[nr++], &head_oid)) {
 +		ret = error(_("failed to find tree of %s"), oid_to_hex(oid));
 +		goto leave_reset_head;
  	}
  
 -	if (unpack_trees(1, &desc, &unpack_tree_opts)) {
 -		rollback_lock_file(&lock);
 -		free((void *)desc.buffer);
 -		return -1;
 +	if (!fill_tree_descriptor(&desc[nr++], oid)) {
 +		ret = error(_("failed to find tree of %s"), oid_to_hex(oid));
 +		goto leave_reset_head;
 +	}
 +
 +	if (unpack_trees(nr, desc, &unpack_tree_opts)) {
 +		ret = -1;
 +		goto leave_reset_head;
  	}
  
  	tree = parse_tree_indirect(oid);
  	prime_cache_tree(the_repository->index, tree);
  
 -	if (write_locked_index(the_repository->index, &lock, COMMIT_LOCK) < 0)
 +	if (write_locked_index(the_repository->index, &lock, COMMIT_LOCK) < 0) {
  		ret = error(_("could not write index"));
 -	free((void *)desc.buffer);
 -
 -	if (ret)
 -		return ret;
 +		goto leave_reset_head;
 +	}
  
  	reflog_action = getenv(GIT_REFLOG_ACTION_ENVIRONMENT);
  	strbuf_addf(&msg, "%s: ", reflog_action ? reflog_action : "rebase");
@@@ -620,8 -614,7 +621,8 @@@
  		reflog_head = msg.buf;
  	}
  	if (!switch_to_branch)
 -		ret = update_ref(reflog_head, "HEAD", oid, orig, REF_NO_DEREF,
 +		ret = update_ref(reflog_head, "HEAD", oid, orig,
 +				 detach_head ? REF_NO_DEREF : 0,
  				 UPDATE_REFS_MSG_ON_ERR);
  	else {
  		ret = create_symref("HEAD", switch_to_branch, msg.buf);
@@@ -630,11 -623,7 +631,11 @@@
  					 UPDATE_REFS_MSG_ON_ERR);
  	}
  
 +leave_reset_head:
  	strbuf_release(&msg);
 +	rollback_lock_file(&lock);
 +	while (nr)
 +		free((void *)desc[--nr].buffer);
  	return ret;
  }
  
@@@ -715,9 -704,6 +716,9 @@@ static int parse_opt_merge(const struc
  {
  	struct rebase_options *opts = opt->value;
  
 +	BUG_ON_OPT_NEG(unset);
 +	BUG_ON_OPT_ARG(arg);
 +
  	if (!is_interactive(opts))
  		opts->type = REBASE_MERGE;
  
@@@ -730,9 -716,6 +731,9 @@@ static int parse_opt_interactive(const 
  {
  	struct rebase_options *opts = opt->value;
  
 +	BUG_ON_OPT_NEG(unset);
 +	BUG_ON_OPT_ARG(arg);
 +
  	opts->type = REBASE_INTERACTIVE;
  	opts->flags |= REBASE_INTERACTIVE_EXPLICIT;
  
@@@ -1018,9 -1001,9 +1019,10 @@@ int cmd_rebase(int argc, const char **a
  		rerere_clear(&merge_rr);
  		string_list_clear(&merge_rr, 1);
  
 -		if (reset_head(NULL, "reset", NULL, 0, NULL, NULL) < 0)
 +		if (reset_head(NULL, "reset", NULL, RESET_HEAD_HARD,
 +			       NULL, NULL) < 0)
  			die(_("could not discard worktree changes"));
+ 		remove_branch_state();
  		if (read_basic_state(&options))
  			exit(1);
  		goto run_rebase;
@@@ -1035,10 -1018,10 +1037,11 @@@
  		if (read_basic_state(&options))
  			exit(1);
  		if (reset_head(&options.orig_head, "reset",
 -			       options.head_name, 0, NULL, NULL) < 0)
 +			       options.head_name, RESET_HEAD_HARD,
 +			       NULL, NULL) < 0)
  			die(_("could not move back to %s"),
  			    oid_to_hex(&options.orig_head));
+ 		remove_branch_state();
  		ret = finish_rebase(&options);
  		goto cleanup;
  	}
@@@ -1400,7 -1383,7 +1403,7 @@@
  			write_file(autostash, "%s", oid_to_hex(&oid));
  			printf(_("Created autostash: %s\n"), buf.buf);
  			if (reset_head(&head->object.oid, "reset --hard",
 -				       NULL, 0, NULL, NULL) < 0)
 +				       NULL, RESET_HEAD_HARD, NULL, NULL) < 0)
  				die(_("could not reset --hard"));
  			printf(_("HEAD is now at %s"),
  			       find_unique_abbrev(&head->object.oid,
@@@ -1520,8 -1503,8 +1523,8 @@@
  			 "it...\n"));
  
  	strbuf_addf(&msg, "rebase: checkout %s", options.onto_name);
 -	if (reset_head(&options.onto->object.oid, "checkout", NULL, 1,
 -	    NULL, msg.buf))
 +	if (reset_head(&options.onto->object.oid, "checkout", NULL,
 +		       RESET_HEAD_DETACH, NULL, msg.buf))
  		die(_("Could not detach HEAD"));
  	strbuf_release(&msg);
  
diff --combined sequencer.c
index 0d87b0739b,2f526390ac..e1a4dd15f1
--- a/sequencer.c
+++ b/sequencer.c
@@@ -669,131 -669,55 +669,131 @@@ missing_author
  	return res;
  }
  
 +/**
 + * Take a series of KEY='VALUE' lines where VALUE part is
 + * sq-quoted, and append <KEY, VALUE> at the end of the string list
 + */
 +static int parse_key_value_squoted(char *buf, struct string_list *list)
 +{
 +	while (*buf) {
 +		struct string_list_item *item;
 +		char *np;
 +		char *cp = strchr(buf, '=');
 +		if (!cp) {
 +			np = strchrnul(buf, '\n');
 +			return error(_("no key present in '%.*s'"),
 +				     (int) (np - buf), buf);
 +		}
 +		np = strchrnul(cp, '\n');
 +		*cp++ = '\0';
 +		item = string_list_append(list, buf);
 +
 +		buf = np + (*np == '\n');
 +		*np = '\0';
 +		cp = sq_dequote(cp);
 +		if (!cp)
 +			return error(_("unable to dequote value of '%s'"),
 +				     item->string);
 +		item->util = xstrdup(cp);
 +	}
 +	return 0;
 +}
  
 -/*
 - * write_author_script() used to fail to terminate the last line with a "'" and
 - * also escaped "'" incorrectly as "'\\\\''" rather than "'\\''". We check for
 - * the terminating "'" on the last line to see how "'" has been escaped in case
 - * git was upgraded while rebase was stopped.
 +/**
 + * Reads and parses the state directory's "author-script" file, and sets name,
 + * email and date accordingly.
 + * Returns 0 on success, -1 if the file could not be parsed.
 + *
 + * The author script is of the format:
 + *
 + *	GIT_AUTHOR_NAME='$author_name'
 + *	GIT_AUTHOR_EMAIL='$author_email'
 + *	GIT_AUTHOR_DATE='$author_date'
 + *
 + * where $author_name, $author_email and $author_date are quoted. We are strict
 + * with our parsing, as the file was meant to be eval'd in the old
 + * git-am.sh/git-rebase--interactive.sh scripts, and thus if the file differs
 + * from what this function expects, it is better to bail out than to do
 + * something that the user does not expect.
   */
 -static int quoting_is_broken(const char *s, size_t n)
 +int read_author_script(const char *path, char **name, char **email, char **date,
 +		       int allow_missing)
  {
 -	/* Skip any empty lines in case the file was hand edited */
 -	while (n > 0 && s[--n] == '\n')
 -		; /* empty */
 -	if (n > 0 && s[n] != '\'')
 -		return 1;
 +	struct strbuf buf = STRBUF_INIT;
 +	struct string_list kv = STRING_LIST_INIT_DUP;
 +	int retval = -1; /* assume failure */
 +	int i, name_i = -2, email_i = -2, date_i = -2, err = 0;
  
 -	return 0;
 +	if (strbuf_read_file(&buf, path, 256) <= 0) {
 +		strbuf_release(&buf);
 +		if (errno == ENOENT && allow_missing)
 +			return 0;
 +		else
 +			return error_errno(_("could not open '%s' for reading"),
 +					   path);
 +	}
 +
 +	if (parse_key_value_squoted(buf.buf, &kv))
 +		goto finish;
 +
 +	for (i = 0; i < kv.nr; i++) {
 +		if (!strcmp(kv.items[i].string, "GIT_AUTHOR_NAME")) {
 +			if (name_i != -2)
 +				name_i = error(_("'GIT_AUTHOR_NAME' already given"));
 +			else
 +				name_i = i;
 +		} else if (!strcmp(kv.items[i].string, "GIT_AUTHOR_EMAIL")) {
 +			if (email_i != -2)
 +				email_i = error(_("'GIT_AUTHOR_EMAIL' already given"));
 +			else
 +				email_i = i;
 +		} else if (!strcmp(kv.items[i].string, "GIT_AUTHOR_DATE")) {
 +			if (date_i != -2)
 +				date_i = error(_("'GIT_AUTHOR_DATE' already given"));
 +			else
 +				date_i = i;
 +		} else {
 +			err = error(_("unknown variable '%s'"),
 +				    kv.items[i].string);
 +		}
 +	}
 +	if (name_i == -2)
 +		error(_("missing 'GIT_AUTHOR_NAME'"));
 +	if (email_i == -2)
 +		error(_("missing 'GIT_AUTHOR_EMAIL'"));
 +	if (date_i == -2)
 +		error(_("missing 'GIT_AUTHOR_DATE'"));
 +	if (date_i < 0 || email_i < 0 || date_i < 0 || err)
 +		goto finish;
 +	*name = kv.items[name_i].util;
 +	*email = kv.items[email_i].util;
 +	*date = kv.items[date_i].util;
 +	retval = 0;
 +finish:
 +	string_list_clear(&kv, !!retval);
 +	strbuf_release(&buf);
 +	return retval;
  }
  
  /*
 - * Read a list of environment variable assignments (such as the author-script
 - * file) into an environment block. Returns -1 on error, 0 otherwise.
 + * Read a GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL AND GIT_AUTHOR_DATE from a
 + * file with shell quoting into struct argv_array. Returns -1 on
 + * error, 0 otherwise.
   */
  static int read_env_script(struct argv_array *env)
  {
 -	struct strbuf script = STRBUF_INIT;
 -	int i, count = 0, sq_bug;
 -	const char *p2;
 -	char *p;
 +	char *name, *email, *date;
  
 -	if (strbuf_read_file(&script, rebase_path_author_script(), 256) <= 0)
 +	if (read_author_script(rebase_path_author_script(),
 +			       &name, &email, &date, 0))
  		return -1;
 -	/* write_author_script() used to quote incorrectly */
 -	sq_bug = quoting_is_broken(script.buf, script.len);
 -	for (p = script.buf; *p; p++)
 -		if (sq_bug && skip_prefix(p, "'\\\\''", &p2))
 -			strbuf_splice(&script, p - script.buf, p2 - p, "'", 1);
 -		else if (skip_prefix(p, "'\\''", &p2))
 -			strbuf_splice(&script, p - script.buf, p2 - p, "'", 1);
 -		else if (*p == '\'')
 -			strbuf_splice(&script, p-- - script.buf, 1, "", 0);
 -		else if (*p == '\n') {
 -			*p = '\0';
 -			count++;
 -		}
  
 -	for (i = 0, p = script.buf; i < count; i++) {
 -		argv_array_push(env, p);
 -		p += strlen(p) + 1;
 -	}
 +	argv_array_pushf(env, "GIT_AUTHOR_NAME=%s", name);
 +	argv_array_pushf(env, "GIT_AUTHOR_EMAIL=%s", email);
 +	argv_array_pushf(env, "GIT_AUTHOR_DATE=%s", date);
 +	free(name);
 +	free(email);
 +	free(date);
  
  	return 0;
  }
@@@ -813,28 -737,54 +813,28 @@@ static char *get_author(const char *mes
  /* Read author-script and return an ident line (author <email> timestamp) */
  static const char *read_author_ident(struct strbuf *buf)
  {
 -	const char *keys[] = {
 -		"GIT_AUTHOR_NAME=", "GIT_AUTHOR_EMAIL=", "GIT_AUTHOR_DATE="
 -	};
  	struct strbuf out = STRBUF_INIT;
 -	char *in, *eol;
 -	const char *val[3];
 -	int i = 0;
 -
 -	if (strbuf_read_file(buf, rebase_path_author_script(), 256) <= 0)
 -		return NULL;
 -
 -	/* dequote values and construct ident line in-place */
 -	for (in = buf->buf; i < 3 && in - buf->buf < buf->len; i++) {
 -		if (!skip_prefix(in, keys[i], (const char **)&in)) {
 -			warning(_("could not parse '%s' (looking for '%s')"),
 -				rebase_path_author_script(), keys[i]);
 -			return NULL;
 -		}
 -
 -		eol = strchrnul(in, '\n');
 -		*eol = '\0';
 -		if (!sq_dequote(in)) {
 -			warning(_("bad quoting on %s value in '%s'"),
 -				keys[i], rebase_path_author_script());
 -			return NULL;
 -		}
 -		val[i] = in;
 -		in = eol + 1;
 -	}
 +	char *name, *email, *date;
  
 -	if (i < 3) {
 -		warning(_("could not parse '%s' (looking for '%s')"),
 -			rebase_path_author_script(), keys[i]);
 +	if (read_author_script(rebase_path_author_script(),
 +			       &name, &email, &date, 0))
  		return NULL;
 -	}
  
  	/* validate date since fmt_ident() will die() on bad value */
 -	if (parse_date(val[2], &out)){
 +	if (parse_date(date, &out)){
  		warning(_("invalid date format '%s' in '%s'"),
 -			val[2], rebase_path_author_script());
 +			date, rebase_path_author_script());
  		strbuf_release(&out);
  		return NULL;
  	}
  
  	strbuf_reset(&out);
 -	strbuf_addstr(&out, fmt_ident(val[0], val[1], val[2], 0));
 +	strbuf_addstr(&out, fmt_ident(name, email, date, 0));
  	strbuf_swap(buf, &out);
  	strbuf_release(&out);
 +	free(name);
 +	free(email);
 +	free(date);
  	return buf->buf;
  }
  
@@@ -1969,7 -1919,7 +1969,7 @@@ static int read_and_refresh_cache(struc
  {
  	struct lock_file index_lock = LOCK_INIT;
  	int index_fd = hold_locked_index(&index_lock, 0);
 -	if (read_index_preload(&the_index, NULL, 0) < 0) {
 +	if (read_index(&the_index) < 0) {
  		rollback_lock_file(&index_lock);
  		return error(_("git %s: failed to read the index"),
  			_(action_name(opts)));
@@@ -2940,7 -2890,7 +2940,7 @@@ static int do_reset(const char *name, i
  	struct tree_desc desc;
  	struct tree *tree;
  	struct unpack_trees_options unpack_tree_opts;
 -	int ret = 0, i;
 +	int ret = 0;
  
  	if (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0)
  		return -1;
@@@ -2960,13 -2910,10 +2960,13 @@@
  		}
  		oidcpy(&oid, &opts->squash_onto);
  	} else {
 +		int i;
 +
  		/* Determine the length of the label */
  		for (i = 0; i < len; i++)
  			if (isspace(name[i]))
 -				len = i;
 +				break;
 +		len = i;
  
  		strbuf_addf(&ref_name, "refs/rewritten/%.*s", len, name);
  		if (get_oid(ref_name.buf, &oid) &&
@@@ -3244,10 -3191,6 +3244,6 @@@ static int do_merge(struct commit *comm
  	}
  
  	merge_commit = to_merge->item;
- 	write_message(oid_to_hex(&merge_commit->object.oid), GIT_SHA1_HEXSZ,
- 		      git_path_merge_head(the_repository), 0);
- 	write_message("no-ff", 5, git_path_merge_mode(the_repository), 0);
- 
  	bases = get_merge_bases(head_commit, merge_commit);
  	if (bases && oideq(&merge_commit->object.oid,
  			   &bases->item->object.oid)) {
@@@ -3256,6 -3199,10 +3252,10 @@@
  		goto leave_merge;
  	}
  
+ 	write_message(oid_to_hex(&merge_commit->object.oid), GIT_SHA1_HEXSZ,
+ 		      git_path_merge_head(the_repository), 0);
+ 	write_message("no-ff", 5, git_path_merge_mode(the_repository), 0);
+ 
  	for (j = bases; j; j = j->next)
  		commit_list_insert(j->item, &reversed);
  	free_commit_list(bases);
@@@ -3512,6 -3459,7 +3512,7 @@@ static int pick_commits(struct todo_lis
  			unlink(rebase_path_author_script());
  			unlink(rebase_path_stopped_sha());
  			unlink(rebase_path_amend());
+ 			unlink(git_path_merge_head(the_repository));
  			delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
  
  			if (item->command == TODO_BREAK)
@@@ -3882,6 -3830,7 +3883,7 @@@ static int commit_staged_changes(struc
  			   opts, flags))
  		return error(_("could not commit staged changes."));
  	unlink(rebase_path_amend());
+ 	unlink(git_path_merge_head(the_repository));
  	if (final_fixup) {
  		unlink(rebase_path_fixup_msg());
  		unlink(rebase_path_squash_msg());
@@@ -4899,7 -4848,7 +4901,7 @@@ int complete_action(struct replay_opts 
  
  	if (checkout_onto(opts, onto_name, oid_to_hex(&oid), orig_head))
  		return -1;
 -;
 +
  	if (require_clean_work_tree("rebase", "", 1, 1))
  		return -1;