refs: add support for transactional symref updates
The reference backends currently support transactional reference updates. While this is exposed to users via 'git-update-ref' and its '--stdin' mode, it is also used internally within various commands. However, we do not support transactional updates of symrefs. This commit adds support for symrefs in both the 'files' and the 'reftable' backend. Here, we add and use `ref_update_has_null_new_value()`, a helper function which is used to check if there is a new_value in a reference update. The new value could either be a symref target `new_target` or a OID `new_oid`. We also add another common function `ref_update_check_old_target` which will be used to check if the update's old_target corresponds to a reference's current target. Now transactional updates (verify, create, delete, update) can be used for: - regular refs - symbolic refs - conversion of regular to symbolic refs and vice versa This also allows us to expose this to users via new commands in 'git-update-ref' in the future. Note that a dangling symref update does not record a new reflog entry, which is unchanged before and after this commit. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Karthik Nayak committed
May 7, 2024 at 14:58 UTC
644daf7785d6f2fa74330fc246d2bd4d3f8bbab2
4 files changed
+197
-40
refs.c
+35
-2
@@ -1217,6 +1217,8 @@ void ref_transaction_free(struct ref_transaction *transaction)
1217
1218
for (i = 0; i < transaction->nr; i++) {
1219
free(transaction->updates[i]->msg);
1220
+ free((char *)transaction->updates[i]->new_target);
1221
+ free((char *)transaction->updates[i]->old_target);
1222
free(transaction->updates[i]);
1223
}
1224
free(transaction->updates);
@@ -1247,10 +1249,13 @@ struct ref_update *ref_transaction_add_update(
1249
1250
update->flags = flags;
1251
1250
- if (flags & REF_HAVE_NEW)
1252
+ update->new_target = xstrdup_or_null(new_target);
1253
+ update->old_target = xstrdup_or_null(old_target);
1254
+ if ((flags & REF_HAVE_NEW) && new_oid)
1255
oidcpy(&update->new_oid, new_oid);
1252
- if (flags & REF_HAVE_OLD)
1256
+ if ((flags & REF_HAVE_OLD) && old_oid)
1257
oidcpy(&update->old_oid, old_oid);
1258
+
1259
update->msg = normalize_reflog_message(msg);
1260
return update;
1261
}
@@ -1286,6 +1291,7 @@ int ref_transaction_update(struct ref_transaction *transaction,
1291
flags &= REF_TRANSACTION_UPDATE_ALLOWED_FLAGS;
1292
1293
flags |= (new_oid ? REF_HAVE_NEW : 0) | (old_oid ? REF_HAVE_OLD : 0);
1294
+ flags |= (new_target ? REF_HAVE_NEW : 0) | (old_target ? REF_HAVE_OLD : 0);
1295
1296
ref_transaction_add_update(transaction, refname, flags,
1297
new_oid, old_oid, new_target,
@@ -2822,3 +2828,30 @@ const char *ref_update_original_update_refname(struct ref_update *update)
2828
2829
return update->refname;
2830
}
2831
+
2832
+int ref_update_has_null_new_value(struct ref_update *update)
2833
+{
2834
+ return !update->new_target && is_null_oid(&update->new_oid);
2835
+}
2836
+
2837
+int ref_update_check_old_target(const char *referent, struct ref_update *update,
2838
+ struct strbuf *err)
2839
+{
2840
+ if (!update->old_target)
2841
+ BUG("called without old_target set");
2842
+
2843
+ if (!strcmp(referent, update->old_target))
2844
+ return 0;
2845
+
2846
+ if (!strcmp(referent, ""))
2847
+ strbuf_addf(err, "verifying symref target: '%s': "
2848
+ "reference is missing but expected %s",
2849
+ ref_update_original_update_refname(update),
2850
+ update->old_target);
2851
+ else
2852
+ strbuf_addf(err, "verifying symref target: '%s': "
2853
+ "is at %s but expected %s",
2854
+ ref_update_original_update_refname(update),
2855
+ referent, update->old_target);
2856
+ return -1;
2857
+}
refs/files-backend.c
+92
-21
@@ -2394,8 +2394,9 @@ static int split_symref_update(struct ref_update *update,
2394
2395
new_update = ref_transaction_add_update(
2396
transaction, referent, new_flags,
2397
- &update->new_oid, &update->old_oid,
2398
- NULL, NULL, update->msg);
2397
+ update->new_target ? NULL : &update->new_oid,
2398
+ update->old_target ? NULL : &update->old_oid,
2399
+ update->new_target, update->old_target, update->msg);
2400
2401
new_update->parent_update = update;
2402
@@ -2483,7 +2484,7 @@ static int lock_ref_for_update(struct files_ref_store *refs,
2484
2485
files_assert_main_repository(refs, "lock_ref_for_update");
2486
2486
- if ((update->flags & REF_HAVE_NEW) && is_null_oid(&update->new_oid))
2487
+ if ((update->flags & REF_HAVE_NEW) && ref_update_has_null_new_value(update))
2488
update->flags |= REF_DELETING;
2489
2490
if (head_ref) {
@@ -2526,7 +2527,14 @@ static int lock_ref_for_update(struct files_ref_store *refs,
2527
ret = TRANSACTION_GENERIC_ERROR;
2528
goto out;
2529
}
2529
- } else if (check_old_oid(update, &lock->old_oid, err)) {
2530
+ }
2531
+
2532
+ if (update->old_target) {
2533
+ if (ref_update_check_old_target(referent.buf, update, err)) {
2534
+ ret = TRANSACTION_GENERIC_ERROR;
2535
+ goto out;
2536
+ }
2537
+ } else if (check_old_oid(update, &lock->old_oid, err)) {
2538
ret = TRANSACTION_GENERIC_ERROR;
2539
goto out;
2540
}
@@ -2547,7 +2555,17 @@ static int lock_ref_for_update(struct files_ref_store *refs,
2555
} else {
2556
struct ref_update *parent_update;
2557
2550
- if (check_old_oid(update, &lock->old_oid, err)) {
2558
+ /*
2559
+ * Even if the ref is a regular ref, if `old_target` is set, we
2560
+ * check the referent value. Ideally `old_target` should only
2561
+ * be set for symrefs, but we're strict about its usage.
2562
+ */
2563
+ if (update->old_target) {
2564
+ if (ref_update_check_old_target(referent.buf, update, err)) {
2565
+ ret = TRANSACTION_GENERIC_ERROR;
2566
+ goto out;
2567
+ }
2568
+ } else if (check_old_oid(update, &lock->old_oid, err)) {
2569
ret = TRANSACTION_GENERIC_ERROR;
2570
goto out;
2571
}
@@ -2565,9 +2583,28 @@ static int lock_ref_for_update(struct files_ref_store *refs,
2583
}
2584
}
2585
2568
- if ((update->flags & REF_HAVE_NEW) &&
2569
- !(update->flags & REF_DELETING) &&
2570
- !(update->flags & REF_LOG_ONLY)) {
2586
+ if (update->new_target && !(update->flags & REF_LOG_ONLY)) {
2587
+ if (create_symref_lock(refs, lock, update->refname,
2588
+ update->new_target, err)) {
2589
+ ret = TRANSACTION_GENERIC_ERROR;
2590
+ goto out;
2591
+ }
2592
+
2593
+ if (close_ref_gently(lock)) {
2594
+ strbuf_addf(err, "couldn't close '%s.lock'",
2595
+ update->refname);
2596
+ ret = TRANSACTION_GENERIC_ERROR;
2597
+ goto out;
2598
+ }
2599
+
2600
+ /*
2601
+ * Once we have created the symref lock, the commit
2602
+ * phase of the transaction only needs to commit the lock.
2603
+ */
2604
+ update->flags |= REF_NEEDS_COMMIT;
2605
+ } else if ((update->flags & REF_HAVE_NEW) &&
2606
+ !(update->flags & REF_DELETING) &&
2607
+ !(update->flags & REF_LOG_ONLY)) {
2608
if (!(update->type & REF_ISSYMREF) &&
2609
oideq(&lock->old_oid, &update->new_oid)) {
2610
/*
@@ -2830,6 +2867,43 @@ cleanup:
2867
return ret;
2868
}
2869
2870
+static int parse_and_write_reflog(struct files_ref_store *refs,
2871
+ struct ref_update *update,
2872
+ struct ref_lock *lock,
2873
+ struct strbuf *err)
2874
+{
2875
+ if (update->new_target) {
2876
+ /*
2877
+ * We want to get the resolved OID for the target, to ensure
2878
+ * that the correct value is added to the reflog.
2879
+ */
2880
+ if (!refs_resolve_ref_unsafe(&refs->base, update->new_target,
2881
+ RESOLVE_REF_READING,
2882
+ &update->new_oid, NULL)) {
2883
+ /*
2884
+ * TODO: currently we skip creating reflogs for dangling
2885
+ * symref updates. It would be nice to capture this as
2886
+ * zero oid updates however.
2887
+ */
2888
+ return 0;
2889
+ }
2890
+ }
2891
+
2892
+ if (files_log_ref_write(refs, lock->ref_name, &lock->old_oid,
2893
+ &update->new_oid, update->msg, update->flags, err)) {
2894
+ char *old_msg = strbuf_detach(err, NULL);
2895
+
2896
+ strbuf_addf(err, "cannot update the ref '%s': %s",
2897
+ lock->ref_name, old_msg);
2898
+ free(old_msg);
2899
+ unlock_ref(lock);
2900
+ update->backend_data = NULL;
2901
+ return -1;
2902
+ }
2903
+
2904
+ return 0;
2905
+}
2906
+
2907
static int files_transaction_finish(struct ref_store *ref_store,
2908
struct ref_transaction *transaction,
2909
struct strbuf *err)
@@ -2860,23 +2934,20 @@ static int files_transaction_finish(struct ref_store *ref_store,
2934
2935
if (update->flags & REF_NEEDS_COMMIT ||
2936
update->flags & REF_LOG_ONLY) {
2863
- if (files_log_ref_write(refs,
2864
- lock->ref_name,
2865
- &lock->old_oid,
2866
- &update->new_oid,
2867
- update->msg, update->flags,
2868
- err)) {
2869
- char *old_msg = strbuf_detach(err, NULL);
2870
-
2871
- strbuf_addf(err, "cannot update the ref '%s': %s",
2872
- lock->ref_name, old_msg);
2873
- free(old_msg);
2874
- unlock_ref(lock);
2875
- update->backend_data = NULL;
2937
+ if (parse_and_write_reflog(refs, update, lock, err)) {
2938
ret = TRANSACTION_GENERIC_ERROR;
2939
goto cleanup;
2940
}
2941
}
2942
+
2943
+ /*
2944
+ * We try creating a symlink, if that succeeds we continue to the
2945
+ * next update. If not, we try and create a regular symref.
2946
+ */
2947
+ if (update->new_target && prefer_symlink_refs)
2948
+ if (!create_ref_symlink(lock, update->new_target))
2949
+ continue;
2950
+
2951
if (update->flags & REF_NEEDS_COMMIT) {
2952
clear_loose_ref_cache(refs);
2953
if (commit_ref(lock)) {
refs/refs-internal.h
+16
@@ -754,4 +754,20 @@ struct ref_store *maybe_debug_wrap_ref_store(const char *gitdir, struct ref_stor
754
*/
755
const char *ref_update_original_update_refname(struct ref_update *update);
756
757
+/*
758
+ * Helper function to check if the new value is null, this
759
+ * takes into consideration that the update could be a regular
760
+ * ref or a symbolic ref.
761
+ */
762
+int ref_update_has_null_new_value(struct ref_update *update);
763
+
764
+/*
765
+ * Check whether the old_target values stored in update are consistent
766
+ * with the referent, which is the symbolic reference's current value.
767
+ * If everything is OK, return 0; otherwise, write an error message to
768
+ * err and return -1.
769
+ */
770
+int ref_update_check_old_target(const char *referent, struct ref_update *update,
771
+ struct strbuf *err);
772
+
773
#endif /* REFS_REFS_INTERNAL_H */
refs/reftable-backend.c
+54
-17
@@ -843,7 +843,7 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
843
* There is no need to write the reference deletion
844
* when the reference in question doesn't exist.
845
*/
846
- if (u->flags & REF_HAVE_NEW && !is_null_oid(&u->new_oid)) {
846
+ if ((u->flags & REF_HAVE_NEW) && !ref_update_has_null_new_value(u)) {
847
ret = queue_transaction_update(refs, tx_data, u,
848
¤t_oid, err);
849
if (ret)
@@ -894,8 +894,10 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
894
* intertwined with the locking in files-backend.c.
895
*/
896
new_update = ref_transaction_add_update(
897
- transaction, referent.buf, new_flags,
898
- &u->new_oid, &u->old_oid, NULL, NULL, u->msg);
897
+ transaction, referent.buf, new_flags,
898
+ &u->new_oid, &u->old_oid, u->new_target,
899
+ u->old_target, u->msg);
900
+
901
new_update->parent_update = u;
902
903
/*
@@ -925,7 +927,12 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
927
* individual refs. But the error messages match what the files
928
* backend returns, which keeps our tests happy.
929
*/
928
- if (u->flags & REF_HAVE_OLD && !oideq(¤t_oid, &u->old_oid)) {
930
+ if (u->old_target) {
931
+ if (ref_update_check_old_target(referent.buf, u, err)) {
932
+ ret = -1;
933
+ goto done;
934
+ }
935
+ } else if ((u->flags & REF_HAVE_OLD) && !oideq(¤t_oid, &u->old_oid)) {
936
if (is_null_oid(&u->old_oid))
937
strbuf_addf(err, _("cannot lock ref '%s': "
938
"reference already exists"),
@@ -1030,7 +1037,9 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data
1037
* - `core.logAllRefUpdates` tells us to create the reflog for
1038
* the given ref.
1039
*/
1033
- if (u->flags & REF_HAVE_NEW && !(u->type & REF_ISSYMREF) && is_null_oid(&u->new_oid)) {
1040
+ if ((u->flags & REF_HAVE_NEW) &&
1041
+ !(u->type & REF_ISSYMREF) &&
1042
+ ref_update_has_null_new_value(u)) {
1043
struct reftable_log_record log = {0};
1044
struct reftable_iterator it = {0};
1045
@@ -1071,24 +1080,52 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data
1080
(u->flags & REF_FORCE_CREATE_REFLOG ||
1081
should_write_log(&arg->refs->base, u->refname))) {
1082
struct reftable_log_record *log;
1083
+ int create_reflog = 1;
1084
+
1085
+ if (u->new_target) {
1086
+ if (!refs_resolve_ref_unsafe(&arg->refs->base, u->new_target,
1087
+ RESOLVE_REF_READING, &u->new_oid, NULL)) {
1088
+ /*
1089
+ * TODO: currently we skip creating reflogs for dangling
1090
+ * symref updates. It would be nice to capture this as
1091
+ * zero oid updates however.
1092
+ */
1093
+ create_reflog = 0;
1094
+ }
1095
+ }
1096
1075
- ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
1076
- log = &logs[logs_nr++];
1077
- memset(log, 0, sizeof(*log));
1078
-
1079
- fill_reftable_log_record(log);
1080
- log->update_index = ts;
1081
- log->refname = xstrdup(u->refname);
1082
- memcpy(log->value.update.new_hash, u->new_oid.hash, GIT_MAX_RAWSZ);
1083
- memcpy(log->value.update.old_hash, tx_update->current_oid.hash, GIT_MAX_RAWSZ);
1084
- log->value.update.message =
1085
- xstrndup(u->msg, arg->refs->write_options.block_size / 2);
1097
+ if (create_reflog) {
1098
+ ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
1099
+ log = &logs[logs_nr++];
1100
+ memset(log, 0, sizeof(*log));
1101
+
1102
+ fill_reftable_log_record(log);
1103
+ log->update_index = ts;
1104
+ log->refname = xstrdup(u->refname);
1105
+ memcpy(log->value.update.new_hash,
1106
+ u->new_oid.hash, GIT_MAX_RAWSZ);
1107
+ memcpy(log->value.update.old_hash,
1108
+ tx_update->current_oid.hash, GIT_MAX_RAWSZ);
1109
+ log->value.update.message =
1110
+ xstrndup(u->msg, arg->refs->write_options.block_size / 2);
1111
+ }
1112
}
1113
1114
if (u->flags & REF_LOG_ONLY)
1115
continue;
1116
1091
- if (u->flags & REF_HAVE_NEW && is_null_oid(&u->new_oid)) {
1117
+ if (u->new_target) {
1118
+ struct reftable_ref_record ref = {
1119
+ .refname = (char *)u->refname,
1120
+ .value_type = REFTABLE_REF_SYMREF,
1121
+ .value.symref = (char *)u->new_target,
1122
+ .update_index = ts,
1123
+ };
1124
+
1125
+ ret = reftable_writer_add_ref(writer, &ref);
1126
+ if (ret < 0)
1127
+ goto done;
1128
+ } else if ((u->flags & REF_HAVE_NEW) && ref_update_has_null_new_value(u)) {
1129
struct reftable_ref_record ref = {
1130
.refname = (char *)u->refname,
1131
.update_index = ts,