reftable: remove name checks
In the preceding commit we have disabled name checks in the "reftable" backend. These checks were responsible for verifying multiple things when writing records to the reftable stack: - Detecting file/directory conflicts. Starting with the preceding commits this is now handled by the reftable backend itself via `refs_verify_refname_available()`. - Validating refnames. This is handled by `check_refname_format()` in the generic ref transacton layer. The code in the reftable library is thus not used anymore and likely to bitrot over time. Remove it. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
Apr 8, 2024 at 14:24 UTC
485c63cf5c8a325d2df14f2eeb22f1a01b55a11c
12 files changed
+1
-462
Makefile
-2
@@ -2655,7 +2655,6 @@ REFTABLE_OBJS += reftable/merged.o
2655
REFTABLE_OBJS += reftable/pq.o
2656
REFTABLE_OBJS += reftable/reader.o
2657
REFTABLE_OBJS += reftable/record.o
2658
-REFTABLE_OBJS += reftable/refname.o
2658
REFTABLE_OBJS += reftable/generic.o
2659
REFTABLE_OBJS += reftable/stack.o
2660
REFTABLE_OBJS += reftable/tree.o
@@ -2668,7 +2667,6 @@ REFTABLE_TEST_OBJS += reftable/merged_test.o
2667
REFTABLE_TEST_OBJS += reftable/pq_test.o
2668
REFTABLE_TEST_OBJS += reftable/record_test.o
2669
REFTABLE_TEST_OBJS += reftable/readwrite_test.o
2671
-REFTABLE_TEST_OBJS += reftable/refname_test.o
2670
REFTABLE_TEST_OBJS += reftable/stack_test.o
2671
REFTABLE_TEST_OBJS += reftable/test_framework.o
2672
REFTABLE_TEST_OBJS += reftable/tree_test.o
refs/reftable-backend.c
-5
@@ -247,11 +247,6 @@ static struct ref_store *reftable_be_init(struct repository *repo,
247
refs->write_options.block_size = 4096;
248
refs->write_options.hash_id = repo->hash_algo->format_id;
249
refs->write_options.default_permissions = calc_shared_perm(0666 & ~mask);
250
- /*
251
- * We verify names via `refs_verify_refname_available()`, so there is
252
- * no need to do the same checks in the reftable library again.
253
- */
254
- refs->write_options.skip_name_check = 1;
250
251
/*
252
* Set up the main reftable stack that is hosted in GIT_COMMON_DIR.
reftable/error.c
-2
@@ -27,8 +27,6 @@ const char *reftable_error_str(int err)
27
return "misuse of the reftable API";
28
case REFTABLE_ZLIB_ERROR:
29
return "zlib failure";
30
- case REFTABLE_NAME_CONFLICT:
31
- return "file/directory conflict";
30
case REFTABLE_EMPTY_TABLE_ERROR:
31
return "wrote empty table";
32
case REFTABLE_REFNAME_ERROR:
reftable/refname.c
deleted
-209
@@ -1,209 +0,0 @@
1
-/*
2
- Copyright 2020 Google LLC
3
-
4
- Use of this source code is governed by a BSD-style
5
- license that can be found in the LICENSE file or at
6
- https://developers.google.com/open-source/licenses/bsd
7
-*/
8
-
9
-#include "system.h"
10
-#include "reftable-error.h"
11
-#include "basics.h"
12
-#include "refname.h"
13
-#include "reftable-iterator.h"
14
-
15
-struct find_arg {
16
- char **names;
17
- const char *want;
18
-};
19
-
20
-static int find_name(size_t k, void *arg)
21
-{
22
- struct find_arg *f_arg = arg;
23
- return strcmp(f_arg->names[k], f_arg->want) >= 0;
24
-}
25
-
26
-static int modification_has_ref(struct modification *mod, const char *name)
27
-{
28
- struct reftable_ref_record ref = { NULL };
29
- int err = 0;
30
-
31
- if (mod->add_len > 0) {
32
- struct find_arg arg = {
33
- .names = mod->add,
34
- .want = name,
35
- };
36
- int idx = binsearch(mod->add_len, find_name, &arg);
37
- if (idx < mod->add_len && !strcmp(mod->add[idx], name)) {
38
- return 0;
39
- }
40
- }
41
-
42
- if (mod->del_len > 0) {
43
- struct find_arg arg = {
44
- .names = mod->del,
45
- .want = name,
46
- };
47
- int idx = binsearch(mod->del_len, find_name, &arg);
48
- if (idx < mod->del_len && !strcmp(mod->del[idx], name)) {
49
- return 1;
50
- }
51
- }
52
-
53
- err = reftable_table_read_ref(&mod->tab, name, &ref);
54
- reftable_ref_record_release(&ref);
55
- return err;
56
-}
57
-
58
-static void modification_release(struct modification *mod)
59
-{
60
- /* don't delete the strings themselves; they're owned by ref records.
61
- */
62
- FREE_AND_NULL(mod->add);
63
- FREE_AND_NULL(mod->del);
64
- mod->add_len = 0;
65
- mod->del_len = 0;
66
-}
67
-
68
-static int modification_has_ref_with_prefix(struct modification *mod,
69
- const char *prefix)
70
-{
71
- struct reftable_iterator it = { NULL };
72
- struct reftable_ref_record ref = { NULL };
73
- int err = 0;
74
-
75
- if (mod->add_len > 0) {
76
- struct find_arg arg = {
77
- .names = mod->add,
78
- .want = prefix,
79
- };
80
- int idx = binsearch(mod->add_len, find_name, &arg);
81
- if (idx < mod->add_len &&
82
- !strncmp(prefix, mod->add[idx], strlen(prefix)))
83
- goto done;
84
- }
85
- err = reftable_table_seek_ref(&mod->tab, &it, prefix);
86
- if (err)
87
- goto done;
88
-
89
- while (1) {
90
- err = reftable_iterator_next_ref(&it, &ref);
91
- if (err)
92
- goto done;
93
-
94
- if (mod->del_len > 0) {
95
- struct find_arg arg = {
96
- .names = mod->del,
97
- .want = ref.refname,
98
- };
99
- int idx = binsearch(mod->del_len, find_name, &arg);
100
- if (idx < mod->del_len &&
101
- !strcmp(ref.refname, mod->del[idx])) {
102
- continue;
103
- }
104
- }
105
-
106
- if (strncmp(ref.refname, prefix, strlen(prefix))) {
107
- err = 1;
108
- goto done;
109
- }
110
- err = 0;
111
- goto done;
112
- }
113
-
114
-done:
115
- reftable_ref_record_release(&ref);
116
- reftable_iterator_destroy(&it);
117
- return err;
118
-}
119
-
120
-static int validate_refname(const char *name)
121
-{
122
- while (1) {
123
- char *next = strchr(name, '/');
124
- if (!*name) {
125
- return REFTABLE_REFNAME_ERROR;
126
- }
127
- if (!next) {
128
- return 0;
129
- }
130
- if (next - name == 0 || (next - name == 1 && *name == '.') ||
131
- (next - name == 2 && name[0] == '.' && name[1] == '.'))
132
- return REFTABLE_REFNAME_ERROR;
133
- name = next + 1;
134
- }
135
- return 0;
136
-}
137
-
138
-int validate_ref_record_addition(struct reftable_table tab,
139
- struct reftable_ref_record *recs, size_t sz)
140
-{
141
- struct modification mod = {
142
- .tab = tab,
143
- .add = reftable_calloc(sz, sizeof(*mod.add)),
144
- .del = reftable_calloc(sz, sizeof(*mod.del)),
145
- };
146
- int i = 0;
147
- int err = 0;
148
- for (; i < sz; i++) {
149
- if (reftable_ref_record_is_deletion(&recs[i])) {
150
- mod.del[mod.del_len++] = recs[i].refname;
151
- } else {
152
- mod.add[mod.add_len++] = recs[i].refname;
153
- }
154
- }
155
-
156
- err = modification_validate(&mod);
157
- modification_release(&mod);
158
- return err;
159
-}
160
-
161
-static void strbuf_trim_component(struct strbuf *sl)
162
-{
163
- while (sl->len > 0) {
164
- int is_slash = (sl->buf[sl->len - 1] == '/');
165
- strbuf_setlen(sl, sl->len - 1);
166
- if (is_slash)
167
- break;
168
- }
169
-}
170
-
171
-int modification_validate(struct modification *mod)
172
-{
173
- struct strbuf slashed = STRBUF_INIT;
174
- int err = 0;
175
- int i = 0;
176
- for (; i < mod->add_len; i++) {
177
- err = validate_refname(mod->add[i]);
178
- if (err)
179
- goto done;
180
- strbuf_reset(&slashed);
181
- strbuf_addstr(&slashed, mod->add[i]);
182
- strbuf_addstr(&slashed, "/");
183
-
184
- err = modification_has_ref_with_prefix(mod, slashed.buf);
185
- if (err == 0) {
186
- err = REFTABLE_NAME_CONFLICT;
187
- goto done;
188
- }
189
- if (err < 0)
190
- goto done;
191
-
192
- strbuf_reset(&slashed);
193
- strbuf_addstr(&slashed, mod->add[i]);
194
- while (slashed.len) {
195
- strbuf_trim_component(&slashed);
196
- err = modification_has_ref(mod, slashed.buf);
197
- if (err == 0) {
198
- err = REFTABLE_NAME_CONFLICT;
199
- goto done;
200
- }
201
- if (err < 0)
202
- goto done;
203
- }
204
- }
205
- err = 0;
206
-done:
207
- strbuf_release(&slashed);
208
- return err;
209
-}
reftable/refname.h
deleted
-29
@@ -1,29 +0,0 @@
1
-/*
2
- Copyright 2020 Google LLC
3
-
4
- Use of this source code is governed by a BSD-style
5
- license that can be found in the LICENSE file or at
6
- https://developers.google.com/open-source/licenses/bsd
7
-*/
8
-#ifndef REFNAME_H
9
-#define REFNAME_H
10
-
11
-#include "reftable-record.h"
12
-#include "reftable-generic.h"
13
-
14
-struct modification {
15
- struct reftable_table tab;
16
-
17
- char **add;
18
- size_t add_len;
19
-
20
- char **del;
21
- size_t del_len;
22
-};
23
-
24
-int validate_ref_record_addition(struct reftable_table tab,
25
- struct reftable_ref_record *recs, size_t sz);
26
-
27
-int modification_validate(struct modification *mod);
28
-
29
-#endif
reftable/refname_test.c
deleted
-101
@@ -1,101 +0,0 @@
1
-/*
2
-Copyright 2020 Google LLC
3
-
4
-Use of this source code is governed by a BSD-style
5
-license that can be found in the LICENSE file or at
6
-https://developers.google.com/open-source/licenses/bsd
7
-*/
8
-
9
-#include "basics.h"
10
-#include "block.h"
11
-#include "blocksource.h"
12
-#include "reader.h"
13
-#include "record.h"
14
-#include "refname.h"
15
-#include "reftable-error.h"
16
-#include "reftable-writer.h"
17
-#include "system.h"
18
-
19
-#include "test_framework.h"
20
-#include "reftable-tests.h"
21
-
22
-struct testcase {
23
- char *add;
24
- char *del;
25
- int error_code;
26
-};
27
-
28
-static void test_conflict(void)
29
-{
30
- struct reftable_write_options opts = { 0 };
31
- struct strbuf buf = STRBUF_INIT;
32
- struct reftable_writer *w =
33
- reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);
34
- struct reftable_ref_record rec = {
35
- .refname = "a/b",
36
- .value_type = REFTABLE_REF_SYMREF,
37
- .value.symref = "destination", /* make sure it's not a symref.
38
- */
39
- .update_index = 1,
40
- };
41
- int err;
42
- int i;
43
- struct reftable_block_source source = { NULL };
44
- struct reftable_reader *rd = NULL;
45
- struct reftable_table tab = { NULL };
46
- struct testcase cases[] = {
47
- { "a/b/c", NULL, REFTABLE_NAME_CONFLICT },
48
- { "b", NULL, 0 },
49
- { "a", NULL, REFTABLE_NAME_CONFLICT },
50
- { "a", "a/b", 0 },
51
-
52
- { "p/", NULL, REFTABLE_REFNAME_ERROR },
53
- { "p//q", NULL, REFTABLE_REFNAME_ERROR },
54
- { "p/./q", NULL, REFTABLE_REFNAME_ERROR },
55
- { "p/../q", NULL, REFTABLE_REFNAME_ERROR },
56
-
57
- { "a/b/c", "a/b", 0 },
58
- { NULL, "a//b", 0 },
59
- };
60
- reftable_writer_set_limits(w, 1, 1);
61
-
62
- err = reftable_writer_add_ref(w, &rec);
63
- EXPECT_ERR(err);
64
-
65
- err = reftable_writer_close(w);
66
- EXPECT_ERR(err);
67
- reftable_writer_free(w);
68
-
69
- block_source_from_strbuf(&source, &buf);
70
- err = reftable_new_reader(&rd, &source, "filename");
71
- EXPECT_ERR(err);
72
-
73
- reftable_table_from_reader(&tab, rd);
74
-
75
- for (i = 0; i < ARRAY_SIZE(cases); i++) {
76
- struct modification mod = {
77
- .tab = tab,
78
- };
79
-
80
- if (cases[i].add) {
81
- mod.add = &cases[i].add;
82
- mod.add_len = 1;
83
- }
84
- if (cases[i].del) {
85
- mod.del = &cases[i].del;
86
- mod.del_len = 1;
87
- }
88
-
89
- err = modification_validate(&mod);
90
- EXPECT(err == cases[i].error_code);
91
- }
92
-
93
- reftable_reader_free(rd);
94
- strbuf_release(&buf);
95
-}
96
-
97
-int refname_test_main(int argc, const char *argv[])
98
-{
99
- RUN_TEST(test_conflict);
100
- return 0;
101
-}
reftable/reftable-error.h
-3
@@ -48,9 +48,6 @@ enum reftable_error {
48
/* Wrote a table without blocks. */
49
REFTABLE_EMPTY_TABLE_ERROR = -8,
50
51
- /* Dir/file conflict. */
52
- REFTABLE_NAME_CONFLICT = -9,
53
-
51
/* Invalid ref name. */
52
REFTABLE_REFNAME_ERROR = -10,
53
reftable/reftable-tests.h
-1
@@ -14,7 +14,6 @@ int block_test_main(int argc, const char **argv);
14
int merged_test_main(int argc, const char **argv);
15
int pq_test_main(int argc, const char **argv);
16
int record_test_main(int argc, const char **argv);
17
-int refname_test_main(int argc, const char **argv);
17
int readwrite_test_main(int argc, const char **argv);
18
int stack_test_main(int argc, const char **argv);
19
int tree_test_main(int argc, const char **argv);
reftable/reftable-writer.h
-4
@@ -38,10 +38,6 @@ struct reftable_write_options {
38
/* Default mode for creating files. If unset, use 0666 (+umask) */
39
unsigned int default_permissions;
40
41
- /* boolean: do not check ref names for validity or dir/file conflicts.
42
- */
43
- unsigned skip_name_check : 1;
44
-
41
/* boolean: copy log messages exactly. If unset, check that the message
42
* is a single line, and add '\n' if missing.
43
*/
reftable/stack.c
+1
-66
@@ -12,8 +12,8 @@ https://developers.google.com/open-source/licenses/bsd
12
#include "system.h"
13
#include "merged.h"
14
#include "reader.h"
15
-#include "refname.h"
15
#include "reftable-error.h"
16
+#include "reftable-generic.h"
17
#include "reftable-record.h"
18
#include "reftable-merged.h"
19
#include "writer.h"
@@ -27,8 +27,6 @@ static int stack_write_compact(struct reftable_stack *st,
27
struct reftable_writer *wr,
28
size_t first, size_t last,
29
struct reftable_log_expiry_config *config);
30
-static int stack_check_addition(struct reftable_stack *st,
31
- const char *new_tab_name);
30
static void reftable_addition_close(struct reftable_addition *add);
31
static int reftable_stack_reload_maybe_reuse(struct reftable_stack *st,
32
int reuse_open);
@@ -781,10 +779,6 @@ int reftable_addition_add(struct reftable_addition *add,
779
goto done;
780
}
781
784
- err = stack_check_addition(add->stack, get_tempfile_path(tab_file));
785
- if (err < 0)
786
- goto done;
787
-
782
if (wr->min_update_index < add->next_update_index) {
783
err = REFTABLE_API_ERROR;
784
goto done;
@@ -1340,65 +1334,6 @@ done:
1334
return err;
1335
}
1336
1343
-static int stack_check_addition(struct reftable_stack *st,
1344
- const char *new_tab_name)
1345
-{
1346
- int err = 0;
1347
- struct reftable_block_source src = { NULL };
1348
- struct reftable_reader *rd = NULL;
1349
- struct reftable_table tab = { NULL };
1350
- struct reftable_ref_record *refs = NULL;
1351
- struct reftable_iterator it = { NULL };
1352
- int cap = 0;
1353
- int len = 0;
1354
- int i = 0;
1355
-
1356
- if (st->config.skip_name_check)
1357
- return 0;
1358
-
1359
- err = reftable_block_source_from_file(&src, new_tab_name);
1360
- if (err < 0)
1361
- goto done;
1362
-
1363
- err = reftable_new_reader(&rd, &src, new_tab_name);
1364
- if (err < 0)
1365
- goto done;
1366
-
1367
- err = reftable_reader_seek_ref(rd, &it, "");
1368
- if (err > 0) {
1369
- err = 0;
1370
- goto done;
1371
- }
1372
- if (err < 0)
1373
- goto done;
1374
-
1375
- while (1) {
1376
- struct reftable_ref_record ref = { NULL };
1377
- err = reftable_iterator_next_ref(&it, &ref);
1378
- if (err > 0)
1379
- break;
1380
- if (err < 0)
1381
- goto done;
1382
-
1383
- REFTABLE_ALLOC_GROW(refs, len + 1, cap);
1384
- refs[len++] = ref;
1385
- }
1386
-
1387
- reftable_table_from_merged_table(&tab, reftable_stack_merged_table(st));
1388
-
1389
- err = validate_ref_record_addition(tab, refs, len);
1390
-
1391
-done:
1392
- for (i = 0; i < len; i++) {
1393
- reftable_ref_record_release(&refs[i]);
1394
- }
1395
-
1396
- free(refs);
1397
- reftable_iterator_destroy(&it);
1398
- reftable_reader_free(rd);
1399
- return err;
1400
-}
1401
-
1337
static int is_table_name(const char *s)
1338
{
1339
const char *dot = strrchr(s, '.');
reftable/stack_test.c
-39
@@ -353,44 +353,6 @@ static void test_reftable_stack_transaction_api_performs_auto_compaction(void)
353
clear_dir(dir);
354
}
355
356
-static void test_reftable_stack_validate_refname(void)
357
-{
358
- struct reftable_write_options cfg = { 0 };
359
- struct reftable_stack *st = NULL;
360
- int err;
361
- char *dir = get_tmp_dir(__LINE__);
362
-
363
- int i;
364
- struct reftable_ref_record ref = {
365
- .refname = "a/b",
366
- .update_index = 1,
367
- .value_type = REFTABLE_REF_SYMREF,
368
- .value.symref = "master",
369
- };
370
- char *additions[] = { "a", "a/b/c" };
371
-
372
- err = reftable_new_stack(&st, dir, cfg);
373
- EXPECT_ERR(err);
374
-
375
- err = reftable_stack_add(st, &write_test_ref, &ref);
376
- EXPECT_ERR(err);
377
-
378
- for (i = 0; i < ARRAY_SIZE(additions); i++) {
379
- struct reftable_ref_record ref = {
380
- .refname = additions[i],
381
- .update_index = 1,
382
- .value_type = REFTABLE_REF_SYMREF,
383
- .value.symref = "master",
384
- };
385
-
386
- err = reftable_stack_add(st, &write_test_ref, &ref);
387
- EXPECT(err == REFTABLE_NAME_CONFLICT);
388
- }
389
-
390
- reftable_stack_destroy(st);
391
- clear_dir(dir);
392
-}
393
-
356
static int write_error(struct reftable_writer *wr, void *arg)
357
{
358
return *((int *)arg);
@@ -1097,7 +1059,6 @@ int stack_test_main(int argc, const char *argv[])
1059
RUN_TEST(test_reftable_stack_transaction_api_performs_auto_compaction);
1060
RUN_TEST(test_reftable_stack_update_index_check);
1061
RUN_TEST(test_reftable_stack_uptodate);
1100
- RUN_TEST(test_reftable_stack_validate_refname);
1062
RUN_TEST(test_sizes_to_segments);
1063
RUN_TEST(test_sizes_to_segments_all_equal);
1064
RUN_TEST(test_sizes_to_segments_empty);
t/helper/test-reftable.c
-1
@@ -13,7 +13,6 @@ int cmd__reftable(int argc, const char **argv)
13
readwrite_test_main(argc, argv);
14
merged_test_main(argc, argv);
15
stack_test_main(argc, argv);
16
- refname_test_main(argc, argv);
16
return 0;
17
}
18