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@xxxxxx>
---
Makefile | 2 -
refs/reftable-backend.c | 5 -
reftable/error.c | 2 -
reftable/refname.c | 206 -------------------------------------
reftable/refname.h | 29 ------
reftable/refname_test.c | 101 ------------------
reftable/reftable-error.h | 3 -
reftable/reftable-tests.h | 1 -
reftable/reftable-writer.h | 4 -
reftable/stack.c | 67 +-----------
reftable/stack_test.c | 39 -------
t/helper/test-reftable.c | 1 -
12 files changed, 1 insertion(+), 459 deletions(-)
delete mode 100644 reftable/refname.c
delete mode 100644 reftable/refname.h
delete mode 100644 reftable/refname_test.c
diff --git a/Makefile b/Makefile
index c43c1bd1a0..05e3d37581 100644
--- a/Makefile
+++ b/Makefile
@@ -2655,7 +2655,6 @@ REFTABLE_OBJS += reftable/merged.o
REFTABLE_OBJS += reftable/pq.o
REFTABLE_OBJS += reftable/reader.o
REFTABLE_OBJS += reftable/record.o
-REFTABLE_OBJS += reftable/refname.o
REFTABLE_OBJS += reftable/generic.o
REFTABLE_OBJS += reftable/stack.o
REFTABLE_OBJS += reftable/tree.o
@@ -2668,7 +2667,6 @@ REFTABLE_TEST_OBJS += reftable/merged_test.o
REFTABLE_TEST_OBJS += reftable/pq_test.o
REFTABLE_TEST_OBJS += reftable/record_test.o
REFTABLE_TEST_OBJS += reftable/readwrite_test.o
-REFTABLE_TEST_OBJS += reftable/refname_test.o
REFTABLE_TEST_OBJS += reftable/stack_test.o
REFTABLE_TEST_OBJS += reftable/test_framework.o
REFTABLE_TEST_OBJS += reftable/tree_test.o
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 7515dd3019..8a54b0d8b2 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -247,11 +247,6 @@ static struct ref_store *reftable_be_init(struct repository *repo,
refs->write_options.block_size = 4096;
refs->write_options.hash_id = repo->hash_algo->format_id;
refs->write_options.default_permissions = calc_shared_perm(0666 & ~mask);
- /*
- * We verify names via `refs_verify_refname_available()`, so there is
- * no need to do the same checks in the reftable library again.
- */
- refs->write_options.skip_name_check = 1;
/*
* Set up the main reftable stack that is hosted in GIT_COMMON_DIR.
diff --git a/reftable/error.c b/reftable/error.c
index 0d1766735e..169f89d2f1 100644
--- a/reftable/error.c
+++ b/reftable/error.c
@@ -27,8 +27,6 @@ const char *reftable_error_str(int err)
return "misuse of the reftable API";
case REFTABLE_ZLIB_ERROR:
return "zlib failure";
- case REFTABLE_NAME_CONFLICT:
- return "file/directory conflict";
case REFTABLE_EMPTY_TABLE_ERROR:
return "wrote empty table";
case REFTABLE_REFNAME_ERROR:
diff --git a/reftable/refname.c b/reftable/refname.c
deleted file mode 100644
index bbfde15754..0000000000
--- a/reftable/refname.c
+++ /dev/null
@@ -1,206 +0,0 @@
-/*
- Copyright 2020 Google LLC
-
- Use of this source code is governed by a BSD-style
- license that can be found in the LICENSE file or at
- https://developers.google.com/open-source/licenses/bsd
-*/
-
-#include "system.h"
-#include "reftable-error.h"
-#include "basics.h"
-#include "refname.h"
-#include "reftable-iterator.h"
-
-struct refname_needle_lesseq_args {
- char **haystack;
- const char *needle;
-};
-
-static int refname_needle_lesseq(size_t k, void *_args)
-{
- struct refname_needle_lesseq_args *args = _args;
- return strcmp(args->needle, args->haystack[k]) <= 0;
-}
-
-static int modification_has_ref(struct modification *mod, const char *name)
-{
- struct reftable_ref_record ref = { NULL };
- int err = 0;
-
- if (mod->add_len > 0) {
- struct refname_needle_lesseq_args args = {
- .haystack = mod->add,
- .needle = name,
- };
- size_t idx = binsearch(mod->add_len, refname_needle_lesseq, &args);
- if (idx < mod->add_len && !strcmp(mod->add[idx], name))
- return 0;
- }
-
- if (mod->del_len > 0) {
- struct refname_needle_lesseq_args args = {
- .haystack = mod->del,
- .needle = name,
- };
- size_t idx = binsearch(mod->del_len, refname_needle_lesseq, &args);
- if (idx < mod->del_len && !strcmp(mod->del[idx], name))
- return 1;
- }
-
- err = reftable_table_read_ref(&mod->tab, name, &ref);
- reftable_ref_record_release(&ref);
- return err;
-}
-
-static void modification_release(struct modification *mod)
-{
- /* don't delete the strings themselves; they're owned by ref records.
- */
- FREE_AND_NULL(mod->add);
- FREE_AND_NULL(mod->del);
- mod->add_len = 0;
- mod->del_len = 0;
-}
-
-static int modification_has_ref_with_prefix(struct modification *mod,
- const char *prefix)
-{
- struct reftable_iterator it = { NULL };
- struct reftable_ref_record ref = { NULL };
- int err = 0;
-
- if (mod->add_len > 0) {
- struct refname_needle_lesseq_args args = {
- .haystack = mod->add,
- .needle = prefix,
- };
- size_t idx = binsearch(mod->add_len, refname_needle_lesseq, &args);
- if (idx < mod->add_len &&
- !strncmp(prefix, mod->add[idx], strlen(prefix)))
- goto done;
- }
- err = reftable_table_seek_ref(&mod->tab, &it, prefix);
- if (err)
- goto done;
-
- while (1) {
- err = reftable_iterator_next_ref(&it, &ref);
- if (err)
- goto done;
-
- if (mod->del_len > 0) {
- struct refname_needle_lesseq_args args = {
- .haystack = mod->del,
- .needle = ref.refname,
- };
- size_t idx = binsearch(mod->del_len, refname_needle_lesseq, &args);
- if (idx < mod->del_len &&
- !strcmp(ref.refname, mod->del[idx]))
- continue;
- }
-
- if (strncmp(ref.refname, prefix, strlen(prefix))) {
- err = 1;
- goto done;
- }
- err = 0;
- goto done;
- }
-
-done:
- reftable_ref_record_release(&ref);
- reftable_iterator_destroy(&it);
- return err;
-}
-
-static int validate_refname(const char *name)
-{
- while (1) {
- char *next = strchr(name, '/');
- if (!*name) {
- return REFTABLE_REFNAME_ERROR;
- }
- if (!next) {
- return 0;
- }
- if (next - name == 0 || (next - name == 1 && *name == '.') ||
- (next - name == 2 && name[0] == '.' && name[1] == '.'))
- return REFTABLE_REFNAME_ERROR;
- name = next + 1;
- }
- return 0;
-}
-
-int validate_ref_record_addition(struct reftable_table tab,
- struct reftable_ref_record *recs, size_t sz)
-{
- struct modification mod = {
- .tab = tab,
- .add = reftable_calloc(sz, sizeof(*mod.add)),
- .del = reftable_calloc(sz, sizeof(*mod.del)),
- };
- int i = 0;
- int err = 0;
- for (; i < sz; i++) {
- if (reftable_ref_record_is_deletion(&recs[i])) {
- mod.del[mod.del_len++] = recs[i].refname;
- } else {
- mod.add[mod.add_len++] = recs[i].refname;
- }
- }
-
- err = modification_validate(&mod);
- modification_release(&mod);
- return err;
-}
-
-static void strbuf_trim_component(struct strbuf *sl)
-{
- while (sl->len > 0) {
- int is_slash = (sl->buf[sl->len - 1] == '/');
- strbuf_setlen(sl, sl->len - 1);
- if (is_slash)
- break;
- }
-}
-
-int modification_validate(struct modification *mod)
-{
- struct strbuf slashed = STRBUF_INIT;
- int err = 0;
- int i = 0;
- for (; i < mod->add_len; i++) {
- err = validate_refname(mod->add[i]);
- if (err)
- goto done;
- strbuf_reset(&slashed);
- strbuf_addstr(&slashed, mod->add[i]);
- strbuf_addstr(&slashed, "/");
-
- err = modification_has_ref_with_prefix(mod, slashed.buf);
- if (err == 0) {
- err = REFTABLE_NAME_CONFLICT;
- goto done;
- }
- if (err < 0)
- goto done;
-
- strbuf_reset(&slashed);
- strbuf_addstr(&slashed, mod->add[i]);
- while (slashed.len) {
- strbuf_trim_component(&slashed);
- err = modification_has_ref(mod, slashed.buf);
- if (err == 0) {
- err = REFTABLE_NAME_CONFLICT;
- goto done;
- }
- if (err < 0)
- goto done;
- }
- }
- err = 0;
-done:
- strbuf_release(&slashed);
- return err;
-}
diff --git a/reftable/refname.h b/reftable/refname.h
deleted file mode 100644
index a24b40fcb4..0000000000
--- a/reftable/refname.h
+++ /dev/null
@@ -1,29 +0,0 @@
-/*
- Copyright 2020 Google LLC
-
- Use of this source code is governed by a BSD-style
- license that can be found in the LICENSE file or at
- https://developers.google.com/open-source/licenses/bsd
-*/
-#ifndef REFNAME_H
-#define REFNAME_H
-
-#include "reftable-record.h"
-#include "reftable-generic.h"
-
-struct modification {
- struct reftable_table tab;
-
- char **add;
- size_t add_len;
-
- char **del;
- size_t del_len;
-};
-
-int validate_ref_record_addition(struct reftable_table tab,
- struct reftable_ref_record *recs, size_t sz);
-
-int modification_validate(struct modification *mod);
-
-#endif
diff --git a/reftable/refname_test.c b/reftable/refname_test.c
deleted file mode 100644
index b9cc62554e..0000000000
--- a/reftable/refname_test.c
+++ /dev/null
@@ -1,101 +0,0 @@
-/*
-Copyright 2020 Google LLC
-
-Use of this source code is governed by a BSD-style
-license that can be found in the LICENSE file or at
-https://developers.google.com/open-source/licenses/bsd
-*/
-
-#include "basics.h"
-#include "block.h"
-#include "blocksource.h"
-#include "reader.h"
-#include "record.h"
-#include "refname.h"
-#include "reftable-error.h"
-#include "reftable-writer.h"
-#include "system.h"
-
-#include "test_framework.h"
-#include "reftable-tests.h"
-
-struct testcase {
- char *add;
- char *del;
- int error_code;
-};
-
-static void test_conflict(void)
-{
- struct reftable_write_options opts = { 0 };
- struct strbuf buf = STRBUF_INIT;
- struct reftable_writer *w =
- reftable_new_writer(&strbuf_add_void, &noop_flush, &buf, &opts);
- struct reftable_ref_record rec = {
- .refname = "a/b",
- .value_type = REFTABLE_REF_SYMREF,
- .value.symref = "destination", /* make sure it's not a symref.
- */
- .update_index = 1,
- };
- int err;
- int i;
- struct reftable_block_source source = { NULL };
- struct reftable_reader *rd = NULL;
- struct reftable_table tab = { NULL };
- struct testcase cases[] = {
- { "a/b/c", NULL, REFTABLE_NAME_CONFLICT },
- { "b", NULL, 0 },
- { "a", NULL, REFTABLE_NAME_CONFLICT },
- { "a", "a/b", 0 },
-
- { "p/", NULL, REFTABLE_REFNAME_ERROR },
- { "p//q", NULL, REFTABLE_REFNAME_ERROR },
- { "p/./q", NULL, REFTABLE_REFNAME_ERROR },
- { "p/../q", NULL, REFTABLE_REFNAME_ERROR },
-
- { "a/b/c", "a/b", 0 },
- { NULL, "a//b", 0 },
- };
- reftable_writer_set_limits(w, 1, 1);
-
- err = reftable_writer_add_ref(w, &rec);
- EXPECT_ERR(err);
-
- err = reftable_writer_close(w);
- EXPECT_ERR(err);
- reftable_writer_free(w);
-
- block_source_from_strbuf(&source, &buf);
- err = reftable_new_reader(&rd, &source, "filename");
- EXPECT_ERR(err);
-
- reftable_table_from_reader(&tab, rd);
-
- for (i = 0; i < ARRAY_SIZE(cases); i++) {
- struct modification mod = {
- .tab = tab,
- };
-
- if (cases[i].add) {
- mod.add = &cases[i].add;
- mod.add_len = 1;
- }
- if (cases[i].del) {
- mod.del = &cases[i].del;
- mod.del_len = 1;
- }
-
- err = modification_validate(&mod);
- EXPECT(err == cases[i].error_code);
- }
-
- reftable_reader_free(rd);
- strbuf_release(&buf);
-}
-
-int refname_test_main(int argc, const char *argv[])
-{
- RUN_TEST(test_conflict);
- return 0;
-}
diff --git a/reftable/reftable-error.h b/reftable/reftable-error.h
index 4c457aaaf8..3a5f5b92c6 100644
--- a/reftable/reftable-error.h
+++ b/reftable/reftable-error.h
@@ -48,9 +48,6 @@ enum reftable_error {
/* Wrote a table without blocks. */
REFTABLE_EMPTY_TABLE_ERROR = -8,
- /* Dir/file conflict. */
- REFTABLE_NAME_CONFLICT = -9,
-
/* Invalid ref name. */
REFTABLE_REFNAME_ERROR = -10,
diff --git a/reftable/reftable-tests.h b/reftable/reftable-tests.h
index 0019cbcfa4..114cc3d053 100644
--- a/reftable/reftable-tests.h
+++ b/reftable/reftable-tests.h
@@ -14,7 +14,6 @@ int block_test_main(int argc, const char **argv);
int merged_test_main(int argc, const char **argv);
int pq_test_main(int argc, const char **argv);
int record_test_main(int argc, const char **argv);
-int refname_test_main(int argc, const char **argv);
int readwrite_test_main(int argc, const char **argv);
int stack_test_main(int argc, const char **argv);
int tree_test_main(int argc, const char **argv);
diff --git a/reftable/reftable-writer.h b/reftable/reftable-writer.h
index 7c7cae5f99..3c119e2bbb 100644
--- a/reftable/reftable-writer.h
+++ b/reftable/reftable-writer.h
@@ -38,10 +38,6 @@ struct reftable_write_options {
/* Default mode for creating files. If unset, use 0666 (+umask) */
unsigned int default_permissions;
- /* boolean: do not check ref names for validity or dir/file conflicts.
- */
- unsigned skip_name_check : 1;
-
/* boolean: copy log messages exactly. If unset, check that the message
* is a single line, and add '\n' if missing.
*/
diff --git a/reftable/stack.c b/reftable/stack.c
index 1ecf1b9751..e264df5ced 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -12,8 +12,8 @@ license that can be found in the LICENSE file or at
#include "system.h"
#include "merged.h"
#include "reader.h"
-#include "refname.h"
#include "reftable-error.h"
+#include "reftable-generic.h"
#include "reftable-record.h"
#include "reftable-merged.h"
#include "writer.h"
@@ -27,8 +27,6 @@ static int stack_write_compact(struct reftable_stack *st,
struct reftable_writer *wr,
size_t first, size_t last,
struct reftable_log_expiry_config *config);
-static int stack_check_addition(struct reftable_stack *st,
- const char *new_tab_name);
static void reftable_addition_close(struct reftable_addition *add);
static int reftable_stack_reload_maybe_reuse(struct reftable_stack *st,
int reuse_open);
@@ -781,10 +779,6 @@ int reftable_addition_add(struct reftable_addition *add,
goto done;
}
- err = stack_check_addition(add->stack, get_tempfile_path(tab_file));
- if (err < 0)
- goto done;
-
if (wr->min_update_index < add->next_update_index) {
err = REFTABLE_API_ERROR;
goto done;
@@ -1340,65 +1334,6 @@ int reftable_stack_read_log(struct reftable_stack *st, const char *refname,
return err;
}
-static int stack_check_addition(struct reftable_stack *st,
- const char *new_tab_name)
-{
- int err = 0;
- struct reftable_block_source src = { NULL };
- struct reftable_reader *rd = NULL;
- struct reftable_table tab = { NULL };
- struct reftable_ref_record *refs = NULL;
- struct reftable_iterator it = { NULL };
- int cap = 0;
- int len = 0;
- int i = 0;
-
- if (st->config.skip_name_check)
- return 0;
-
- err = reftable_block_source_from_file(&src, new_tab_name);
- if (err < 0)
- goto done;
-
- err = reftable_new_reader(&rd, &src, new_tab_name);
- if (err < 0)
- goto done;
-
- err = reftable_reader_seek_ref(rd, &it, "");
- if (err > 0) {
- err = 0;
- goto done;
- }
- if (err < 0)
- goto done;
-
- while (1) {
- struct reftable_ref_record ref = { NULL };
- err = reftable_iterator_next_ref(&it, &ref);
- if (err > 0)
- break;
- if (err < 0)
- goto done;
-
- REFTABLE_ALLOC_GROW(refs, len + 1, cap);
- refs[len++] = ref;
- }
-
- reftable_table_from_merged_table(&tab, reftable_stack_merged_table(st));
-
- err = validate_ref_record_addition(tab, refs, len);
-
-done:
- for (i = 0; i < len; i++) {
- reftable_ref_record_release(&refs[i]);
- }
-
- free(refs);
- reftable_iterator_destroy(&it);
- reftable_reader_free(rd);
- return err;
-}
-
static int is_table_name(const char *s)
{
const char *dot = strrchr(s, '.');
diff --git a/reftable/stack_test.c b/reftable/stack_test.c
index 0dc9a44648..b88097c3b6 100644
--- a/reftable/stack_test.c
+++ b/reftable/stack_test.c
@@ -353,44 +353,6 @@ static void test_reftable_stack_transaction_api_performs_auto_compaction(void)
clear_dir(dir);
}
-static void test_reftable_stack_validate_refname(void)
-{
- struct reftable_write_options cfg = { 0 };
- struct reftable_stack *st = NULL;
- int err;
- char *dir = get_tmp_dir(__LINE__);
-
- int i;
- struct reftable_ref_record ref = {
- .refname = "a/b",
- .update_index = 1,
- .value_type = REFTABLE_REF_SYMREF,
- .value.symref = "master",
- };
- char *additions[] = { "a", "a/b/c" };
-
- err = reftable_new_stack(&st, dir, cfg);
- EXPECT_ERR(err);
-
- err = reftable_stack_add(st, &write_test_ref, &ref);
- EXPECT_ERR(err);
-
- for (i = 0; i < ARRAY_SIZE(additions); i++) {
- struct reftable_ref_record ref = {
- .refname = additions[i],
- .update_index = 1,
- .value_type = REFTABLE_REF_SYMREF,
- .value.symref = "master",
- };
-
- err = reftable_stack_add(st, &write_test_ref, &ref);
- EXPECT(err == REFTABLE_NAME_CONFLICT);
- }
-
- reftable_stack_destroy(st);
- clear_dir(dir);
-}
-
static int write_error(struct reftable_writer *wr, void *arg)
{
return *((int *)arg);
@@ -1097,7 +1059,6 @@ int stack_test_main(int argc, const char *argv[])
RUN_TEST(test_reftable_stack_transaction_api_performs_auto_compaction);
RUN_TEST(test_reftable_stack_update_index_check);
RUN_TEST(test_reftable_stack_uptodate);
- RUN_TEST(test_reftable_stack_validate_refname);
RUN_TEST(test_sizes_to_segments);
RUN_TEST(test_sizes_to_segments_all_equal);
RUN_TEST(test_sizes_to_segments_empty);
diff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c
index 00237ef0d9..bae731669c 100644
--- a/t/helper/test-reftable.c
+++ b/t/helper/test-reftable.c
@@ -13,7 +13,6 @@ int cmd__reftable(int argc, const char **argv)
readwrite_test_main(argc, argv);
merged_test_main(argc, argv);
stack_test_main(argc, argv);
- refname_test_main(argc, argv);
return 0;
}
--
2.44.GIT
Attachment:
signature.asc
Description: PGP signature
