Merge branch 'ps/reftable-write-optim'

Code to write out reftable has seen some optimization and
simplification.

* ps/reftable-write-optim:
  reftable/block: reuse compressed array
  reftable/block: reuse zstream when writing log blocks
  reftable/writer: reset `last_key` instead of releasing it
  reftable/writer: unify releasing memory
  reftable/writer: refactorings for `writer_flush_nonempty_block()`
  reftable/writer: refactorings for `writer_add_record()`
  refs/reftable: don't recompute committer ident
  reftable: remove name checks
  refs/reftable: skip duplicate name checks
  refs/reftable: perform explicit D/F check when writing symrefs
  refs/reftable: fix D/F conflict error message on ref copy
This commit is contained in:
Junio C Hamano 2024-05-08 10:18:43 -07:00
commit 5aec7231c8
16 changed files with 226 additions and 552 deletions

View file

@ -2665,7 +2665,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
@ -2678,7 +2677,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

View file

@ -172,32 +172,30 @@ static int should_write_log(struct ref_store *refs, const char *refname)
}
}
static void fill_reftable_log_record(struct reftable_log_record *log)
static void fill_reftable_log_record(struct reftable_log_record *log, const struct ident_split *split)
{
const char *info = git_committer_info(0);
struct ident_split split = {0};
const char *tz_begin;
int sign = 1;
if (split_ident_line(&split, info, strlen(info)))
BUG("failed splitting committer info");
reftable_log_record_release(log);
log->value_type = REFTABLE_LOG_UPDATE;
log->value.update.name =
xstrndup(split.name_begin, split.name_end - split.name_begin);
xstrndup(split->name_begin, split->name_end - split->name_begin);
log->value.update.email =
xstrndup(split.mail_begin, split.mail_end - split.mail_begin);
log->value.update.time = atol(split.date_begin);
if (*split.tz_begin == '-') {
xstrndup(split->mail_begin, split->mail_end - split->mail_begin);
log->value.update.time = atol(split->date_begin);
tz_begin = split->tz_begin;
if (*tz_begin == '-') {
sign = -1;
split.tz_begin++;
tz_begin++;
}
if (*split.tz_begin == '+') {
if (*tz_begin == '+') {
sign = 1;
split.tz_begin++;
tz_begin++;
}
log->value.update.tz_offset = sign * atoi(split.tz_begin);
log->value.update.tz_offset = sign * atoi(tz_begin);
}
static int read_ref_without_reload(struct reftable_stack *stack,
@ -1021,9 +1019,15 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data
reftable_stack_merged_table(arg->stack);
uint64_t ts = reftable_stack_next_update_index(arg->stack);
struct reftable_log_record *logs = NULL;
struct ident_split committer_ident = {0};
size_t logs_nr = 0, logs_alloc = 0, i;
const char *committer_info;
int ret = 0;
committer_info = git_committer_info(0);
if (split_ident_line(&committer_ident, committer_info, strlen(committer_info)))
BUG("failed splitting committer info");
QSORT(arg->updates, arg->updates_nr, transaction_update_cmp);
reftable_writer_set_limits(writer, ts, ts);
@ -1089,7 +1093,7 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data
log = &logs[logs_nr++];
memset(log, 0, sizeof(*log));
fill_reftable_log_record(log);
fill_reftable_log_record(log, &committer_ident);
log->update_index = ts;
log->refname = xstrdup(u->refname);
memcpy(log->value.update.new_hash, u->new_oid.hash, GIT_MAX_RAWSZ);
@ -1227,6 +1231,7 @@ static int reftable_be_pack_refs(struct ref_store *ref_store,
struct write_create_symref_arg {
struct reftable_ref_store *refs;
struct reftable_stack *stack;
struct strbuf *err;
const char *refname;
const char *target;
const char *logmsg;
@ -1242,13 +1247,20 @@ static int write_create_symref_table(struct reftable_writer *writer, void *cb_da
.value.symref = (char *)create->target,
.update_index = ts,
};
struct ident_split committer_ident = {0};
struct reftable_log_record log = {0};
struct object_id new_oid;
struct object_id old_oid;
const char *committer_info;
int ret;
reftable_writer_set_limits(writer, ts, ts);
ret = refs_verify_refname_available(&create->refs->base, create->refname,
NULL, NULL, create->err);
if (ret < 0)
return ret;
ret = reftable_writer_add_ref(writer, &ref);
if (ret)
return ret;
@ -1267,7 +1279,11 @@ static int write_create_symref_table(struct reftable_writer *writer, void *cb_da
!should_write_log(&create->refs->base, create->refname))
return 0;
fill_reftable_log_record(&log);
committer_info = git_committer_info(0);
if (split_ident_line(&committer_ident, committer_info, strlen(committer_info)))
BUG("failed splitting committer info");
fill_reftable_log_record(&log, &committer_ident);
log.refname = xstrdup(create->refname);
log.update_index = ts;
log.value.update.message = xstrndup(create->logmsg,
@ -1290,12 +1306,14 @@ static int reftable_be_create_symref(struct ref_store *ref_store,
struct reftable_ref_store *refs =
reftable_be_downcast(ref_store, REF_STORE_WRITE, "create_symref");
struct reftable_stack *stack = stack_for(refs, refname, &refname);
struct strbuf err = STRBUF_INIT;
struct write_create_symref_arg arg = {
.refs = refs,
.stack = stack,
.refname = refname,
.target = target,
.logmsg = logmsg,
.err = &err,
};
int ret;
@ -1311,9 +1329,15 @@ static int reftable_be_create_symref(struct ref_store *ref_store,
done:
assert(ret != REFTABLE_API_ERROR);
if (ret)
error("unable to write symref for %s: %s", refname,
reftable_error_str(ret));
if (ret) {
if (err.len)
error("%s", err.buf);
else
error("unable to write symref for %s: %s", refname,
reftable_error_str(ret));
}
strbuf_release(&err);
return ret;
}
@ -1335,10 +1359,16 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
struct reftable_log_record old_log = {0}, *logs = NULL;
struct reftable_iterator it = {0};
struct string_list skip = STRING_LIST_INIT_NODUP;
struct ident_split committer_ident = {0};
struct strbuf errbuf = STRBUF_INIT;
size_t logs_nr = 0, logs_alloc = 0, i;
const char *committer_info;
int ret;
committer_info = git_committer_info(0);
if (split_ident_line(&committer_ident, committer_info, strlen(committer_info)))
BUG("failed splitting committer info");
if (reftable_stack_read_ref(arg->stack, arg->oldname, &old_ref)) {
ret = error(_("refname %s not found"), arg->oldname);
goto done;
@ -1361,7 +1391,8 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
/*
* Verify that the new refname is available.
*/
string_list_insert(&skip, arg->oldname);
if (arg->delete_old)
string_list_insert(&skip, arg->oldname);
ret = refs_verify_refname_available(&arg->refs->base, arg->newname,
NULL, &skip, &errbuf);
if (ret < 0) {
@ -1412,7 +1443,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
memset(&logs[logs_nr], 0, sizeof(logs[logs_nr]));
fill_reftable_log_record(&logs[logs_nr]);
fill_reftable_log_record(&logs[logs_nr], &committer_ident);
logs[logs_nr].refname = (char *)arg->newname;
logs[logs_nr].update_index = deletion_ts;
logs[logs_nr].value.update.message =
@ -1444,7 +1475,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
*/
ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
memset(&logs[logs_nr], 0, sizeof(logs[logs_nr]));
fill_reftable_log_record(&logs[logs_nr]);
fill_reftable_log_record(&logs[logs_nr], &committer_ident);
logs[logs_nr].refname = (char *)arg->newname;
logs[logs_nr].update_index = creation_ts;
logs[logs_nr].value.update.message =

View file

@ -76,6 +76,10 @@ void block_writer_init(struct block_writer *bw, uint8_t typ, uint8_t *buf,
bw->entries = 0;
bw->restart_len = 0;
bw->last_key.len = 0;
if (!bw->zstream) {
REFTABLE_CALLOC_ARRAY(bw->zstream, 1);
deflateInit(bw->zstream, 9);
}
}
uint8_t block_writer_type(struct block_writer *bw)
@ -139,39 +143,52 @@ int block_writer_finish(struct block_writer *w)
w->next += 2;
put_be24(w->buf + 1 + w->header_off, w->next);
/*
* Log records are stored zlib-compressed. Note that the compression
* also spans over the restart points we have just written.
*/
if (block_writer_type(w) == BLOCK_TYPE_LOG) {
int block_header_skip = 4 + w->header_off;
uLongf src_len = w->next - block_header_skip;
uLongf dest_cap = src_len * 1.001 + 12;
uint8_t *compressed;
uLongf src_len = w->next - block_header_skip, compressed_len;
int ret;
REFTABLE_ALLOC_ARRAY(compressed, dest_cap);
ret = deflateReset(w->zstream);
if (ret != Z_OK)
return REFTABLE_ZLIB_ERROR;
while (1) {
uLongf out_dest_len = dest_cap;
int zresult = compress2(compressed, &out_dest_len,
w->buf + block_header_skip,
src_len, 9);
if (zresult == Z_BUF_ERROR && dest_cap < LONG_MAX) {
dest_cap *= 2;
compressed =
reftable_realloc(compressed, dest_cap);
if (compressed)
continue;
}
/*
* Precompute the upper bound of how many bytes the compressed
* data may end up with. Combined with `Z_FINISH`, `deflate()`
* is guaranteed to return `Z_STREAM_END`.
*/
compressed_len = deflateBound(w->zstream, src_len);
REFTABLE_ALLOC_GROW(w->compressed, compressed_len, w->compressed_cap);
if (Z_OK != zresult) {
reftable_free(compressed);
return REFTABLE_ZLIB_ERROR;
}
w->zstream->next_out = w->compressed;
w->zstream->avail_out = compressed_len;
w->zstream->next_in = w->buf + block_header_skip;
w->zstream->avail_in = src_len;
memcpy(w->buf + block_header_skip, compressed,
out_dest_len);
w->next = out_dest_len + block_header_skip;
reftable_free(compressed);
break;
}
/*
* We want to perform all decompression in a single step, which
* is why we can pass Z_FINISH here. As we have precomputed the
* deflated buffer's size via `deflateBound()` this function is
* guaranteed to succeed according to the zlib documentation.
*/
ret = deflate(w->zstream, Z_FINISH);
if (ret != Z_STREAM_END)
return REFTABLE_ZLIB_ERROR;
/*
* Overwrite the uncompressed data we have already written and
* adjust the `next` pointer to point right after the
* compressed data.
*/
memcpy(w->buf + block_header_skip, w->compressed,
w->zstream->total_out);
w->next = w->zstream->total_out + block_header_skip;
}
return w->next;
}
@ -514,7 +531,10 @@ int block_iter_seek_key(struct block_iter *it, const struct block_reader *br,
void block_writer_release(struct block_writer *bw)
{
deflateEnd(bw->zstream);
FREE_AND_NULL(bw->zstream);
FREE_AND_NULL(bw->restarts);
FREE_AND_NULL(bw->compressed);
strbuf_release(&bw->last_key);
/* the block is not owned. */
}

View file

@ -18,6 +18,10 @@ license that can be found in the LICENSE file or at
* allocation overhead.
*/
struct block_writer {
z_stream *zstream;
unsigned char *compressed;
size_t compressed_cap;
uint8_t *buf;
uint32_t block_size;

View file

@ -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:

View file

@ -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;
}

View file

@ -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

View file

@ -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;
}

View file

@ -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,

View file

@ -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);

View file

@ -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.
*/

View file

@ -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);
@ -787,10 +785,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;
@ -1355,65 +1349,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, '.');

View file

@ -396,44 +396,6 @@ static void test_reftable_stack_auto_compaction_fails_gracefully(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);
@ -1105,7 +1067,6 @@ int stack_test_main(int argc, const char *argv[])
RUN_TEST(test_reftable_stack_auto_compaction_fails_gracefully);
RUN_TEST(test_reftable_stack_update_index_check);
RUN_TEST(test_reftable_stack_uptodate);
RUN_TEST(test_reftable_stack_validate_refname);
RUN_TEST(test_suggest_compaction_segment);
RUN_TEST(test_suggest_compaction_segment_nothing);
return 0;

View file

@ -109,7 +109,7 @@ static void writer_reinit_block_writer(struct reftable_writer *w, uint8_t typ)
block_start = header_size(writer_version(w));
}
strbuf_release(&w->last_key);
strbuf_reset(&w->last_key);
block_writer_init(&w->block_writer_data, typ, w->block,
w->opts.block_size, block_start,
hash_size(w->opts.hash_id));
@ -149,11 +149,21 @@ void reftable_writer_set_limits(struct reftable_writer *w, uint64_t min,
w->max_update_index = max;
}
static void writer_release(struct reftable_writer *w)
{
if (w) {
reftable_free(w->block);
w->block = NULL;
block_writer_release(&w->block_writer_data);
w->block_writer = NULL;
writer_clear_index(w);
strbuf_release(&w->last_key);
}
}
void reftable_writer_free(struct reftable_writer *w)
{
if (!w)
return;
reftable_free(w->block);
writer_release(w);
reftable_free(w);
}
@ -209,7 +219,8 @@ static int writer_add_record(struct reftable_writer *w,
struct reftable_record *rec)
{
struct strbuf key = STRBUF_INIT;
int err = -1;
int err;
reftable_record_key(rec, &key);
if (strbuf_cmp(&w->last_key, &key) >= 0) {
err = REFTABLE_API_ERROR;
@ -218,27 +229,42 @@ static int writer_add_record(struct reftable_writer *w,
strbuf_reset(&w->last_key);
strbuf_addbuf(&w->last_key, &key);
if (!w->block_writer) {
if (!w->block_writer)
writer_reinit_block_writer(w, reftable_record_type(rec));
}
assert(block_writer_type(w->block_writer) == reftable_record_type(rec));
if (block_writer_type(w->block_writer) != reftable_record_type(rec))
BUG("record of type %d added to writer of type %d",
reftable_record_type(rec), block_writer_type(w->block_writer));
if (block_writer_add(w->block_writer, rec) == 0) {
/*
* Try to add the record to the writer. If this succeeds then we're
* done. Otherwise the block writer may have hit the block size limit
* and needs to be flushed.
*/
if (!block_writer_add(w->block_writer, rec)) {
err = 0;
goto done;
}
/*
* The current block is full, so we need to flush and reinitialize the
* writer to start writing the next block.
*/
err = writer_flush_block(w);
if (err < 0) {
if (err < 0)
goto done;
}
writer_reinit_block_writer(w, reftable_record_type(rec));
/*
* Try to add the record to the writer again. If this still fails then
* the record does not fit into the block size.
*
* TODO: it would be great to have `block_writer_add()` return proper
* error codes so that we don't have to second-guess the failure
* mode here.
*/
err = block_writer_add(w->block_writer, rec);
if (err == -1) {
/* we are writing into memory, so an error can only mean it
* doesn't fit. */
if (err) {
err = REFTABLE_ENTRY_TOO_BIG_ERROR;
goto done;
}
@ -452,7 +478,7 @@ static int writer_finish_section(struct reftable_writer *w)
bstats->max_index_level = max_level;
/* Reinit lastKey, as the next section can start with any key. */
w->last_key.len = 0;
strbuf_reset(&w->last_key);
return 0;
}
@ -627,74 +653,87 @@ int reftable_writer_close(struct reftable_writer *w)
}
done:
/* free up memory. */
block_writer_release(&w->block_writer_data);
writer_clear_index(w);
strbuf_release(&w->last_key);
writer_release(w);
return err;
}
static void writer_clear_index(struct reftable_writer *w)
{
for (size_t i = 0; i < w->index_len; i++)
for (size_t i = 0; w->index && i < w->index_len; i++)
strbuf_release(&w->index[i].last_key);
FREE_AND_NULL(w->index);
w->index_len = 0;
w->index_cap = 0;
}
static const int debug = 0;
static int writer_flush_nonempty_block(struct reftable_writer *w)
{
struct reftable_index_record index_record = {
.last_key = STRBUF_INIT,
};
uint8_t typ = block_writer_type(w->block_writer);
struct reftable_block_stats *bstats =
writer_reftable_block_stats(w, typ);
uint64_t block_typ_off = (bstats->blocks == 0) ? w->next : 0;
int raw_bytes = block_writer_finish(w->block_writer);
int padding = 0;
int err = 0;
struct reftable_index_record ir = { .last_key = STRBUF_INIT };
struct reftable_block_stats *bstats;
int raw_bytes, padding = 0, err;
uint64_t block_typ_off;
/*
* Finish the current block. This will cause the block writer to emit
* restart points and potentially compress records in case we are
* writing a log block.
*
* Note that this is still happening in memory.
*/
raw_bytes = block_writer_finish(w->block_writer);
if (raw_bytes < 0)
return raw_bytes;
if (!w->opts.unpadded && typ != BLOCK_TYPE_LOG) {
/*
* By default, all records except for log records are padded to the
* block size.
*/
if (!w->opts.unpadded && typ != BLOCK_TYPE_LOG)
padding = w->opts.block_size - raw_bytes;
}
if (block_typ_off > 0) {
bstats = writer_reftable_block_stats(w, typ);
block_typ_off = (bstats->blocks == 0) ? w->next : 0;
if (block_typ_off > 0)
bstats->offset = block_typ_off;
}
bstats->entries += w->block_writer->entries;
bstats->restarts += w->block_writer->restart_len;
bstats->blocks++;
w->stats.blocks++;
if (debug) {
fprintf(stderr, "block %c off %" PRIu64 " sz %d (%d)\n", typ,
w->next, raw_bytes,
get_be24(w->block + w->block_writer->header_off + 1));
}
if (w->next == 0) {
/*
* If this is the first block we're writing to the table then we need
* to also write the reftable header.
*/
if (!w->next)
writer_write_header(w, w->block);
}
err = padded_write(w, w->block, raw_bytes, padding);
if (err < 0)
return err;
/*
* Add an index record for every block that we're writing. If we end up
* having more than a threshold of index records we will end up writing
* an index section in `writer_finish_section()`. Each index record
* contains the last record key of the block it is indexing as well as
* the offset of that block.
*
* Note that this also applies when flushing index blocks, in which
* case we will end up with a multi-level index.
*/
REFTABLE_ALLOC_GROW(w->index, w->index_len + 1, w->index_cap);
ir.offset = w->next;
strbuf_reset(&ir.last_key);
strbuf_addbuf(&ir.last_key, &w->block_writer->last_key);
w->index[w->index_len] = ir;
index_record.offset = w->next;
strbuf_reset(&index_record.last_key);
strbuf_addbuf(&index_record.last_key, &w->block_writer->last_key);
w->index[w->index_len] = index_record;
w->index_len++;
w->next += padding + raw_bytes;
w->block_writer = NULL;
return 0;
}

View file

@ -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;
}

View file

@ -286,7 +286,7 @@ test_expect_success 'ref transaction: creating symbolic ref fails with F/D confl
git init repo &&
test_commit -C repo A &&
cat >expect <<-EOF &&
error: unable to write symref for refs/heads: file/directory conflict
error: ${SQ}refs/heads/main${SQ} exists; cannot create ${SQ}refs/heads${SQ}
EOF
test_must_fail git -C repo symbolic-ref refs/heads refs/heads/foo 2>err &&
test_cmp expect err
@ -854,6 +854,39 @@ test_expect_success 'reflog: updates via HEAD update HEAD reflog' '
)
'
test_expect_success 'branch: copying branch with D/F conflict' '
test_when_finished "rm -rf repo" &&
git init repo &&
(
cd repo &&
test_commit A &&
git branch branch &&
cat >expect <<-EOF &&
error: ${SQ}refs/heads/branch${SQ} exists; cannot create ${SQ}refs/heads/branch/moved${SQ}
fatal: branch copy failed
EOF
test_must_fail git branch -c branch branch/moved 2>err &&
test_cmp expect err
)
'
test_expect_success 'branch: moving branch with D/F conflict' '
test_when_finished "rm -rf repo" &&
git init repo &&
(
cd repo &&
test_commit A &&
git branch branch &&
git branch conflict &&
cat >expect <<-EOF &&
error: ${SQ}refs/heads/conflict${SQ} exists; cannot create ${SQ}refs/heads/conflict/moved${SQ}
fatal: branch rename failed
EOF
test_must_fail git branch -m branch conflict/moved 2>err &&
test_cmp expect err
)
'
test_expect_success 'worktree: adding worktree creates separate stack' '
test_when_finished "rm -rf repo worktree" &&
git init repo &&