From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 16 Apr 2019 10:28:11 +0000 (+0900)
Subject: Merge branch 'jk/refs-double-abort'
X-Git-Tag: v2.22.0-rc0~97
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/2e08c892a7037ce2c7dfe30134eb54a2825bf0be?ds=inline;hp=-c

Merge branch 'jk/refs-double-abort'

A corner case bug in the refs API has been corrected.

* jk/refs-double-abort:
refs/files-backend: don't look at an aborted transaction
refs/files-backend: handle packed transaction prepare failure
---

2e08c892a7037ce2c7dfe30134eb54a2825bf0be
diff --combined refs/files-backend.c
index 5848f32ef8,09608e6c43..63e55e6773
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@@ -214,33 -214,6 +214,33 @@@ static void files_ref_path(struct files
  	}
  }
  
 +/*
 + * Manually add refs/bisect, refs/rewritten and refs/worktree, which, being
 + * per-worktree, might not appear in the directory listing for
 + * refs/ in the main repo.
 + */
 +static void add_per_worktree_entries_to_dir(struct ref_dir *dir, const char *dirname)
 +{
 +	const char *prefixes[] = { "refs/bisect/", "refs/worktree/", "refs/rewritten/" };
 +	int ip;
 +
 +	if (strcmp(dirname, "refs/"))
 +		return;
 +
 +	for (ip = 0; ip < ARRAY_SIZE(prefixes); ip++) {
 +		const char *prefix = prefixes[ip];
 +		int prefix_len = strlen(prefix);
 +		struct ref_entry *child_entry;
 +		int pos;
 +
 +		pos = search_ref_dir(dir, prefix, prefix_len);
 +		if (pos >= 0)
 +			continue;
 +		child_entry = create_dir_entry(dir->cache, prefix, prefix_len, 1);
 +		add_entry_to_dir(dir, child_entry);
 +	}
 +}
 +
  /*
   * Read the loose references from the namespace dirname into dir
   * (without recursing).  dirname must end with '/'.  dir must be the
@@@ -324,7 -297,28 +324,7 @@@ static void loose_fill_ref_dir(struct r
  	strbuf_release(&path);
  	closedir(d);
  
 -	/*
 -	 * Manually add refs/bisect and refs/worktree, which, being
 -	 * per-worktree, might not appear in the directory listing for
 -	 * refs/ in the main repo.
 -	 */
 -	if (!strcmp(dirname, "refs/")) {
 -		int pos = search_ref_dir(dir, "refs/bisect/", 12);
 -
 -		if (pos < 0) {
 -			struct ref_entry *child_entry = create_dir_entry(
 -					dir->cache, "refs/bisect/", 12, 1);
 -			add_entry_to_dir(dir, child_entry);
 -		}
 -
 -		pos = search_ref_dir(dir, "refs/worktree/", 11);
 -
 -		if (pos < 0) {
 -			struct ref_entry *child_entry = create_dir_entry(
 -					dir->cache, "refs/worktree/", 11, 1);
 -			add_entry_to_dir(dir, child_entry);
 -		}
 -	}
 +	add_per_worktree_entries_to_dir(dir, dirname);
  }
  
  static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs)
@@@ -2260,7 -2254,8 +2260,7 @@@ static int split_head_update(struct ref
   * Note that the new update will itself be subject to splitting when
   * the iteration gets to it.
   */
 -static int split_symref_update(struct files_ref_store *refs,
 -			       struct ref_update *update,
 +static int split_symref_update(struct ref_update *update,
  			       const char *referent,
  			       struct ref_transaction *transaction,
  			       struct string_list *affected_refnames,
@@@ -2454,7 -2449,7 +2454,7 @@@ static int lock_ref_for_update(struct f
  			 * of processing the split-off update, so we
  			 * don't have to do it here.
  			 */
 -			ret = split_symref_update(refs, update,
 +			ret = split_symref_update(update,
  						  referent.buf, transaction,
  						  affected_refnames, err);
  			if (ret)
@@@ -2701,18 -2696,32 +2701,32 @@@ static int files_transaction_prepare(st
  		if (is_packed_transaction_needed(refs->packed_ref_store,
  						 packed_transaction)) {
  			ret = ref_transaction_prepare(packed_transaction, err);
+ 			/*
+ 			 * A failure during the prepare step will abort
+ 			 * itself, but not free. Do that now, and disconnect
+ 			 * from the files_transaction so it does not try to
+ 			 * abort us when we hit the cleanup code below.
+ 			 */
+ 			if (ret) {
+ 				ref_transaction_free(packed_transaction);
+ 				backend_data->packed_transaction = NULL;
+ 			}
  		} else {
  			/*
  			 * We can skip rewriting the `packed-refs`
  			 * file. But we do need to leave it locked, so
  			 * that somebody else doesn't pack a reference
  			 * that we are trying to delete.
+ 			 *
+ 			 * We need to disconnect our transaction from
+ 			 * backend_data, since the abort (whether successful or
+ 			 * not) will free it.
  			 */
+ 			backend_data->packed_transaction = NULL;
  			if (ref_transaction_abort(packed_transaction, err)) {
  				ret = TRANSACTION_GENERIC_ERROR;
  				goto cleanup;
  			}
- 			backend_data->packed_transaction = NULL;
  		}
  	}