refs/reftable: extract code from the transaction preparation
Extract the core logic for preparing individual reference updates from `reftable_be_transaction_prepare()` into `prepare_single_update()`. This dedicated function now handles all validation and preparation steps for each reference update in the transaction, including object ID verification, HEAD reference handling, and symref processing. The refactoring consolidates all reference update validation into a single logical block, which improves code maintainability and readability. More importantly, this restructuring lays the groundwork for implementing batched reference update support in the reftable backend, which will be introduced in a followup commit. No functional changes are included in this commit - it is purely a code reorganization to support future enhancements. Signed-off-by: Karthik Nayak <karthik.188@gmail.com> Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Karthik Nayak committed
Apr 8, 2025 at 10:51 UTC
ca89c18d5cac11ca965b0f5088262c7b6210c572
1 file changed
+237
-226
refs/reftable-backend.c
+237
-226
@@ -1069,6 +1069,239 @@ static int queue_transaction_update(struct reftable_ref_store *refs,
1069
return 0;
1070
}
1071
1072
+static int prepare_single_update(struct reftable_ref_store *refs,
1073
+ struct reftable_transaction_data *tx_data,
1074
+ struct ref_transaction *transaction,
1075
+ struct reftable_backend *be,
1076
+ struct ref_update *u,
1077
+ struct string_list *refnames_to_check,
1078
+ unsigned int head_type,
1079
+ struct strbuf *head_referent,
1080
+ struct strbuf *referent,
1081
+ struct strbuf *err)
1082
+{
1083
+ struct object_id current_oid = {0};
1084
+ const char *rewritten_ref;
1085
+ int ret = 0;
1086
+
1087
+ /*
1088
+ * There is no need to reload the respective backends here as
1089
+ * we have already reloaded them when preparing the transaction
1090
+ * update. And given that the stacks have been locked there
1091
+ * shouldn't have been any concurrent modifications of the
1092
+ * stack.
1093
+ */
1094
+ ret = backend_for(&be, refs, u->refname, &rewritten_ref, 0);
1095
+ if (ret)
1096
+ return ret;
1097
+
1098
+ /* Verify that the new object ID is valid. */
1099
+ if ((u->flags & REF_HAVE_NEW) && !is_null_oid(&u->new_oid) &&
1100
+ !(u->flags & REF_SKIP_OID_VERIFICATION) &&
1101
+ !(u->flags & REF_LOG_ONLY)) {
1102
+ struct object *o = parse_object(refs->base.repo, &u->new_oid);
1103
+ if (!o) {
1104
+ strbuf_addf(err,
1105
+ _("trying to write ref '%s' with nonexistent object %s"),
1106
+ u->refname, oid_to_hex(&u->new_oid));
1107
+ return -1;
1108
+ }
1109
+
1110
+ if (o->type != OBJ_COMMIT && is_branch(u->refname)) {
1111
+ strbuf_addf(err, _("trying to write non-commit object %s to branch '%s'"),
1112
+ oid_to_hex(&u->new_oid), u->refname);
1113
+ return -1;
1114
+ }
1115
+ }
1116
+
1117
+ /*
1118
+ * When we update the reference that HEAD points to we enqueue
1119
+ * a second log-only update for HEAD so that its reflog is
1120
+ * updated accordingly.
1121
+ */
1122
+ if (head_type == REF_ISSYMREF &&
1123
+ !(u->flags & REF_LOG_ONLY) &&
1124
+ !(u->flags & REF_UPDATE_VIA_HEAD) &&
1125
+ !strcmp(rewritten_ref, head_referent->buf)) {
1126
+ /*
1127
+ * First make sure that HEAD is not already in the
1128
+ * transaction. This check is O(lg N) in the transaction
1129
+ * size, but it happens at most once per transaction.
1130
+ */
1131
+ if (string_list_has_string(&transaction->refnames, "HEAD")) {
1132
+ /* An entry already existed */
1133
+ strbuf_addf(err,
1134
+ _("multiple updates for 'HEAD' (including one "
1135
+ "via its referent '%s') are not allowed"),
1136
+ u->refname);
1137
+ return TRANSACTION_NAME_CONFLICT;
1138
+ }
1139
+
1140
+ ref_transaction_add_update(
1141
+ transaction, "HEAD",
1142
+ u->flags | REF_LOG_ONLY | REF_NO_DEREF,
1143
+ &u->new_oid, &u->old_oid, NULL, NULL, NULL,
1144
+ u->msg);
1145
+ }
1146
+
1147
+ ret = reftable_backend_read_ref(be, rewritten_ref,
1148
+ ¤t_oid, referent, &u->type);
1149
+ if (ret < 0)
1150
+ return ret;
1151
+ if (ret > 0 && !ref_update_expects_existing_old_ref(u)) {
1152
+ /*
1153
+ * The reference does not exist, and we either have no
1154
+ * old object ID or expect the reference to not exist.
1155
+ * We can thus skip below safety checks as well as the
1156
+ * symref splitting. But we do want to verify that
1157
+ * there is no conflicting reference here so that we
1158
+ * can output a proper error message instead of failing
1159
+ * at a later point.
1160
+ */
1161
+ string_list_append(refnames_to_check, u->refname);
1162
+
1163
+ /*
1164
+ * There is no need to write the reference deletion
1165
+ * when the reference in question doesn't exist.
1166
+ */
1167
+ if ((u->flags & REF_HAVE_NEW) && !ref_update_has_null_new_value(u)) {
1168
+ ret = queue_transaction_update(refs, tx_data, u,
1169
+ ¤t_oid, err);
1170
+ if (ret)
1171
+ return ret;
1172
+ }
1173
+
1174
+ return 0;
1175
+ }
1176
+ if (ret > 0) {
1177
+ /* The reference does not exist, but we expected it to. */
1178
+ strbuf_addf(err, _("cannot lock ref '%s': "
1179
+
1180
+
1181
+ "unable to resolve reference '%s'"),
1182
+ ref_update_original_update_refname(u), u->refname);
1183
+ return -1;
1184
+ }
1185
+
1186
+ if (u->type & REF_ISSYMREF) {
1187
+ /*
1188
+ * The reftable stack is locked at this point already,
1189
+ * so it is safe to call `refs_resolve_ref_unsafe()`
1190
+ * here without causing races.
1191
+ */
1192
+ const char *resolved = refs_resolve_ref_unsafe(&refs->base, u->refname, 0,
1193
+ ¤t_oid, NULL);
1194
+
1195
+ if (u->flags & REF_NO_DEREF) {
1196
+ if (u->flags & REF_HAVE_OLD && !resolved) {
1197
+ strbuf_addf(err, _("cannot lock ref '%s': "
1198
+ "error reading reference"), u->refname);
1199
+ return -1;
1200
+ }
1201
+ } else {
1202
+ struct ref_update *new_update;
1203
+ int new_flags;
1204
+
1205
+ new_flags = u->flags;
1206
+ if (!strcmp(rewritten_ref, "HEAD"))
1207
+ new_flags |= REF_UPDATE_VIA_HEAD;
1208
+
1209
+ if (string_list_has_string(&transaction->refnames, referent->buf)) {
1210
+ strbuf_addf(err,
1211
+ _("multiple updates for '%s' (including one "
1212
+ "via symref '%s') are not allowed"),
1213
+ referent->buf, u->refname);
1214
+ return TRANSACTION_NAME_CONFLICT;
1215
+ }
1216
+
1217
+ /*
1218
+ * If we are updating a symref (eg. HEAD), we should also
1219
+ * update the branch that the symref points to.
1220
+ *
1221
+ * This is generic functionality, and would be better
1222
+ * done in refs.c, but the current implementation is
1223
+ * intertwined with the locking in files-backend.c.
1224
+ */
1225
+ new_update = ref_transaction_add_update(
1226
+ transaction, referent->buf, new_flags,
1227
+ u->new_target ? NULL : &u->new_oid,
1228
+ u->old_target ? NULL : &u->old_oid,
1229
+ u->new_target, u->old_target,
1230
+ u->committer_info, u->msg);
1231
+
1232
+ new_update->parent_update = u;
1233
+
1234
+ /*
1235
+ * Change the symbolic ref update to log only. Also, it
1236
+ * doesn't need to check its old OID value, as that will be
1237
+ * done when new_update is processed.
1238
+ */
1239
+ u->flags |= REF_LOG_ONLY | REF_NO_DEREF;
1240
+ u->flags &= ~REF_HAVE_OLD;
1241
+ }
1242
+ }
1243
+
1244
+ /*
1245
+ * Verify that the old object matches our expectations. Note
1246
+ * that the error messages here do not make a lot of sense in
1247
+ * the context of the reftable backend as we never lock
1248
+ * individual refs. But the error messages match what the files
1249
+ * backend returns, which keeps our tests happy.
1250
+ */
1251
+ if (u->old_target) {
1252
+ if (!(u->type & REF_ISSYMREF)) {
1253
+ strbuf_addf(err, _("cannot lock ref '%s': "
1254
+ "expected symref with target '%s': "
1255
+ "but is a regular ref"),
1256
+ ref_update_original_update_refname(u),
1257
+ u->old_target);
1258
+ return -1;
1259
+ }
1260
+
1261
+ if (ref_update_check_old_target(referent->buf, u, err)) {
1262
+ return -1;
1263
+ }
1264
+ } else if ((u->flags & REF_HAVE_OLD) && !oideq(¤t_oid, &u->old_oid)) {
1265
+ if (is_null_oid(&u->old_oid)) {
1266
+ strbuf_addf(err, _("cannot lock ref '%s': "
1267
+ "reference already exists"),
1268
+ ref_update_original_update_refname(u));
1269
+ return TRANSACTION_CREATE_EXISTS;
1270
+ }
1271
+ else if (is_null_oid(¤t_oid))
1272
+ strbuf_addf(err, _("cannot lock ref '%s': "
1273
+ "reference is missing but expected %s"),
1274
+ ref_update_original_update_refname(u),
1275
+ oid_to_hex(&u->old_oid));
1276
+ else
1277
+ strbuf_addf(err, _("cannot lock ref '%s': "
1278
+ "is at %s but expected %s"),
1279
+ ref_update_original_update_refname(u),
1280
+ oid_to_hex(¤t_oid),
1281
+ oid_to_hex(&u->old_oid));
1282
+ return TRANSACTION_NAME_CONFLICT;
1283
+ }
1284
+
1285
+ /*
1286
+ * If all of the following conditions are true:
1287
+ *
1288
+ * - We're not about to write a symref.
1289
+ * - We're not about to write a log-only entry.
1290
+ * - Old and new object ID are different.
1291
+ *
1292
+ * Then we're essentially doing a no-op update that can be
1293
+ * skipped. This is not only for the sake of efficiency, but
1294
+ * also skips writing unneeded reflog entries.
1295
+ */
1296
+ if ((u->type & REF_ISSYMREF) ||
1297
+ (u->flags & REF_LOG_ONLY) ||
1298
+ (u->flags & REF_HAVE_NEW && !oideq(¤t_oid, &u->new_oid)))
1299
+ return queue_transaction_update(refs, tx_data, u,
1300
+ ¤t_oid, err);
1301
+
1302
+ return 0;
1303
+}
1304
+
1305
static int reftable_be_transaction_prepare(struct ref_store *ref_store,
1306
struct ref_transaction *transaction,
1307
struct strbuf *err)
@@ -1133,234 +1366,12 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store,
1366
ret = 0;
1367
1368
for (i = 0; i < transaction->nr; i++) {
1136
- struct ref_update *u = transaction->updates[i];
1137
- struct object_id current_oid = {0};
1138
- const char *rewritten_ref;
1139
-
1140
- /*
1141
- * There is no need to reload the respective backends here as
1142
- * we have already reloaded them when preparing the transaction
1143
- * update. And given that the stacks have been locked there
1144
- * shouldn't have been any concurrent modifications of the
1145
- * stack.
1146
- */
1147
- ret = backend_for(&be, refs, u->refname, &rewritten_ref, 0);
1369
+ ret = prepare_single_update(refs, tx_data, transaction, be,
1370
+ transaction->updates[i],
1371
+ &refnames_to_check, head_type,
1372
+ &head_referent, &referent, err);
1373
if (ret)
1374
goto done;
1150
-
1151
- /* Verify that the new object ID is valid. */
1152
- if ((u->flags & REF_HAVE_NEW) && !is_null_oid(&u->new_oid) &&
1153
- !(u->flags & REF_SKIP_OID_VERIFICATION) &&
1154
- !(u->flags & REF_LOG_ONLY)) {
1155
- struct object *o = parse_object(refs->base.repo, &u->new_oid);
1156
- if (!o) {
1157
- strbuf_addf(err,
1158
- _("trying to write ref '%s' with nonexistent object %s"),
1159
- u->refname, oid_to_hex(&u->new_oid));
1160
- ret = -1;
1161
- goto done;
1162
- }
1163
-
1164
- if (o->type != OBJ_COMMIT && is_branch(u->refname)) {
1165
- strbuf_addf(err, _("trying to write non-commit object %s to branch '%s'"),
1166
- oid_to_hex(&u->new_oid), u->refname);
1167
- ret = -1;
1168
- goto done;
1169
- }
1170
- }
1171
-
1172
- /*
1173
- * When we update the reference that HEAD points to we enqueue
1174
- * a second log-only update for HEAD so that its reflog is
1175
- * updated accordingly.
1176
- */
1177
- if (head_type == REF_ISSYMREF &&
1178
- !(u->flags & REF_LOG_ONLY) &&
1179
- !(u->flags & REF_UPDATE_VIA_HEAD) &&
1180
- !strcmp(rewritten_ref, head_referent.buf)) {
1181
- /*
1182
- * First make sure that HEAD is not already in the
1183
- * transaction. This check is O(lg N) in the transaction
1184
- * size, but it happens at most once per transaction.
1185
- */
1186
- if (string_list_has_string(&transaction->refnames, "HEAD")) {
1187
- /* An entry already existed */
1188
- strbuf_addf(err,
1189
- _("multiple updates for 'HEAD' (including one "
1190
- "via its referent '%s') are not allowed"),
1191
- u->refname);
1192
- ret = TRANSACTION_NAME_CONFLICT;
1193
- goto done;
1194
- }
1195
-
1196
- ref_transaction_add_update(
1197
- transaction, "HEAD",
1198
- u->flags | REF_LOG_ONLY | REF_NO_DEREF,
1199
- &u->new_oid, &u->old_oid, NULL, NULL, NULL,
1200
- u->msg);
1201
- }
1202
-
1203
- ret = reftable_backend_read_ref(be, rewritten_ref,
1204
- ¤t_oid, &referent, &u->type);
1205
- if (ret < 0)
1206
- goto done;
1207
- if (ret > 0 && !ref_update_expects_existing_old_ref(u)) {
1208
- /*
1209
- * The reference does not exist, and we either have no
1210
- * old object ID or expect the reference to not exist.
1211
- * We can thus skip below safety checks as well as the
1212
- * symref splitting. But we do want to verify that
1213
- * there is no conflicting reference here so that we
1214
- * can output a proper error message instead of failing
1215
- * at a later point.
1216
- */
1217
- string_list_append(&refnames_to_check, u->refname);
1218
-
1219
- /*
1220
- * There is no need to write the reference deletion
1221
- * when the reference in question doesn't exist.
1222
- */
1223
- if ((u->flags & REF_HAVE_NEW) && !ref_update_has_null_new_value(u)) {
1224
- ret = queue_transaction_update(refs, tx_data, u,
1225
- ¤t_oid, err);
1226
- if (ret)
1227
- goto done;
1228
- }
1229
-
1230
- continue;
1231
- }
1232
- if (ret > 0) {
1233
- /* The reference does not exist, but we expected it to. */
1234
- strbuf_addf(err, _("cannot lock ref '%s': "
1235
- "unable to resolve reference '%s'"),
1236
- ref_update_original_update_refname(u), u->refname);
1237
- ret = -1;
1238
- goto done;
1239
- }
1240
-
1241
- if (u->type & REF_ISSYMREF) {
1242
- /*
1243
- * The reftable stack is locked at this point already,
1244
- * so it is safe to call `refs_resolve_ref_unsafe()`
1245
- * here without causing races.
1246
- */
1247
- const char *resolved = refs_resolve_ref_unsafe(&refs->base, u->refname, 0,
1248
- ¤t_oid, NULL);
1249
-
1250
- if (u->flags & REF_NO_DEREF) {
1251
- if (u->flags & REF_HAVE_OLD && !resolved) {
1252
- strbuf_addf(err, _("cannot lock ref '%s': "
1253
- "error reading reference"), u->refname);
1254
- ret = -1;
1255
- goto done;
1256
- }
1257
- } else {
1258
- struct ref_update *new_update;
1259
- int new_flags;
1260
-
1261
- new_flags = u->flags;
1262
- if (!strcmp(rewritten_ref, "HEAD"))
1263
- new_flags |= REF_UPDATE_VIA_HEAD;
1264
-
1265
- if (string_list_has_string(&transaction->refnames, referent.buf)) {
1266
- strbuf_addf(err,
1267
- _("multiple updates for '%s' (including one "
1268
- "via symref '%s') are not allowed"),
1269
- referent.buf, u->refname);
1270
- ret = TRANSACTION_NAME_CONFLICT;
1271
- goto done;
1272
- }
1273
-
1274
- /*
1275
- * If we are updating a symref (eg. HEAD), we should also
1276
- * update the branch that the symref points to.
1277
- *
1278
- * This is generic functionality, and would be better
1279
- * done in refs.c, but the current implementation is
1280
- * intertwined with the locking in files-backend.c.
1281
- */
1282
- new_update = ref_transaction_add_update(
1283
- transaction, referent.buf, new_flags,
1284
- u->new_target ? NULL : &u->new_oid,
1285
- u->old_target ? NULL : &u->old_oid,
1286
- u->new_target, u->old_target,
1287
- u->committer_info, u->msg);
1288
-
1289
- new_update->parent_update = u;
1290
-
1291
- /*
1292
- * Change the symbolic ref update to log only. Also, it
1293
- * doesn't need to check its old OID value, as that will be
1294
- * done when new_update is processed.
1295
- */
1296
- u->flags |= REF_LOG_ONLY | REF_NO_DEREF;
1297
- u->flags &= ~REF_HAVE_OLD;
1298
- }
1299
- }
1300
-
1301
- /*
1302
- * Verify that the old object matches our expectations. Note
1303
- * that the error messages here do not make a lot of sense in
1304
- * the context of the reftable backend as we never lock
1305
- * individual refs. But the error messages match what the files
1306
- * backend returns, which keeps our tests happy.
1307
- */
1308
- if (u->old_target) {
1309
- if (!(u->type & REF_ISSYMREF)) {
1310
- strbuf_addf(err, _("cannot lock ref '%s': "
1311
- "expected symref with target '%s': "
1312
- "but is a regular ref"),
1313
- ref_update_original_update_refname(u),
1314
- u->old_target);
1315
- ret = -1;
1316
- goto done;
1317
- }
1318
-
1319
- if (ref_update_check_old_target(referent.buf, u, err)) {
1320
- ret = -1;
1321
- goto done;
1322
- }
1323
- } else if ((u->flags & REF_HAVE_OLD) && !oideq(¤t_oid, &u->old_oid)) {
1324
- ret = TRANSACTION_NAME_CONFLICT;
1325
- if (is_null_oid(&u->old_oid)) {
1326
- strbuf_addf(err, _("cannot lock ref '%s': "
1327
- "reference already exists"),
1328
- ref_update_original_update_refname(u));
1329
- ret = TRANSACTION_CREATE_EXISTS;
1330
- }
1331
- else if (is_null_oid(¤t_oid))
1332
- strbuf_addf(err, _("cannot lock ref '%s': "
1333
- "reference is missing but expected %s"),
1334
- ref_update_original_update_refname(u),
1335
- oid_to_hex(&u->old_oid));
1336
- else
1337
- strbuf_addf(err, _("cannot lock ref '%s': "
1338
- "is at %s but expected %s"),
1339
- ref_update_original_update_refname(u),
1340
- oid_to_hex(¤t_oid),
1341
- oid_to_hex(&u->old_oid));
1342
- goto done;
1343
- }
1344
-
1345
- /*
1346
- * If all of the following conditions are true:
1347
- *
1348
- * - We're not about to write a symref.
1349
- * - We're not about to write a log-only entry.
1350
- * - Old and new object ID are different.
1351
- *
1352
- * Then we're essentially doing a no-op update that can be
1353
- * skipped. This is not only for the sake of efficiency, but
1354
- * also skips writing unneeded reflog entries.
1355
- */
1356
- if ((u->type & REF_ISSYMREF) ||
1357
- (u->flags & REF_LOG_ONLY) ||
1358
- (u->flags & REF_HAVE_NEW && !oideq(¤t_oid, &u->new_oid))) {
1359
- ret = queue_transaction_update(refs, tx_data, u,
1360
- ¤t_oid, err);
1361
- if (ret)
1362
- goto done;
1363
- }
1375
}
1376
1377
ret = refs_verify_refnames_available(ref_store, &refnames_to_check,