Git fork

Merge branch 'kn/reflog-migration-fix-followup'

Code clean-up.

* kn/reflog-migration-fix-followup:
reftable: prevent 'update_index' changes after adding records
refs: use 'uint64_t' for 'ref_update.index'
refs: mark `ref_transaction_update_reflog()` as static

+118 -46
+16 -8
refs.c
··· 1318 1318 return 0; 1319 1319 } 1320 1320 1321 - int ref_transaction_update_reflog(struct ref_transaction *transaction, 1322 - const char *refname, 1323 - const struct object_id *new_oid, 1324 - const struct object_id *old_oid, 1325 - const char *committer_info, unsigned int flags, 1326 - const char *msg, unsigned int index, 1327 - struct strbuf *err) 1321 + /* 1322 + * Similar to`ref_transaction_update`, but this function is only for adding 1323 + * a reflog update. Supports providing custom committer information. The index 1324 + * field can be utiltized to order updates as desired. When not used, the 1325 + * updates default to being ordered by refname. 1326 + */ 1327 + static int ref_transaction_update_reflog(struct ref_transaction *transaction, 1328 + const char *refname, 1329 + const struct object_id *new_oid, 1330 + const struct object_id *old_oid, 1331 + const char *committer_info, 1332 + unsigned int flags, 1333 + const char *msg, 1334 + uint64_t index, 1335 + struct strbuf *err) 1328 1336 { 1329 1337 struct ref_update *update; 1330 1338 ··· 2805 2813 } 2806 2814 2807 2815 struct reflog_migration_data { 2808 - unsigned int index; 2816 + uint64_t index; 2809 2817 const char *refname; 2810 2818 struct ref_store *old_refs; 2811 2819 struct ref_transaction *transaction;
-14
refs.h
··· 772 772 struct strbuf *err); 773 773 774 774 /* 775 - * Similar to`ref_transaction_update`, but this function is only for adding 776 - * a reflog update. Supports providing custom committer information. The index 777 - * field can be utiltized to order updates as desired. When not used, the 778 - * updates default to being ordered by refname. 779 - */ 780 - int ref_transaction_update_reflog(struct ref_transaction *transaction, 781 - const char *refname, 782 - const struct object_id *new_oid, 783 - const struct object_id *old_oid, 784 - const char *committer_info, unsigned int flags, 785 - const char *msg, unsigned int index, 786 - struct strbuf *err); 787 - 788 - /* 789 775 * Add a reference creation to transaction. new_oid is the value that 790 776 * the reference should have after the update; it must not be 791 777 * null_oid. It is verified that the reference does not exist
+2 -2
refs/refs-internal.h
··· 120 120 * when migrating reflogs and we want to ensure we carry over the 121 121 * same order. 122 122 */ 123 - unsigned int index; 123 + uint64_t index; 124 124 125 125 /* 126 126 * If this ref_update was split off of a symref update via ··· 203 203 enum ref_transaction_state state; 204 204 void *backend_data; 205 205 unsigned int flags; 206 - unsigned int max_index; 206 + uint64_t max_index; 207 207 }; 208 208 209 209 /*
+16 -6
refs/reftable-backend.c
··· 942 942 size_t updates_nr; 943 943 size_t updates_alloc; 944 944 size_t updates_expected; 945 - unsigned int max_index; 945 + uint64_t max_index; 946 946 }; 947 947 948 948 struct reftable_transaction_data { ··· 1444 1444 * multiple entries. Each entry will contain a different update_index, 1445 1445 * so set the limits accordingly. 1446 1446 */ 1447 - reftable_writer_set_limits(writer, ts, ts + arg->max_index); 1447 + ret = reftable_writer_set_limits(writer, ts, ts + arg->max_index); 1448 + if (ret < 0) 1449 + goto done; 1448 1450 1449 1451 for (i = 0; i < arg->updates_nr; i++) { 1450 1452 struct reftable_transaction_update *tx_update = &arg->updates[i]; ··· 1766 1768 deletion_ts = creation_ts = reftable_stack_next_update_index(arg->be->stack); 1767 1769 if (arg->delete_old) 1768 1770 creation_ts++; 1769 - reftable_writer_set_limits(writer, deletion_ts, creation_ts); 1771 + ret = reftable_writer_set_limits(writer, deletion_ts, creation_ts); 1772 + if (ret < 0) 1773 + goto done; 1770 1774 1771 1775 /* 1772 1776 * Add the new reference. If this is a rename then we also delete the ··· 2298 2302 if (ret <= 0) 2299 2303 goto done; 2300 2304 2301 - reftable_writer_set_limits(writer, ts, ts); 2305 + ret = reftable_writer_set_limits(writer, ts, ts); 2306 + if (ret < 0) 2307 + goto done; 2302 2308 2303 2309 /* 2304 2310 * The existence entry has both old and new object ID set to the ··· 2357 2363 uint64_t ts = reftable_stack_next_update_index(arg->stack); 2358 2364 int ret; 2359 2365 2360 - reftable_writer_set_limits(writer, ts, ts); 2366 + ret = reftable_writer_set_limits(writer, ts, ts); 2367 + if (ret < 0) 2368 + goto out; 2361 2369 2362 2370 ret = reftable_stack_init_log_iterator(arg->stack, &it); 2363 2371 if (ret < 0) ··· 2434 2442 if (arg->records[i].value_type == REFTABLE_LOG_UPDATE) 2435 2443 live_records++; 2436 2444 2437 - reftable_writer_set_limits(writer, ts, ts); 2445 + ret = reftable_writer_set_limits(writer, ts, ts); 2446 + if (ret < 0) 2447 + return ret; 2438 2448 2439 2449 if (!is_null_oid(&arg->update_oid)) { 2440 2450 struct reftable_ref_record ref = {0};
+1
reftable/reftable-error.h
··· 30 30 31 31 /* Misuse of the API: 32 32 * - on writing a record with NULL refname. 33 + * - on writing a record before setting the writer limits. 33 34 * - on writing a reftable_ref_record outside the table limits 34 35 * - on writing a ref or log record before the stack's 35 36 * next_update_inde*x
+14 -10
reftable/reftable-writer.h
··· 124 124 int (*flush_func)(void *), 125 125 void *writer_arg, const struct reftable_write_options *opts); 126 126 127 - /* Set the range of update indices for the records we will add. When writing a 128 - table into a stack, the min should be at least 129 - reftable_stack_next_update_index(), or REFTABLE_API_ERROR is returned. 130 - 131 - For transactional updates to a stack, typically min==max, and the 132 - update_index can be obtained by inspeciting the stack. When converting an 133 - existing ref database into a single reftable, this would be a range of 134 - update-index timestamps. 127 + /* 128 + * Set the range of update indices for the records we will add. When writing a 129 + * table into a stack, the min should be at least 130 + * reftable_stack_next_update_index(), or REFTABLE_API_ERROR is returned. 131 + * 132 + * For transactional updates to a stack, typically min==max, and the 133 + * update_index can be obtained by inspeciting the stack. When converting an 134 + * existing ref database into a single reftable, this would be a range of 135 + * update-index timestamps. 136 + * 137 + * The function should be called before adding any records to the writer. If not 138 + * it will fail with REFTABLE_API_ERROR. 135 139 */ 136 - void reftable_writer_set_limits(struct reftable_writer *w, uint64_t min, 137 - uint64_t max); 140 + int reftable_writer_set_limits(struct reftable_writer *w, uint64_t min, 141 + uint64_t max); 138 142 139 143 /* 140 144 Add a reftable_ref_record. The record should have names that come after
+4 -2
reftable/stack.c
··· 1058 1058 1059 1059 for (size_t i = first; i <= last; i++) 1060 1060 st->stats.bytes += st->readers[i]->size; 1061 - reftable_writer_set_limits(wr, st->readers[first]->min_update_index, 1062 - st->readers[last]->max_update_index); 1061 + err = reftable_writer_set_limits(wr, st->readers[first]->min_update_index, 1062 + st->readers[last]->max_update_index); 1063 + if (err < 0) 1064 + goto done; 1063 1065 1064 1066 err = reftable_merged_table_new(&mt, st->readers + first, subtabs_len, 1065 1067 st->opts.hash_id);
+14 -1
reftable/writer.c
··· 179 179 return 0; 180 180 } 181 181 182 - void reftable_writer_set_limits(struct reftable_writer *w, uint64_t min, 182 + int reftable_writer_set_limits(struct reftable_writer *w, uint64_t min, 183 183 uint64_t max) 184 184 { 185 + /* 186 + * Set the min/max update index limits for the reftable writer. 187 + * This must be called before adding any records, since: 188 + * - The 'next' field gets set after writing the first block. 189 + * - The 'last_key' field updates with each new record (but resets 190 + * after sections). 191 + * Returns REFTABLE_API_ERROR if called after writing has begun. 192 + */ 193 + if (w->next || w->last_key.len) 194 + return REFTABLE_API_ERROR; 195 + 185 196 w->min_update_index = min; 186 197 w->max_update_index = max; 198 + 199 + return 0; 187 200 } 188 201 189 202 static void writer_release(struct reftable_writer *w)
+51 -3
t/unit-tests/t-reftable-stack.c
··· 103 103 static int write_test_ref(struct reftable_writer *wr, void *arg) 104 104 { 105 105 struct reftable_ref_record *ref = arg; 106 - reftable_writer_set_limits(wr, ref->update_index, ref->update_index); 106 + check(!reftable_writer_set_limits(wr, ref->update_index, 107 + ref->update_index)); 107 108 return reftable_writer_add_ref(wr, ref); 108 109 } 109 110 ··· 143 144 { 144 145 struct write_log_arg *wla = arg; 145 146 146 - reftable_writer_set_limits(wr, wla->update_index, wla->update_index); 147 + check(!reftable_writer_set_limits(wr, wla->update_index, 148 + wla->update_index)); 147 149 return reftable_writer_add_log(wr, wla->log); 148 150 } 149 151 ··· 961 963 962 964 static int write_nothing(struct reftable_writer *wr, void *arg UNUSED) 963 965 { 964 - reftable_writer_set_limits(wr, 1, 1); 966 + check(!reftable_writer_set_limits(wr, 1, 1)); 965 967 return 0; 966 968 } 967 969 ··· 1369 1371 clear_dir(dir); 1370 1372 } 1371 1373 1374 + static int write_limits_after_ref(struct reftable_writer *wr, void *arg) 1375 + { 1376 + struct reftable_ref_record *ref = arg; 1377 + check(!reftable_writer_set_limits(wr, ref->update_index, ref->update_index)); 1378 + check(!reftable_writer_add_ref(wr, ref)); 1379 + return reftable_writer_set_limits(wr, ref->update_index, ref->update_index); 1380 + } 1381 + 1382 + static void t_reftable_invalid_limit_updates(void) 1383 + { 1384 + struct reftable_ref_record ref = { 1385 + .refname = (char *) "HEAD", 1386 + .update_index = 1, 1387 + .value_type = REFTABLE_REF_SYMREF, 1388 + .value.symref = (char *) "master", 1389 + }; 1390 + struct reftable_write_options opts = { 1391 + .default_permissions = 0660, 1392 + }; 1393 + struct reftable_addition *add = NULL; 1394 + char *dir = get_tmp_dir(__LINE__); 1395 + struct reftable_stack *st = NULL; 1396 + int err; 1397 + 1398 + err = reftable_new_stack(&st, dir, &opts); 1399 + check(!err); 1400 + 1401 + reftable_addition_destroy(add); 1402 + 1403 + err = reftable_stack_new_addition(&add, st, 0); 1404 + check(!err); 1405 + 1406 + /* 1407 + * write_limits_after_ref also updates the update indexes after adding 1408 + * the record. This should cause an err to be returned, since the limits 1409 + * must be set at the start. 1410 + */ 1411 + err = reftable_addition_add(add, write_limits_after_ref, &ref); 1412 + check_int(err, ==, REFTABLE_API_ERROR); 1413 + 1414 + reftable_addition_destroy(add); 1415 + reftable_stack_destroy(st); 1416 + clear_dir(dir); 1417 + } 1418 + 1372 1419 int cmd_main(int argc UNUSED, const char *argv[] UNUSED) 1373 1420 { 1374 1421 TEST(t_empty_add(), "empty addition to stack"); 1375 1422 TEST(t_read_file(), "read_lines works"); 1376 1423 TEST(t_reflog_expire(), "expire reflog entries"); 1424 + TEST(t_reftable_invalid_limit_updates(), "prevent limit updates after adding records"); 1377 1425 TEST(t_reftable_stack_add(), "add multiple refs and logs to stack"); 1378 1426 TEST(t_reftable_stack_add_one(), "add a single ref record to stack"); 1379 1427 TEST(t_reftable_stack_add_performs_auto_compaction(), "addition to stack triggers auto-compaction");