On 25/09/17 12:33AM, Karthik Nayak wrote:

Justin Tobler [off-list ref] writes:

quoted

On 25/09/13 10:54PM, Karthik Nayak wrote:

quoted

+
 /*
  * Lock refname, without following symrefs, and set *lock_p to point
  * at a newly-allocated lock object. Fill in lock->old_oid, referent,

@@ -677,16 +697,17 @@ static void unlock_ref(struct ref_lock *lock)
  * - Generate informative error messages in the case of failure
  */
 static enum ref_transaction_error lock_raw_ref(struct files_ref_store *refs,
-					       struct ref_update *update,
+					       struct ref_transaction *transaction,
 					       size_t update_idx,
 					       int mustexist,
 					       struct string_list *refnames_to_check,
-					       const struct string_list *extras,
 					       struct ref_lock **lock_p,
 					       struct strbuf *referent,
 					       struct strbuf *err)
 {
 	enum ref_transaction_error ret = REF_TRANSACTION_ERROR_GENERIC;
+	struct ref_update *update = transaction->updates[update_idx];
+	const struct string_list *extras = &transaction->refnames;
 	const char *refname = update->refname;
 	unsigned int *type = &update->type;
 	struct ref_lock *lock;

@@ -776,6 +797,9 @@ static enum ref_transaction_error lock_raw_ref(struct files_ref_store *refs,
 			goto retry;
 		} else {
 			unable_to_lock_message(ref_file.buf, myerr, err);

huh, so if if we have a lockfile error due to a case-insensitve
filesystem, does this mean we print the error message from
`unable_to_lock_message()` and the new message?

If so, I wonder if we would be better off skipping the former since it
could be a bit misleading.

I would say both are necessary. The errors added here are more technical
and really talk about why we faced an issue. The error in
'builtin/fetch.c' is more about guidance to how to overcome that issue.

Also this error is client agnostic, so we'd add the error here for users
of both regular transactions and batched updates. The error in
'builtin/fetch.c' is very specific to users of 'git-fetch(1)'. So I
think both hold value.

Ah ok, I was orginally concerned that both error messages would be
printed when a reference update is rejected due to the case conflict
error. I see now that the "Unable to create '%s.lock': %s." message only
gets printed if the transaction actually fails. With this change, a case
conflict error on an individual references in batched updates doesn't
result in the entire transaction failing. Thus, only the new message is
printed. Makes sense.

Thanks,
-Justin