reftable: stop using Git subsystems
cover reftable: stop using Git subsystems
Hi,
this patch series is another step towards detangling the reftable library from libgit.a. This step removes all uses of Git subsystems like "hash.h", "tempfile.h" and "lockfile.h" in favor of either providing our own infra for this ("hash.h") or by implementing compatibility shims that bridge between Git and the reftable library ("tempfile.h", "lockfile.h").
This is the second-last step in the overall effort to detangle those two codebases from each other. The last step will be to split out POSIX bits from "git-compat-util.h" into a separate "compat/posix.h" header that we then use from the reftable library such that we don't pull in any of the Git-specific parts into it.
This series is built on top of fd3785337b (The third batch, 2024-10-22) with ps/reftable-strbuf at 20590cd287 (reftable: handle trivial reftable_buf errors, 2024-10-17) merged into it.
Thanks!
Patrick
Patrick Steinhardt (7):
reftable/system: move "dir.h" to its only user
reftable: explicitly handle hash format IDs reftable/system: stop depending on "hash.h" reftable/stack: stop using fsync_component() directly reftable/system: provide thin wrapper for tempfile subsystem reftable/stack: drop only use of get_locked_file_path() reftable/system: provide thin wrapper for lockfile subsystem
Makefile | 1 + refs/reftable-backend.c | 19 +++- reftable/basics.c | 13 ++- reftable/basics.h | 9 +- reftable/merged.c | 4 +- reftable/merged.h | 3 +- reftable/reader.c | 14 ++- reftable/reader.h | 4 +- reftable/reftable-basics.h | 13 +++ reftable/reftable-merged.h | 4 +- reftable/reftable-reader.h | 2 +- reftable/reftable-record.h | 12 +- reftable/reftable-writer.h | 8 +- reftable/stack.c | 171 ++++++++++++++-------------- reftable/system.c | 126 ++++++++++++++++++++ reftable/system.h | 88 +++++++++++++- reftable/writer.c | 20 +++- t/helper/test-reftable.c | 4 +- t/unit-tests/lib-reftable.c | 5 +- t/unit-tests/lib-reftable.h | 2 +- t/unit-tests/t-reftable-block.c | 41 +++---- t/unit-tests/t-reftable-merged.c | 26 ++--- t/unit-tests/t-reftable-pq.c | 3 +- t/unit-tests/t-reftable-reader.c | 4 +- t/unit-tests/t-reftable-readwrite.c | 41 +++---- t/unit-tests/t-reftable-record.c | 59 +++++----- t/unit-tests/t-reftable-stack.c | 37 +++--- 27 files changed, 504 insertions(+), 229 deletions(-) create mode 100644 reftable/system.c
Signature · 1 line
2.47.0.118.gfd3785337b.dirty
PATCH 1/7 reftable/system: move "dir.h" to its only user
We still include "dir.h" in "reftable/system.h" evne though it is not used by anything but by a single unit test. Move it over into that unit test so that we don't accidentally use any functionality provided by it in the reftable codebase.
--- reftable/system.h | 1 - t/unit-tests/t-reftable-stack.c | 1 + 2 files changed, 1 insertion(+), 1 deletion(-)
reftable/system.h +0 -1
@@ -15,7 +15,6 @@ license that can be found in the LICENSE file or at#include "lockfile.h" #include "tempfile.h" #include "hash.h" /* hash ID, sizes.*/ -#include "dir.h" /* remove_dir_recursively, for tests.*/ int hash_size (uint32_t id);
t/unit-tests/t-reftable-stack.c +1 -0
@@ -8,6 +8,7 @@ license that can be found in the LICENSE file or at#include "test-lib.h" #include "lib-reftable.h" +#include "dir.h" #include "reftable/merged.h" #include "reftable/reader.h" #include "reftable/reftable-error.h"
Signature · 1 line
2.47.0.118.gfd3785337b.dirty
Re: [PATCH 1/7] reftable/system: move "dir.h" to its only user
Quoted 4 lines · Patrick Steinhardt
We still include "
dir.h" in "reftable/system.h" evne though it is not used by anything but by a single unit test. Move it over into that unit test so that we don't accidentally use any functionality provided by it in the reftable codebase.
s/evne/even/
Quoted 1 line
Signed-off-by Patrick Steinhardt <[email protected]>
PATCH 2/7 reftable: explicitly handle hash format IDs
In reply to Patrick SteinhardtThe hash format IDs are used for two different things across the reftable codebase:
- They are used as a 32 bit unsigned integer when reading and writing the header in order to identify the hash function.
- They are used internally to identify which hash function is in use.
When one only considers the second usecase one might think that one can easily change the representation of those hash IDs. But because those IDs end up in the reftable header and footer on disk it is important that those never change.
Create separate constants REFTABLE_FORMAT_ID_* and use them in contexts where we read or write reftable headers. This serves multiple purposes:
- It allows us to more easily discern cases where we actually use those constants for the on-disk format.
- It detangles us from the same constants that are defined in libgit.a, which is another required step to convert the reftable library to become standalone.
- It makes the next step easier where we stop using GIT_*_FORMAT_ID constants in favor of a custom enum.
--- reftable/basics.h | 7 +++++++ reftable/reader.c | 10 ++++++---- reftable/writer.c | 16 +++++++++++++++- 3 files changed, 28 insertions(+), 5 deletions(-)
reftable/basics.h +7 -0
@@ -150,4 +150,11 @@ int common_prefix_size(struct reftable_buf *a, struct reftable_buf *b);int hash_size (uint32_t id); +/* +* Format IDs that identify the hash function used by a reftable. Note that +* these constants end up on disk and thus mustn't change. +*/ +#define REFTABLE_FORMAT_ID_SHA1 ((uint32_t) 0x73686131) +#define REFTABLE_FORMAT_ID_SHA256 ((uint32_t) 0x73323536) +#endif
reftable/reader.c +6 -4
@@ -109,16 +109,18 @@ static int parse_footer(struct reftable_reader *r, uint8_t *footer,if (r->version ==1 ) { r->hash_id =GIT_SHA1_FORMAT_ID ; }else { - r->hash_id =get_be32 (f); -switch (r->hash_id) { -case GIT_SHA1_FORMAT_ID : +switch (get_be32 (f)) { +case REFTABLE_FORMAT_ID_SHA1 : + r->hash_id =GIT_SHA1_FORMAT_ID ;break ; -case GIT_SHA256_FORMAT_ID : +case REFTABLE_FORMAT_ID_SHA256 : + r->hash_id =GIT_SHA256_FORMAT_ID ;break ;default : err =REFTABLE_FORMAT_ERROR ; goto done; } + f +=4 ; }
reftable/writer.c +15 -1
@@ -103,8 +103,22 @@ static int writer_write_header(struct reftable_writer *w, uint8_t *dest)put_be64 (dest +8 , w->min_update_index);put_be64 (dest +16 , w->max_update_index);if (writer_version (w) ==2 ) { -put_be32 (dest +24 , w->opts.hash_id); +uint32_t hash_id; + +switch (w->opts.hash_id) { +case GIT_SHA1_FORMAT_ID : + hash_id =REFTABLE_FORMAT_ID_SHA1 ; +break ; +case GIT_SHA256_FORMAT_ID : + hash_id =REFTABLE_FORMAT_ID_SHA256 ; +break ; +default : +return -1 ; + } + +put_be32 (dest +24 , hash_id); } +return header_size (writer_version (w)); }
Signature · 1 line
2.47.0.118.gfd3785337b.dirty
Re: [PATCH 2/7] reftable: explicitly handle hash format IDs
Quoted 5 lines · Patrick Steinhardt
reftable/basics.h +0
-0
reftable/basics.h int common_prefix_size(struct reftable_buf *a, struct reftable_buf *b); · line 158
Quoted 9 lines
reftable/basics.h +6
-0@@ -150,4 +150,11 @@ int common_prefix_size(struct reftable_buf *a, struct reftable_buf *b);int hash_size (uint32_t id); +/* +* Format IDs that identify the hash function used by a reftable. Note that +* these constants end up on disk and thus mustn't change. +*/ +#define REFTABLE_FORMAT_ID_SHA1 ((uint32_t) 0x73686131) +#define REFTABLE_FORMAT_ID_SHA256 ((uint32_t) 0x73323536)
This context is provided in the comments for GIT_*_FORMAT_ID, but now that they being decoupled it might be nice to mention here that they stand for "sha1" and "s256" respectively.
Quoted 15 lines
reftable/basics.h +1
-0+#endif reftable/reader.c +2
-3@@ -109,16 +109,18 @@ static int parse_footer(struct reftable_reader *r, uint8_t *footer,if (r->version ==1 ) { r->hash_id =GIT_SHA1_FORMAT_ID ; }else { - r->hash_id =get_be32 (f); -switch (r->hash_id) { -case GIT_SHA1_FORMAT_ID : +switch (get_be32 (f)) { +case REFTABLE_FORMAT_ID_SHA1 :
reftable/reader.c static int parse_footer(struct reftable_reader *r, uint8_t *footer, · line 122
Quoted 10 lines
reftable/reader.c +3
-1+ r->hash_id =GIT_SHA1_FORMAT_ID ;break ; -case GIT_SHA256_FORMAT_ID : +case REFTABLE_FORMAT_ID_SHA256 : + r->hash_id =GIT_SHA256_FORMAT_ID ;break ;default : err =REFTABLE_FORMAT_ERROR ; goto done; }
Instead of directly mapping hash_id to GIT_*_FORMAT_ID, we use the newly defined constants to map to the corresponding format. As an internal identifier, the format ID can now be freely changed without risk of affecting how on-disk reftable headers are parsed. Makes sense to me and seems like a good change.
-Justin
PATCH 3/7 reftable/system: stop depending on "hash.h"
In reply to Patrick SteinhardtWe include "hash.h" in "reftable/system.h" such that we can use hash format IDs as well as the raw size of SHA1 and SHA256. As we are in the process of converting the reftable library to become standalone we of course cannot rely on those constants anymore.
Introduce a new enum reftable_hash to replace internal uses of the hash format IDs and new constants that replace internal uses of the hash size. Adapt the reftable backend to set up the correct hash function.
--- refs/reftable-backend.c | 12 +++++- reftable/basics.c | 13 ++++--- reftable/basics.h | 2 +- reftable/merged.c | 4 +- reftable/merged.h | 3 +- reftable/reader.c | 8 ++-- reftable/reader.h | 4 +- reftable/reftable-basics.h | 13 +++++++ reftable/reftable-merged.h | 4 +- reftable/reftable-reader.h | 2 +- reftable/reftable-record.h | 12 +++--- reftable/reftable-writer.h | 2 +- reftable/stack.c | 4 +- reftable/system.h | 3 -- reftable/writer.c | 8 ++-- t/helper/test-reftable.c | 4 +- t/unit-tests/lib-reftable.c | 4 +- t/unit-tests/lib-reftable.h | 2 +- t/unit-tests/t-reftable-block.c | 40 +++++++++---------- t/unit-tests/t-reftable-merged.c | 26 ++++++------- t/unit-tests/t-reftable-pq.c | 2 +- t/unit-tests/t-reftable-reader.c | 4 +- t/unit-tests/t-reftable-readwrite.c | 40 +++++++++---------- t/unit-tests/t-reftable-record.c | 59 +++++++++++++++-------------- t/unit-tests/t-reftable-stack.c | 34 ++++++++--------- 25 files changed, 166 insertions(+), 143 deletions(-)
refs/reftable-backend.c +11 -1
@@ -15,6 +15,7 @@#include "../object.h" #include "../path.h" #include "../refs.h" +#include "../reftable/reftable-basics.h" #include "../reftable/reftable-stack.h" #include "../reftable/reftable-record.h" #include "../reftable/reftable-error.h"
@@ -289,7 +290,16 @@ static struct ref_store *reftable_be_init(struct repository *repo, refs->store_flags = store_flags; refs->log_all_ref_updates =repo_settings_get_log_all_ref_updates (repo); - refs->write_options.hash_id = repo->hash_algo->format_id; +switch (repo->hash_algo->format_id) { +case GIT_SHA1_FORMAT_ID : + refs->write_options.hash_id =REFTABLE_HASH_SHA1 ; +break ; +case GIT_SHA256_FORMAT_ID : + refs->write_options.hash_id =REFTABLE_HASH_SHA256 ; +break ; +default : +BUG ("unknown hash algorithm %d" , repo->hash_algo->format_id); + } refs->write_options.default_permissions =calc_shared_perm (0666 & ~mask); refs->write_options.disable_auto_compact = !git_env_bool ("GIT_TEST_REFTABLE_AUTOCOMPACTION" ,1 );
reftable/basics.c +7 -6
@@ -271,14 +271,15 @@ int common_prefix_size(struct reftable_buf *a, struct reftable_buf *b)return p; } -int hash_size (uint32_t id) +int hash_size (enum reftable_hash id) { +if (!id) +return REFTABLE_HASH_SIZE_SHA1 ;switch (id) { -case 0 : -case GIT_SHA1_FORMAT_ID : -return GIT_SHA1_RAWSZ ; -case GIT_SHA256_FORMAT_ID : -return GIT_SHA256_RAWSZ ; +case REFTABLE_HASH_SHA1 : +return REFTABLE_HASH_SIZE_SHA1 ; +case REFTABLE_HASH_SHA256 : +return REFTABLE_HASH_SIZE_SHA256 ; }abort (); }
reftable/basics.h +1 -1
@@ -148,7 +148,7 @@ char *reftable_strdup(const char *str);/* Find the longest shared prefix size of `a` and `b` */ int common_prefix_size (struct reftable_buf *a,struct reftable_buf *b); -int hash_size (uint32_t id); +int hash_size (enum reftable_hash id);/* * Format IDs that identify the hash function used by a reftable. Note that
reftable/merged.c +2 -2
@@ -181,7 +181,7 @@ static void iterator_from_merged_iter(struct reftable_iterator *it,int reftable_merged_table_new (struct reftable_merged_table **dest,struct reftable_reader **readers,size_t n, -uint32_t hash_id) +enum reftable_hash hash_id) {struct reftable_merged_table *m =NULL ;uint64_t last_max =0 ;
@@ -293,7 +293,7 @@ int reftable_merged_table_init_log_iterator(struct reftable_merged_table *mt,return merged_table_init_iter (mt, it,BLOCK_TYPE_LOG ); } -uint32_t reftable_merged_table_hash_id (struct reftable_merged_table *mt) +enum reftable_hash reftable_merged_table_hash_id (struct reftable_merged_table *mt) {return mt->hash_id; }
reftable/merged.h +2 -1
@@ -10,11 +10,12 @@ license that can be found in the LICENSE file or at#define MERGED_H #include "system.h" +#include "basics.h" struct reftable_merged_table {struct reftable_reader **readers;size_t readers_len; -uint32_t hash_id; +enum reftable_hash hash_id;/* If unset, produce deletions. This is useful for compaction. For the * full stack, deletions should be produced. */
reftable/reader.c +4 -4
@@ -67,7 +67,7 @@ static int reader_get_block(struct reftable_reader *r,return block_source_read_block (&r->source, dest, off, sz); } -uint32_t reftable_reader_hash_id (struct reftable_reader *r) +enum reftable_hash reftable_reader_hash_id (struct reftable_reader *r) {return r->hash_id; }
@@ -107,14 +107,14 @@ static int parse_footer(struct reftable_reader *r, uint8_t *footer, f +=8 ;if (r->version ==1 ) { - r->hash_id =GIT_SHA1_FORMAT_ID ; + r->hash_id =REFTABLE_HASH_SHA1 ; }else {switch (get_be32 (f)) {case REFTABLE_FORMAT_ID_SHA1 : - r->hash_id =GIT_SHA1_FORMAT_ID ; + r->hash_id =REFTABLE_HASH_SHA1 ;break ;case REFTABLE_FORMAT_ID_SHA256 : - r->hash_id =GIT_SHA256_FORMAT_ID ; + r->hash_id =REFTABLE_HASH_SHA256 ;break ;default : err =REFTABLE_FORMAT_ERROR ;
reftable/reader.h +2 -2
@@ -37,8 +37,8 @@ struct reftable_reader {/* Size of the file, excluding the footer. */ uint64_t size; -/* 'sha1' for SHA1, 's256' for SHA-256 */ -uint32_t hash_id; +/* The hash function used for ref records. */ +enum reftable_hash hash_id;uint32_t block_size;uint64_t min_update_index;
reftable/reftable-basics.h +13 -0
@@ -11,6 +11,19 @@#include <stddef.h> +/* +* Hash functions understood by the reftable library. Note that the values are +* arbitrary and somewhat random such that we can easily detect cases where the +* hash hasn't been properly set up. +*/ +enum reftable_hash { +REFTABLE_HASH_SHA1 =89 , +REFTABLE_HASH_SHA256 =247 , +}; +#define REFTABLE_HASH_SIZE_SHA1 20 +#define REFTABLE_HASH_SIZE_SHA256 32 +#define REFTABLE_HASH_SIZE_MAX REFTABLE_HASH_SIZE_SHA256 +/* Overrides the functions to use for memory management. */ void reftable_set_alloc (void *(*malloc)(size_t ),void *(*realloc)(void *,size_t ),void (*free)(void *));
reftable/reftable-merged.h +2 -2
@@ -34,7 +34,7 @@ struct reftable_reader;*/ int reftable_merged_table_new (struct reftable_merged_table **dest,struct reftable_reader **readers,size_t n, -uint32_t hash_id); +enum reftable_hash hash_id);/* Initialize a merged table iterator for reading refs. */ int reftable_merged_table_init_ref_iterator(struct reftable_merged_table *mt,
@@ -56,6 +56,6 @@ reftable_merged_table_min_update_index(struct reftable_merged_table *mt);void reftable_merged_table_free (struct reftable_merged_table *m);/* return the hash ID of the merged table. */ -uint32_t reftable_merged_table_hash_id (struct reftable_merged_table *m); +enum reftable_hash reftable_merged_table_hash_id (struct reftable_merged_table *m);#endif
reftable/reftable-reader.h +1 -1
@@ -54,7 +54,7 @@ int reftable_reader_init_log_iterator(struct reftable_reader *r, struct reftable_iterator *it);/* returns the hash ID used in this table. */ -uint32_t reftable_reader_hash_id (struct reftable_reader *r); +enum reftable_hash reftable_reader_hash_id (struct reftable_reader *r);/* return an iterator for the refs pointing to `oid`. */ int reftable_reader_refs_for(struct reftable_reader *r,
reftable/reftable-record.h +6 -6
@@ -9,7 +9,7 @@ license that can be found in the LICENSE file or at#ifndef REFTABLE_RECORD_H #define REFTABLE_RECORD_H -#include "hash.h" +#include "reftable-basics.h" #include <stdint.h> /*
@@ -40,10 +40,10 @@ struct reftable_ref_record {#define REFTABLE_NR_REF_VALUETYPES 4 } value_type;union { -unsigned char val1[GIT_MAX_RAWSZ ]; +unsigned char val1[REFTABLE_HASH_SIZE_MAX ];struct { -unsigned char value[GIT_MAX_RAWSZ ];/* first hash */ -unsigned char target_value[GIT_MAX_RAWSZ ];/* second hash */ +unsigned char value[REFTABLE_HASH_SIZE_MAX ];/* first hash */ +unsigned char target_value[REFTABLE_HASH_SIZE_MAX ];/* second hash */ } val2;char *symref;/* referent, malloced 0-terminated string */ } value;
@@ -85,8 +85,8 @@ struct reftable_log_record {union {struct { -unsigned char new_hash[GIT_MAX_RAWSZ ]; -unsigned char old_hash[GIT_MAX_RAWSZ ]; +unsigned char new_hash[REFTABLE_HASH_SIZE_MAX ]; +unsigned char old_hash[REFTABLE_HASH_SIZE_MAX ];char *name;char *email;uint64_t time;
reftable/reftable-writer.h +1 -1
@@ -33,7 +33,7 @@ struct reftable_write_options {/* 4-byte identifier ("sha1", "s256") of the hash. * Defaults to SHA1 if unset */ -uint32_t hash_id; +enum reftable_hash hash_id;/* Default mode for creating files. If unset, use 0666 (+umask) */ unsigned int default_permissions;
reftable/stack.c +2 -2
@@ -73,7 +73,7 @@ int reftable_new_stack(struct reftable_stack **dest, const char *dir,if (_opts) opts = *_opts;if (opts.hash_id ==0 ) - opts.hash_id =GIT_SHA1_FORMAT_ID ; + opts.hash_id =REFTABLE_HASH_SHA1 ; *dest =NULL ;
@@ -1603,7 +1603,7 @@ struct segment suggest_compaction_segment(uint64_t *sizes, size_t n,static uint64_t *stack_table_sizes_for_compaction (struct reftable_stack *st) { -int version = (st->opts.hash_id ==GIT_SHA1_FORMAT_ID ) ?1 :2 ; +int version = (st->opts.hash_id ==REFTABLE_HASH_SHA1 ) ?1 :2 ;int overhead =header_size (version) -1 ;uint64_t *sizes;
reftable/system.h +0 -3
@@ -14,8 +14,5 @@ license that can be found in the LICENSE file or at#include "git-compat-util.h" #include "lockfile.h" #include "tempfile.h" -#include "hash.h" /* hash ID, sizes.*/ - -int hash_size (uint32_t id);#endif
reftable/writer.c +4 -4
@@ -79,7 +79,7 @@ static void options_set_defaults(struct reftable_write_options *opts) }if (opts->hash_id ==0 ) { - opts->hash_id =GIT_SHA1_FORMAT_ID ; + opts->hash_id =REFTABLE_HASH_SHA1 ; }if (opts->block_size ==0 ) { opts->block_size =DEFAULT_BLOCK_SIZE ;
@@ -88,7 +88,7 @@ static void options_set_defaults(struct reftable_write_options *opts)static int writer_version (struct reftable_writer *w) { -return (w->opts.hash_id ==0 || w->opts.hash_id ==GIT_SHA1_FORMAT_ID ) ? +return (w->opts.hash_id ==0 || w->opts.hash_id ==REFTABLE_HASH_SHA1 ) ?1 :2 ; }
@@ -106,10 +106,10 @@ static int writer_write_header(struct reftable_writer *w, uint8_t *dest)uint32_t hash_id;switch (w->opts.hash_id) { -case GIT_SHA1_FORMAT_ID : +case REFTABLE_HASH_SHA1 : hash_id =REFTABLE_FORMAT_ID_SHA1 ;break ; -case GIT_SHA256_FORMAT_ID : +case REFTABLE_HASH_SHA256 : hash_id =REFTABLE_FORMAT_ID_SHA256 ;break ;default :
t/helper/test-reftable.c +2 -2
@@ -156,7 +156,7 @@ int cmd__dump_reftable(int argc, const char **argv)int opt_dump_blocks =0 ;int opt_dump_table =0 ;int opt_dump_stack =0 ; -uint32_t opt_hash_id =GIT_SHA1_FORMAT_ID ; +uint32_t opt_hash_id =REFTABLE_HASH_SHA1 ;const char *arg =NULL , *argv0 = argv[0 ];for (; argc >1 ; argv++, argc--)
@@ -167,7 +167,7 @@ int cmd__dump_reftable(int argc, const char **argv)else if (!strcmp ("-t" , argv[1 ])) opt_dump_table =1 ;else if (!strcmp ("-6" , argv[1 ])) - opt_hash_id =GIT_SHA256_FORMAT_ID ; + opt_hash_id =REFTABLE_HASH_SHA256 ;else if (!strcmp ("-s" , argv[1 ])) opt_dump_stack =1 ;else if (!strcmp ("-?" , argv[1 ]) || !strcmp ("-h" , argv[1 ])) {
t/unit-tests/lib-reftable.c +2 -2
@@ -3,7 +3,7 @@#include "reftable/constants.h" #include "reftable/writer.h" -void t_reftable_set_hash (uint8_t *p,int i,uint32_t id) +void t_reftable_set_hash (uint8_t *p,int i,enum reftable_hash id) {memset (p, (uint8_t )i,hash_size (id)); }
@@ -82,7 +82,7 @@ void t_reftable_write_to_buf(struct reftable_buf *buf,size_t off = i * (opts.block_size ? opts.block_size :DEFAULT_BLOCK_SIZE );if (!off) - off =header_size (opts.hash_id ==GIT_SHA256_FORMAT_ID ?2 :1 ); + off =header_size (opts.hash_id ==REFTABLE_HASH_SHA256 ?2 :1 ); check_char(buf->buf[off], ==, 'r'); }
t/unit-tests/lib-reftable.h +1 -1
@@ -6,7 +6,7 @@struct reftable_buf ; -void t_reftable_set_hash (uint8_t *p,int i,uint32_t id); +void t_reftable_set_hash (uint8_t *p,int i,enum reftable_hash id);struct reftable_writer *t_reftable_strbuf_writer (struct reftable_buf *buf,struct reftable_write_options *opts);
t/unit-tests/t-reftable-block.c +20 -20
@@ -36,7 +36,7 @@ static void t_ref_block_read_write(void) block.len = block_size;block_source_from_buf (&block.source ,&buf); ret =block_writer_init (&bw,BLOCK_TYPE_REF , block.data, block_size, - header_off,hash_size (GIT_SHA1_FORMAT_ID )); + header_off,hash_size (REFTABLE_HASH_SHA1 ));check (!ret); rec.u.ref.refname = (char *)"" ;
@@ -47,7 +47,7 @@ static void t_ref_block_read_write(void)for (i =0 ; i <N ; i++) { rec.u.ref.refname =xstrfmt ("branch%02" PRIuMAX, (uintmax_t )i); rec.u.ref.value_type =REFTABLE_REF_VAL1 ; -memset (rec.u.ref.value.val1, i,GIT_SHA1_RAWSZ ); +memset (rec.u.ref.value.val1, i,REFTABLE_HASH_SIZE_SHA1 ); recs[i] = rec; ret =block_writer_add (&bw, &rec);
@@ -61,7 +61,7 @@ static void t_ref_block_read_write(void)block_writer_release (&bw); -block_reader_init (&br, &block, header_off, block_size,GIT_SHA1_RAWSZ ); +block_reader_init (&br, &block, header_off, block_size,REFTABLE_HASH_SIZE_SHA1 );block_iter_seek_start (&it, &br);
@@ -72,7 +72,7 @@ static void t_ref_block_read_write(void) check_int(i, ==, N);break ; } -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); }for (i =0 ; i <N ; i++) {
@@ -85,7 +85,7 @@ static void t_ref_block_read_write(void) ret =block_iter_next (&it, &rec); check_int(ret, ==, 0); -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); want.len--; ret =block_iter_seek_key (&it, &br, &want);
@@ -93,7 +93,7 @@ static void t_ref_block_read_write(void) ret =block_iter_next (&it, &rec); check_int(ret, ==, 0); -check (reftable_record_equal (&recs[10 * (i /10 )], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[10 * (i /10 )], &rec,REFTABLE_HASH_SIZE_SHA1 )); }block_reader_release (&br);
@@ -130,7 +130,7 @@ static void t_log_block_read_write(void) block.len = block_size;block_source_from_buf (&block.source ,&buf); ret =block_writer_init (&bw,BLOCK_TYPE_LOG , block.data, block_size, - header_off,hash_size (GIT_SHA1_FORMAT_ID )); + header_off,hash_size (REFTABLE_HASH_SHA1 ));check (!ret);for (i =0 ; i <N ; i++) {
@@ -150,7 +150,7 @@ static void t_log_block_read_write(void)block_writer_release (&bw); -block_reader_init (&br, &block, header_off, block_size,GIT_SHA1_RAWSZ ); +block_reader_init (&br, &block, header_off, block_size,REFTABLE_HASH_SIZE_SHA1 );block_iter_seek_start (&it, &br);
@@ -161,7 +161,7 @@ static void t_log_block_read_write(void) check_int(i, ==, N);break ; } -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); }for (i =0 ; i <N ; i++) {
@@ -175,7 +175,7 @@ static void t_log_block_read_write(void) ret =block_iter_next (&it, &rec); check_int(ret, ==, 0); -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); want.len--; ret =block_iter_seek_key (&it, &br, &want);
@@ -183,7 +183,7 @@ static void t_log_block_read_write(void) ret =block_iter_next (&it, &rec); check_int(ret, ==, 0); -check (reftable_record_equal (&recs[10 * (i /10 )], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[10 * (i /10 )], &rec,REFTABLE_HASH_SIZE_SHA1 )); }block_reader_release (&br);
@@ -220,7 +220,7 @@ static void t_obj_block_read_write(void) block.len = block_size;block_source_from_buf (&block.source, &buf); ret =block_writer_init (&bw,BLOCK_TYPE_OBJ , block.data, block_size, - header_off,hash_size (GIT_SHA1_FORMAT_ID )); + header_off,hash_size (REFTABLE_HASH_SHA1 ));check (!ret);for (i =0 ; i <N ; i++) {
@@ -242,7 +242,7 @@ static void t_obj_block_read_write(void)block_writer_release (&bw); -block_reader_init (&br, &block, header_off, block_size,GIT_SHA1_RAWSZ ); +block_reader_init (&br, &block, header_off, block_size,REFTABLE_HASH_SIZE_SHA1 );block_iter_seek_start (&it, &br);
@@ -253,7 +253,7 @@ static void t_obj_block_read_write(void) check_int(i, ==, N);break ; } -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); }for (i =0 ; i <N ; i++) {
@@ -266,7 +266,7 @@ static void t_obj_block_read_write(void) ret =block_iter_next (&it, &rec); check_int(ret, ==, 0); -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); }block_reader_release (&br);
@@ -304,7 +304,7 @@ static void t_index_block_read_write(void) block.len = block_size;block_source_from_buf (&block.source, &buf); ret =block_writer_init (&bw,BLOCK_TYPE_INDEX , block.data, block_size, - header_off,hash_size (GIT_SHA1_FORMAT_ID )); + header_off,hash_size (REFTABLE_HASH_SHA1 ));check (!ret);for (i =0 ; i <N ; i++) {
@@ -326,7 +326,7 @@ static void t_index_block_read_write(void)block_writer_release (&bw); -block_reader_init (&br, &block, header_off, block_size,GIT_SHA1_RAWSZ ); +block_reader_init (&br, &block, header_off, block_size,REFTABLE_HASH_SIZE_SHA1 );block_iter_seek_start (&it, &br);
@@ -337,7 +337,7 @@ static void t_index_block_read_write(void) check_int(i, ==, N);break ; } -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); }for (i =0 ; i <N ; i++) {
@@ -350,7 +350,7 @@ static void t_index_block_read_write(void) ret =block_iter_next (&it, &rec); check_int(ret, ==, 0); -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); want.len--; ret =block_iter_seek_key (&it, &br, &want);
@@ -358,7 +358,7 @@ static void t_index_block_read_write(void) ret =block_iter_next (&it, &rec); check_int(ret, ==, 0); -check (reftable_record_equal (&recs[10 * (i /10 )], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[10 * (i /10 )], &rec,REFTABLE_HASH_SIZE_SHA1 )); }block_reader_release (&br);
t/unit-tests/t-reftable-merged.c +13 -13
@@ -42,7 +42,7 @@ merged_table_from_records(struct reftable_ref_record **refs,check (!err); } - err =reftable_merged_table_new (&mt, *readers, n,GIT_SHA1_FORMAT_ID ); + err =reftable_merged_table_new (&mt, *readers, n,REFTABLE_HASH_SHA1 );check (!err);return mt; }
@@ -91,7 +91,7 @@ static void t_merged_single_record(void) err =reftable_iterator_next_ref (&it, &ref);check (!err); -check (reftable_ref_record_equal (&r2[0 ], &ref,GIT_SHA1_RAWSZ )); +check (reftable_ref_record_equal (&r2[0 ], &ref,REFTABLE_HASH_SIZE_SHA1 ));reftable_ref_record_release (&ref);reftable_iterator_destroy (&it);readers_destroy (readers,3 );
@@ -168,7 +168,7 @@ static void t_merged_refs(void)check (!err); err =reftable_iterator_seek_ref (&it,"a" );check (!err); - check_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID); + check_int(reftable_merged_table_hash_id(mt), ==, REFTABLE_HASH_SHA1); check_int(reftable_merged_table_min_update_index(mt), ==, 1); check_int(reftable_merged_table_max_update_index(mt), ==, 3);
@@ -186,7 +186,7 @@ static void t_merged_refs(void) check_int(ARRAY_SIZE(want), ==, len);for (i =0 ; i < len; i++)check (reftable_ref_record_equal (want[i], &out[i], -GIT_SHA1_RAWSZ )); +REFTABLE_HASH_SIZE_SHA1 ));for (i =0 ; i < len; i++)reftable_ref_record_release (&out[i]);reftable_free (out);
@@ -252,12 +252,12 @@ static void t_merged_seek_multiple_times(void) err =reftable_iterator_next_ref (&it, &rec);check (!err); - err =reftable_ref_record_equal (&rec, &r1[1 ],GIT_SHA1_RAWSZ ); + err =reftable_ref_record_equal (&rec, &r1[1 ],REFTABLE_HASH_SIZE_SHA1 );check (err ==1 ); err =reftable_iterator_next_ref (&it, &rec);check (!err); - err =reftable_ref_record_equal (&rec, &r2[1 ],GIT_SHA1_RAWSZ ); + err =reftable_ref_record_equal (&rec, &r2[1 ],REFTABLE_HASH_SIZE_SHA1 );check (err ==1 ); err =reftable_iterator_next_ref (&it, &rec);
@@ -300,7 +300,7 @@ merged_table_from_log_records(struct reftable_log_record **logs,check (!err); } - err =reftable_merged_table_new (&mt, *readers, n,GIT_SHA1_FORMAT_ID ); + err =reftable_merged_table_new (&mt, *readers, n,REFTABLE_HASH_SHA1 );check (!err);return mt; }
@@ -377,7 +377,7 @@ static void t_merged_logs(void)check (!err); err =reftable_iterator_seek_log (&it,"a" );check (!err); - check_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID); + check_int(reftable_merged_table_hash_id(mt), ==, REFTABLE_HASH_SHA1); check_int(reftable_merged_table_min_update_index(mt), ==, 1); check_int(reftable_merged_table_max_update_index(mt), ==, 3);
@@ -395,7 +395,7 @@ static void t_merged_logs(void) check_int(ARRAY_SIZE(want), ==, len);for (i =0 ; i < len; i++)check (reftable_log_record_equal (want[i], &out[i], -GIT_SHA1_RAWSZ )); +REFTABLE_HASH_SIZE_SHA1 )); err =merged_table_init_iter (mt, &it,BLOCK_TYPE_LOG );check (!err);
@@ -404,7 +404,7 @@ static void t_merged_logs(void)reftable_log_record_release (&out[0 ]); err =reftable_iterator_next_log (&it, &out[0 ]);check (!err); -check (reftable_log_record_equal (&out[0 ], &r3[0 ],GIT_SHA1_RAWSZ )); +check (reftable_log_record_equal (&out[0 ], &r3[0 ],REFTABLE_HASH_SIZE_SHA1 ));reftable_iterator_destroy (&it);for (i =0 ; i < len; i++)
@@ -448,11 +448,11 @@ static void t_default_write_opts(void)check (!err); hash_id =reftable_reader_hash_id (rd); - check_int(hash_id, ==, GIT_SHA1_FORMAT_ID); + check_int(hash_id, ==, REFTABLE_HASH_SHA1); - err =reftable_merged_table_new (&merged, &rd,1 ,GIT_SHA256_FORMAT_ID ); + err =reftable_merged_table_new (&merged, &rd,1 ,REFTABLE_HASH_SHA256 ); check_int(err, ==, REFTABLE_FORMAT_ERROR); - err =reftable_merged_table_new (&merged, &rd,1 ,GIT_SHA1_FORMAT_ID ); + err =reftable_merged_table_new (&merged, &rd,1 ,REFTABLE_HASH_SHA1 );check (!err);reftable_reader_decref (rd);
t/unit-tests/t-reftable-pq.c +1 -1
@@ -132,7 +132,7 @@ static void t_merged_iter_pqueue_top(void)merged_iter_pqueue_check (&pq);check (pq_entry_equal (&top, &e)); -check (reftable_record_equal (top.rec, &recs[i],GIT_SHA1_RAWSZ )); +check (reftable_record_equal (top.rec, &recs[i],REFTABLE_HASH_SIZE_SHA1 ));for (size_t j =0 ; i < pq.len; j++) {check (pq_less (&top, &pq.heap[j])); check_int(top.index, >, j);
t/unit-tests/t-reftable-reader.c +2 -2
@@ -31,7 +31,7 @@ static int t_reader_seek_once(void) ret =reftable_iterator_next_ref (&it, &ref);check (!ret); - ret =reftable_ref_record_equal (&ref, &records[0 ],GIT_SHA1_RAWSZ ); + ret =reftable_ref_record_equal (&ref, &records[0 ],REFTABLE_HASH_SIZE_SHA1 ); check_int(ret, ==, 1); ret =reftable_iterator_next_ref (&it, &ref);
@@ -74,7 +74,7 @@ static int t_reader_reseek(void) ret =reftable_iterator_next_ref (&it, &ref);check (!ret); - ret =reftable_ref_record_equal (&ref, &records[0 ],GIT_SHA1_RAWSZ ); + ret =reftable_ref_record_equal (&ref, &records[0 ],REFTABLE_HASH_SIZE_SHA1 ); check_int(ret, ==, 1); ret =reftable_iterator_next_ref (&it, &ref);
t/unit-tests/t-reftable-readwrite.c +20 -20
@@ -41,7 +41,7 @@ static void t_buffer(void) }static void write_table (char ***names,struct reftable_buf *buf,int N , -int block_size,uint32_t hash_id) +int block_size,enum reftable_hash hash_id) {struct reftable_write_options opts = { .block_size = block_size,
@@ -62,7 +62,7 @@ static void write_table(char ***names, struct reftable_buf *buf, int N, refs[i].refname = (*names)[i] =xstrfmt ("refs/heads/branch%02d" , i); refs[i].update_index = update_index; refs[i].value_type =REFTABLE_REF_VAL1 ; -t_reftable_set_hash (refs[i].value.val1, i,GIT_SHA1_FORMAT_ID ); +t_reftable_set_hash (refs[i].value.val1, i,REFTABLE_HASH_SHA1 ); }for (i =0 ; i <N ; i++) {
@@ -70,7 +70,7 @@ static void write_table(char ***names, struct reftable_buf *buf, int N, logs[i].update_index = update_index; logs[i].value_type =REFTABLE_LOG_UPDATE ;t_reftable_set_hash (logs[i].value.update.new_hash, i, -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 ); logs[i].value.update.message = (char *)"message" ; }
@@ -104,7 +104,7 @@ static void t_log_buffer_size(void)/* This tests buffer extension for log compression. Must use a random hash, to ensure that the compressed part is larger than the original. */ -for (i =0 ; i <GIT_SHA1_RAWSZ ; i++) { +for (i =0 ; i <REFTABLE_HASH_SIZE_SHA1 ; i++) { log.value.update.old_hash[i] = (uint8_t )(git_rand () %256 ); log.value.update.new_hash[i] = (uint8_t )(git_rand () %256 ); }
@@ -191,9 +191,9 @@ static void t_log_write_read(void) log.update_index = i; log.value_type =REFTABLE_LOG_UPDATE ;t_reftable_set_hash (log.value.update.old_hash, i, -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 );t_reftable_set_hash (log.value.update.new_hash, i +1 , -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 ); err =reftable_writer_add_log (w, &log);check (!err);
@@ -326,7 +326,7 @@ static void t_table_read_write_sequential(void)int err =0 ;int j =0 ; -write_table (&names, &buf,N ,256 ,GIT_SHA1_FORMAT_ID ); +write_table (&names, &buf,N ,256 ,REFTABLE_HASH_SHA1 );block_source_from_buf (&source, &buf);
@@ -361,7 +361,7 @@ static void t_table_write_small_table(void)char **names;struct reftable_buf buf =REFTABLE_BUF_INIT ;int N =1 ; -write_table (&names, &buf,N ,4096 ,GIT_SHA1_FORMAT_ID ); +write_table (&names, &buf,N ,4096 ,REFTABLE_HASH_SHA1 ); check_int(buf.len, <, 200);reftable_buf_release (&buf);free_names (names);
@@ -378,7 +378,7 @@ static void t_table_read_api(void)struct reftable_log_record log = {0 };struct reftable_iterator it = {0 }; -write_table (&names, &buf,N ,256 ,GIT_SHA1_FORMAT_ID ); +write_table (&names, &buf,N ,256 ,REFTABLE_HASH_SHA1 );block_source_from_buf (&source, &buf);
@@ -400,7 +400,7 @@ static void t_table_read_api(void)reftable_buf_release (&buf); } -static void t_table_read_write_seek (int index,int hash_id) +static void t_table_read_write_seek (int index,enum reftable_hash hash_id) {char **names;struct reftable_buf buf =REFTABLE_BUF_INIT ;
@@ -467,24 +467,24 @@ static void t_table_read_write_seek(int index, int hash_id)static void t_table_read_write_seek_linear (void ) { -t_table_read_write_seek (0 ,GIT_SHA1_FORMAT_ID ); +t_table_read_write_seek (0 ,REFTABLE_HASH_SHA1 ); }static void t_table_read_write_seek_linear_sha256 (void ) { -t_table_read_write_seek (0 ,GIT_SHA256_FORMAT_ID ); +t_table_read_write_seek (0 ,REFTABLE_HASH_SHA256 ); }static void t_table_read_write_seek_index (void ) { -t_table_read_write_seek (1 ,GIT_SHA1_FORMAT_ID ); +t_table_read_write_seek (1 ,REFTABLE_HASH_SHA1 ); }static void t_table_refs_for (int indexed) {char **want_names;int want_names_len =0 ; -uint8_t want_hash[GIT_SHA1_RAWSZ ]; +uint8_t want_hash[REFTABLE_HASH_SIZE_SHA1 ];struct reftable_write_options opts = { .block_size =256 ,
@@ -500,10 +500,10 @@ static void t_table_refs_for(int indexed) want_names =reftable_calloc (N +1 ,sizeof (*want_names));check (want_names !=NULL ); -t_reftable_set_hash (want_hash,4 ,GIT_SHA1_FORMAT_ID ); +t_reftable_set_hash (want_hash,4 ,REFTABLE_HASH_SHA1 );for (i =0 ; i <N ; i++) { -uint8_t hash[GIT_SHA1_RAWSZ ]; +uint8_t hash[REFTABLE_HASH_SIZE_SHA1 ];char fill[51 ] = {0 };char name[100 ];struct reftable_ref_record ref = {0 };
@@ -517,9 +517,9 @@ static void t_table_refs_for(int indexed) ref.value_type =REFTABLE_REF_VAL2 ;t_reftable_set_hash (ref.value.val2.value, i /4 , -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 );t_reftable_set_hash (ref.value.val2.target_value,3 + i /4 , -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 );/* 80 bytes / entry, so 3 entries per block. Yields 17 */
@@ -527,8 +527,8 @@ static void t_table_refs_for(int indexed) n =reftable_writer_add_ref (w, &ref); check_int(n, ==, 0); -if (!memcmp (ref.value.val2.value, want_hash,GIT_SHA1_RAWSZ ) || - !memcmp (ref.value.val2.target_value, want_hash,GIT_SHA1_RAWSZ )) +if (!memcmp (ref.value.val2.value, want_hash,REFTABLE_HASH_SIZE_SHA1 ) || + !memcmp (ref.value.val2.target_value, want_hash,REFTABLE_HASH_SIZE_SHA1 )) want_names[want_names_len++] =xstrdup (name); }
t/unit-tests/t-reftable-record.c +30 -29
175 lines not on this page · raw message
t/unit-tests/t-reftable-stack.c +17 -17
@@ -121,7 +121,7 @@ static void write_n_ref_tables(struct reftable_stack *st,snprintf (buf,sizeof (buf),"refs/heads/branch-%04" PRIuMAX, (uintmax_t )i); ref.refname = buf; -t_reftable_set_hash (ref.value.val1, i,GIT_SHA1_FORMAT_ID ); +t_reftable_set_hash (ref.value.val1, i,REFTABLE_HASH_SHA1 ); err =reftable_stack_add (st, &write_test_ref, &ref);check (!err);
@@ -169,7 +169,7 @@ static void t_reftable_stack_add_one(void) err =reftable_stack_read_ref (st, ref.refname, &dest);check (!err); -check (reftable_ref_record_equal (&ref, &dest,GIT_SHA1_RAWSZ )); +check (reftable_ref_record_equal (&ref, &dest,REFTABLE_HASH_SIZE_SHA1 )); check_int(st->readers_len, >, 0);#ifndef GIT_WINDOWS_NATIVE
@@ -280,7 +280,7 @@ static void t_reftable_stack_transaction_api(void) err =reftable_stack_read_ref (st, ref.refname, &dest);check (!err); check_int(REFTABLE_REF_SYMREF, ==, dest.value_type); -check (reftable_ref_record_equal (&ref, &dest,GIT_SHA1_RAWSZ )); +check (reftable_ref_record_equal (&ref, &dest,REFTABLE_HASH_SIZE_SHA1 ));reftable_ref_record_release (&dest);reftable_stack_destroy (st);
@@ -340,7 +340,7 @@ static void t_reftable_stack_transaction_with_reload(void)for (size_t i =0 ; i <ARRAY_SIZE (refs); i++) { err =reftable_stack_read_ref (st2, refs[i].refname, &ref);check (!err); -check (reftable_ref_record_equal (&refs[i], &ref,GIT_SHA1_RAWSZ )); +check (reftable_ref_record_equal (&refs[i], &ref,REFTABLE_HASH_SIZE_SHA1 )); }reftable_ref_record_release (&ref);
@@ -530,13 +530,13 @@ static void t_reftable_stack_add(void) refs[i].refname =xstrdup (buf); refs[i].update_index = i +1 ; refs[i].value_type =REFTABLE_REF_VAL1 ; -t_reftable_set_hash (refs[i].value.val1, i,GIT_SHA1_FORMAT_ID ); +t_reftable_set_hash (refs[i].value.val1, i,REFTABLE_HASH_SHA1 ); logs[i].refname =xstrdup (buf); logs[i].update_index =N + i +1 ; logs[i].value_type =REFTABLE_LOG_UPDATE ; logs[i].value.update.email =xstrdup ("identity@invalid" ); -t_reftable_set_hash (logs[i].value.update.new_hash, i,GIT_SHA1_FORMAT_ID ); +t_reftable_set_hash (logs[i].value.update.new_hash, i,REFTABLE_HASH_SHA1 ); }for (i =0 ; i <N ; i++) {
@@ -562,7 +562,7 @@ static void t_reftable_stack_add(void)int err =reftable_stack_read_ref (st, refs[i].refname, &dest);check (!err);check (reftable_ref_record_equal (&dest, refs + i, -GIT_SHA1_RAWSZ )); +REFTABLE_HASH_SIZE_SHA1 ));reftable_ref_record_release (&dest); }
@@ -571,7 +571,7 @@ static void t_reftable_stack_add(void)int err =reftable_stack_read_log (st, refs[i].refname, &dest);check (!err);check (reftable_log_record_equal (&dest, logs + i, -GIT_SHA1_RAWSZ )); +REFTABLE_HASH_SIZE_SHA1 ));reftable_log_record_release (&dest); }
@@ -622,14 +622,14 @@ static void t_reftable_stack_iterator(void) refs[i].refname =xstrfmt ("branch%02" PRIuMAX, (uintmax_t )i); refs[i].update_index = i +1 ; refs[i].value_type =REFTABLE_REF_VAL1 ; -t_reftable_set_hash (refs[i].value.val1, i,GIT_SHA1_FORMAT_ID ); +t_reftable_set_hash (refs[i].value.val1, i,REFTABLE_HASH_SHA1 ); logs[i].refname =xstrfmt ("branch%02" PRIuMAX, (uintmax_t )i); logs[i].update_index = i +1 ; logs[i].value_type =REFTABLE_LOG_UPDATE ; logs[i].value.update.email =xstrdup ("johndoe@invalid" ); logs[i].value.update.message =xstrdup ("commit\n" ); -t_reftable_set_hash (logs[i].value.update.new_hash, i,GIT_SHA1_FORMAT_ID ); +t_reftable_set_hash (logs[i].value.update.new_hash, i,REFTABLE_HASH_SHA1 ); }for (i =0 ; i <N ; i++) {
@@ -656,7 +656,7 @@ static void t_reftable_stack_iterator(void)if (err >0 )break ;check (!err); -check (reftable_ref_record_equal (&ref, &refs[i],GIT_SHA1_RAWSZ )); +check (reftable_ref_record_equal (&ref, &refs[i],REFTABLE_HASH_SIZE_SHA1 ));reftable_ref_record_release (&ref); } check_int(i, ==, N);
@@ -674,7 +674,7 @@ static void t_reftable_stack_iterator(void)if (err >0 )break ;check (!err); -check (reftable_log_record_equal (&log, &logs[i],GIT_SHA1_RAWSZ )); +check (reftable_log_record_equal (&log, &logs[i],REFTABLE_HASH_SIZE_SHA1 ));reftable_log_record_release (&log); } check_int(i, ==, N);
@@ -767,7 +767,7 @@ static void t_reftable_stack_tombstone(void)if (i %2 ==0 ) { refs[i].value_type =REFTABLE_REF_VAL1 ;t_reftable_set_hash (refs[i].value.val1, i, -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 ); } logs[i].refname =xstrdup (buf);
@@ -776,7 +776,7 @@ static void t_reftable_stack_tombstone(void)if (i %2 ==0 ) { logs[i].value_type =REFTABLE_LOG_UPDATE ;t_reftable_set_hash (logs[i].value.update.new_hash, i, -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 ); logs[i].value.update.email =xstrdup ("identity@invalid" ); }
@@ -836,7 +836,7 @@ static void t_reftable_stack_hash_id(void) .value.symref = (char *) "target", .update_index = 1, }; -struct reftable_write_options opts32 = { .hash_id =GIT_SHA256_FORMAT_ID }; +struct reftable_write_options opts32 = { .hash_id =REFTABLE_HASH_SHA256 };struct reftable_stack *st32 =NULL ;struct reftable_write_options opts_default = {0 };struct reftable_stack *st_default =NULL ;
@@ -859,7 +859,7 @@ static void t_reftable_stack_hash_id(void) err =reftable_stack_read_ref (st_default,"master" , &dest);check (!err); -check (reftable_ref_record_equal (&ref, &dest,GIT_SHA1_RAWSZ )); +check (reftable_ref_record_equal (&ref, &dest,REFTABLE_HASH_SIZE_SHA1 ));reftable_ref_record_release (&dest);reftable_stack_destroy (st);reftable_stack_destroy (st_default);
@@ -909,7 +909,7 @@ static void t_reflog_expire(void) logs[i].value.update.time = i; logs[i].value.update.email =xstrdup ("identity@invalid" );t_reftable_set_hash (logs[i].value.update.new_hash, i, -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 ); }for (i =1 ; i <=N ; i++) {
Signature · 1 line
2.47.0.118.gfd3785337b.dirty
Re: [PATCH 3/7] reftable/system: stop depending on "hash.h"
Quoted 17 lines · Patrick Steinhardt
refs/reftable-backend.c +1
-1@@ -15,6 +15,7 @@#include "../object.h" #include "../path.h" #include "../refs.h" +#include "../reftable/reftable-basics.h" #include "../reftable/reftable-stack.h" #include "../reftable/reftable-record.h" #include "../reftable/reftable-error.h" @@ -289,7 +290,16 @@ static struct ref_store *reftable_be_init(struct repository *repo, refs->store_flags = store_flags; refs->log_all_ref_updates =repo_settings_get_log_all_ref_updates (repo); - refs->write_options.hash_id = repo->hash_algo->format_id;
refs/reftable-backend.c static struct ref_store *reftable_be_init(struct repository *repo, · line 302
Quoted 10 lines
refs/reftable-backend.c +10
-0+switch (repo->hash_algo->format_id) { +case GIT_SHA1_FORMAT_ID : + refs->write_options.hash_id =REFTABLE_HASH_SHA1 ; +break ; +case GIT_SHA256_FORMAT_ID : + refs->write_options.hash_id =REFTABLE_HASH_SHA256 ; +break ; +default : +BUG ("unknown hash algorithm %d" , repo->hash_algo->format_id); + }
Here we define the mapping between the Git's format ID and the reftable hash external to the reftable library. This facilitates swapping uses of GIT_*_FORMAT_ID to REFTABLE_HASH_* as done in the rest of the patch which looks good.
Quoted 3 lines
refs/reftable-backend.c +0
-0refs->write_options.default_permissions =calc_shared_perm (0666 & ~mask); refs->write_options.disable_auto_compact = !git_env_bool ("GIT_TEST_REFTABLE_AUTOCOMPACTION" ,1 );
[snip]
reftable/merged.h license that can be found in the LICENSE file or at · line 13
Quoted 9 lines
reftable/merged.h +1
-0@@ -10,11 +10,12 @@ license that can be found in the LICENSE file or at#define MERGED_H #include "system.h" +#include "basics.h"
Naive question, being that "merged.h" only depends on reftable-basics.h:reftable_hash and not any of the internal reftable basics components, would it be best to instead reference it directly? Or because "merged.h" is also internal do we also prefer to use the internal "basics.h"?
Probably doesn't really matter, but I was just curious if there was any reasoning. :)
-Justin
Re: [PATCH 3/7] reftable/system: stop depending on "hash.h"
Quoted 21 lines · Justin Tobler
reftable/merged.h license that can be found in the LICENSE file or at · line 13
Quoted 13 lines · Patrick Steinhardt
refs/reftable-backend.c +0
-0reftable/merged.h +1
-0@@ -10,11 +10,12 @@ license that can be found in the LICENSE file or at#define MERGED_H #include "system.h" +#include "basics.h" Naive question, being that "
merged.h" only depends onreftable-basics.h:reftable_hashand not any of the internal reftable basics components, would it be best to instead reference it directly? Or because "merged.h" is also internal do we also prefer to use the internal "basics.h"?
Quoted 2 lines
Probably doesn't really matter, but I was just curious if there was any reasoning. :)
Not really, let's do as you propose.
Patrick
PATCH 4/7 reftable/stack: stop using `fsync_component()` directly
In reply to Patrick SteinhardtWe're executing fsync_component() directly in the reftable library so that we can fsync data to disk depending on "core.fsync". But as we're in the process of converting the reftable library to become standalone we cannot use that function in the library anymore.
Refactor the code such that users of the library can inject a custom fsync function via the write options. This allows us to get rid of the dependency on "write-or-die.h".
--- refs/reftable-backend.c | 7 ++++++ reftable/reftable-writer.h | 6 +++++ reftable/stack.c | 49 +++++++++++++++++++++++++------------- 3 files changed, 45 insertions(+), 17 deletions(-)
refs/reftable-backend.c +7 -0
@@ -24,6 +24,7 @@#include "../setup.h" #include "../strmap.h" #include "../trace2.h" +#include "../write-or-die.h" #include "parse.h" #include "refs-internal.h"
@@ -273,6 +274,11 @@ static int reftable_be_config(const char *var, const char *value,return 0 ; } +static int reftable_be_fsync (int fd) +{ +return fsync_component (FSYNC_COMPONENT_REFERENCE , fd); +} +static struct ref_store *reftable_be_init (struct repository *repo,const char *gitdir,unsigned int store_flags)
@@ -304,6 +310,7 @@ static struct ref_store *reftable_be_init(struct repository *repo, refs->write_options.disable_auto_compact = !git_env_bool ("GIT_TEST_REFTABLE_AUTOCOMPACTION" ,1 ); refs->write_options.lock_timeout_ms =100 ; + refs->write_options.fsync = reftable_be_fsync;git_config (reftable_be_config, &refs->write_options);
reftable/reftable-writer.h +6 -0
@@ -62,6 +62,12 @@ struct reftable_write_options {* negative value will cause us to block indefinitely. */ long lock_timeout_ms; + +/* +* Optional callback used to fsync files to disk. Falls back to using +* fsync(3P) when unset. +*/ +int (*fsync)(int fd); };/* reftable_block_stats holds statistics for a single block type */
reftable/stack.c +32 -17
111 lines not on this page · raw message
Signature · 1 line
2.47.0.118.gfd3785337b.dirty
Re: [PATCH 4/7] reftable/stack: stop using `fsync_component()` directly
On 24/10/23 11:56AM, Patrick Steinhardt wrote:
[snip]
Quoted 19 lines
reftable/stack.c +1
-2@@ -8,7 +8,6 @@ license that can be found in the LICENSE file or at#include "stack.h" -#include "../write-or-die.h" #include "system.h" #include "constants.h" #include "merged.h" @@ -43,17 +42,28 @@ static int stack_filename(struct reftable_buf *dest, struct reftable_stack *st,return 0 ; } -static ssize_t reftable_fd_write (void *arg,const void *data,size_t sz) +static int stack_fsync (struct reftable_stack *st,int fd) {
reftable/stack.c static int stack_filename(struct reftable_buf *dest, struct reftable_stack *st, · line 53
Quoted 10 lines
reftable/stack.c +5
-3-int *fdp = (int *)arg; -return write_in_full (*fdp, data, sz); +if (st->opts.fsync) +return st->opts.fsync (fd); +return fsync (fd); } -static int reftable_fd_flush (void *arg) +struct fd_writer { +struct reftable_stack *stack;
Out of curiousity, from the stack I think we only need the callback in the options. Any reason we provide the whole stack here?
reftable/stack.c static int stack_filename(struct reftable_buf *dest, struct reftable_stack *st, · line 56
Quoted 11 lines
reftable/stack.c +7
-2+int fd; +}; + +static ssize_t fd_writer_write (void *arg,const void *data,size_t sz) { -int *fdp = (int *)arg; +struct fd_writer *writer = arg; +return write_in_full (writer->fd, data, sz); +} -return fsync_component (FSYNC_COMPONENT_REFERENCE , *fdp);
Previously when the writer was flushed it would invoke fsync_component(). Now that a callback can be configured in the stack options, the callback needs to also be propagated to fd_writer_write() in addition to the file descriptor being synced. This explains why fd_writer is now used.
The rest of the patch updates writer configuration and fsync call sites. Looks good.
-Justin
Re: [PATCH 4/7] reftable/stack: stop using `fsync_component()` directly
Quoted 20 lines · Justin Tobler
On 24/10/23 11:56AM, Patrick Steinhardt wrote:
[snip]Quoted 18 lines
reftable/stack.c +4
-3@@ -43,17 +42,28 @@ static int stack_filename(struct reftable_buf *dest, struct reftable_stack *st,return 0 ; } -static ssize_t reftable_fd_write (void *arg,const void *data,size_t sz) +static int stack_fsync (struct reftable_stack *st,int fd) { -int *fdp = (int *)arg; -return write_in_full (*fdp, data, sz); +if (st->opts.fsync) +return st->opts.fsync (fd); +return fsync (fd); }
Quoted 6 lines
reftable/stack.c static int stack_filename(struct reftable_buf *dest, struct reftable_stack *st, · line 53
Quoted 3 lines
reftable/stack.c +2
-1-static int reftable_fd_flush (void *arg) +struct fd_writer { +struct reftable_stack *stack;Out of curiousity, from the stack I think we only need the callback in the options. Any reason we provide the whole stack here?
I just think that passing around function pointers doesn't make for a good calling convention here, as it hides the fact that it is possible to call this without a callback. But there isn't a reason to pass in the whole stack, it would also be fine to instead pass in e.g. the write options.
I think I'll do that instead.
Patrick
PATCH 5/7 reftable/system: provide thin wrapper for tempfile subsystem
In reply to Patrick SteinhardtWe use the tempfile subsystem to write temporary tables, but given that we're in the process of converting the reftable library to become standalone we cannot use this subsystem directly anymore. While we could in theory convert the code to use mkstemp(3p) instead, we'd lose access to our infrastructure that automatically prunes tempfiles via atexit(3p) or signal handlers.
Provide a thin wrapper for the tempfile subsystem instead. Like this, the compatibility shim is fully self-contained in "reftable/system.c". Downstream users of the reftable library would have to implement their own tempfile shims by replacing "system.c" with a custom version.
--- Makefile | 1 + reftable/stack.c | 57 +++++++++++++++++++---------------------------- reftable/system.c | 49 ++++++++++++++++++++++++++++++++++++++++ reftable/system.h | 41 +++++++++++++++++++++++++++++++++- 4 files changed, 113 insertions(+), 35 deletions(-) create mode 100644 reftable/system.c
Makefile +1 -0
@@ -2722,6 +2722,7 @@ REFTABLE_OBJS += reftable/pq.o REFTABLE_OBJS += reftable/reader.o REFTABLE_OBJS += reftable/record.o REFTABLE_OBJS += reftable/stack.o +REFTABLE_OBJS += reftable/system.o REFTABLE_OBJS += reftable/tree.o REFTABLE_OBJS += reftable/writer.o
reftable/stack.c +23 -34
159 lines not on this page · raw message
reftable/system.c new file +49 -0
49 lines not on this page · raw message
reftable/system.h +40 -1
46 lines not on this page · raw message
Signature · 1 line
2.47.0.118.gfd3785337b.dirty
PATCH 6/7 reftable/stack: drop only use of `get_locked_file_path()`
In reply to Patrick SteinhardtWe've got a single callsite where we call get_locked_file_path(). As we're about to convert our usage of the lockfile subsystem to instead be used via a compatibility shim we'd have to implement more logic for this single callsite. While that would be okay if Git was the only supposed user of the reftable library, it's a bit more awkward when considering that we have to reimplement this functionality for every user of the library eventually.
Refactor the code such that we don't call get_locked_file_path() anymore.
--- reftable/stack.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
reftable/stack.c +9 -3
@@ -1493,9 +1493,15 @@ static int stack_compact_range(struct reftable_stack *st,*/ for (i =0 ; i < nlocks; i++) {struct lock_file *table_lock = &table_locks[i]; -char *table_path =get_locked_file_path (table_lock); -unlink (table_path); -reftable_free (table_path); +const char *lock_path =get_lock_file_path (table_lock); + +reftable_buf_reset (&table_name); + err =reftable_buf_add (&table_name, lock_path, +strlen (lock_path) -strlen (".lock" )); +if (err) +continue ; + +unlink (table_name.buf); } done:
Signature · 1 line
2.47.0.118.gfd3785337b.dirty
PATCH 7/7 reftable/system: provide thin wrapper for lockfile subsystem
In reply to Patrick SteinhardtWe use the lockfile subsystem to write lockfiles for "tables.list". As with the tempfile subsystem, the lockfile subsystem also hooks into our infrastructure to prune stale locks via atexit(3p) or signal handlers.
Furthermore, the lockfile subsystem also handles locking timeouts, which do add quite a bit of logic. Having to reimplement that in the context of Git wouldn't make a whole lot of sense, and it is quite likely that downstream users of the reftable library may have a better idea for how exactly to implement timeouts.
So again, provide a thin wrapper for the lockfile subsystem instead such that the compatibility shim is fully self-contained.
--- reftable/stack.c | 63 ++++++++++------------- reftable/system.c | 77 +++++++++++++++++++++++++++++ reftable/system.h | 45 ++++++++++++++++- t/unit-tests/lib-reftable.c | 1 + t/unit-tests/t-reftable-block.c | 1 + t/unit-tests/t-reftable-pq.c | 1 + t/unit-tests/t-reftable-readwrite.c | 1 + t/unit-tests/t-reftable-stack.c | 2 + 8 files changed, 154 insertions(+), 37 deletions(-)
reftable/stack.c +27 -36
@@ -657,7 +657,7 @@ static int format_name(struct reftable_buf *dest, uint64_t min, uint64_t max) }struct reftable_addition { -struct lock_file tables_list_lock; +struct reftable_flock tables_list_lock;struct reftable_stack *stack;char **new_tables;
@@ -676,10 +676,8 @@ static int reftable_stack_init_addition(struct reftable_addition *add, add->stack = st; - err =hold_lock_file_for_update_timeout (&add->tables_list_lock, - st->list_file, -LOCK_NO_DEREF , - st->opts.lock_timeout_ms); + err =flock_acquire (&add->tables_list_lock, st->list_file, + st->opts.lock_timeout_ms);if (err <0 ) {if (errno ==EEXIST ) { err =REFTABLE_LOCK_ERROR ;
@@ -689,7 +687,7 @@ static int reftable_stack_init_addition(struct reftable_addition *add, goto done; }if (st->opts.default_permissions) { -if (chmod (get_lock_file_path (&add->tables_list_lock), +if (chmod (add->tables_list_lock.path, st->opts.default_permissions) <0 ) { err =REFTABLE_IO_ERROR ; goto done;
@@ -733,7 +731,7 @@ static void reftable_addition_close(struct reftable_addition *add) add->new_tables_len =0 ; add->new_tables_cap =0 ; -rollback_lock_file (&add->tables_list_lock); +flock_release (&add->tables_list_lock);reftable_buf_release (&nm); }
@@ -749,7 +747,6 @@ void reftable_addition_destroy(struct reftable_addition *add)int reftable_addition_commit (struct reftable_addition *add) {struct reftable_buf table_list =REFTABLE_BUF_INIT ; -int lock_file_fd =get_lock_file_fd (&add->tables_list_lock);int err =0 ;size_t i;
@@ -767,20 +764,20 @@ int reftable_addition_commit(struct reftable_addition *add) goto done; } - err =write_in_full (lock_file_fd, table_list.buf, table_list.len); + err =write_in_full (add->tables_list_lock.fd, table_list.buf, table_list.len);reftable_buf_release (&table_list);if (err <0 ) { err =REFTABLE_IO_ERROR ; goto done; } - err =stack_fsync (add->stack, lock_file_fd); + err =stack_fsync (add->stack, add->tables_list_lock.fd);if (err <0 ) { err =REFTABLE_IO_ERROR ; goto done; } - err =commit_lock_file (&add->tables_list_lock); + err =flock_commit (&add->tables_list_lock);if (err <0 ) { err =REFTABLE_IO_ERROR ; goto done;
@@ -1160,8 +1157,8 @@ static int stack_compact_range(struct reftable_stack *st,struct reftable_buf new_table_name =REFTABLE_BUF_INIT ;struct reftable_buf new_table_path =REFTABLE_BUF_INIT ;struct reftable_buf table_name =REFTABLE_BUF_INIT ; -struct lock_file tables_list_lock =LOCK_INIT ; -struct lock_file *table_locks =NULL ; +struct reftable_flock tables_list_lock =REFTABLE_FLOCK_INIT ; +struct reftable_flock *table_locks =NULL ;struct reftable_tmpfile new_table =REFTABLE_TMPFILE_INIT ;int is_empty_table =0 , err =0 ;size_t first_to_replace, last_to_replace;
@@ -1179,10 +1176,7 @@ static int stack_compact_range(struct reftable_stack *st,* Hold the lock so that we can read "tables.list" and lock all tables * which are part of the user-specified range. */ - err =hold_lock_file_for_update_timeout (&tables_list_lock, - st->list_file, -LOCK_NO_DEREF , - st->opts.lock_timeout_ms); + err =flock_acquire (&tables_list_lock, st->list_file, st->opts.lock_timeout_ms);if (err <0 ) {if (errno ==EEXIST ) err =REFTABLE_LOCK_ERROR ;
@@ -1205,19 +1199,20 @@ static int stack_compact_range(struct reftable_stack *st,* older process is still busy compacting tables which are preexisting * from the point of view of the newer process. */ -REFTABLE_CALLOC_ARRAY (table_locks, last - first +1 ); +REFTABLE_ALLOC_ARRAY (table_locks, last - first +1 );if (!table_locks) { err =REFTABLE_OUT_OF_MEMORY_ERROR ; goto done; } +for (i =0 ; i < last - first +1 ; i++) + table_locks[i] =REFTABLE_FLOCK_INIT ;for (i = last +1 ; i > first; i--) { err =stack_filename (&table_name, st,reader_name (st->readers[i -1 ]));if (err <0 ) goto done; - err =hold_lock_file_for_update (&table_locks[nlocks], - table_name.buf,LOCK_NO_DEREF ); + err =flock_acquire (&table_locks[nlocks], table_name.buf,0 );if (err <0 ) {/* * When the table is locked already we may do a
@@ -1253,7 +1248,7 @@ static int stack_compact_range(struct reftable_stack *st,* run into file descriptor exhaustion when we compress a lot * of tables. */ - err =close_lock_file_gently (&table_locks[nlocks++]); + err =flock_close (&table_locks[nlocks++]);if (err <0 ) { err =REFTABLE_IO_ERROR ; goto done;
@@ -1265,7 +1260,7 @@ static int stack_compact_range(struct reftable_stack *st,* "tables.list" lock while compacting the locked tables. This allows * concurrent updates to the stack to proceed. */ - err =rollback_lock_file (&tables_list_lock); + err =flock_release (&tables_list_lock);if (err <0 ) { err =REFTABLE_IO_ERROR ; goto done;
@@ -1288,10 +1283,7 @@ static int stack_compact_range(struct reftable_stack *st,* "tables.list". We'll then replace the compacted range of tables with * the new table. */ - err =hold_lock_file_for_update_timeout (&tables_list_lock, - st->list_file, -LOCK_NO_DEREF , - st->opts.lock_timeout_ms); + err =flock_acquire (&tables_list_lock, st->list_file, st->opts.lock_timeout_ms);if (err <0 ) {if (errno ==EEXIST ) err =REFTABLE_LOCK_ERROR ;
@@ -1301,7 +1293,7 @@ static int stack_compact_range(struct reftable_stack *st, }if (st->opts.default_permissions) { -if (chmod (get_lock_file_path (&tables_list_lock), +if (chmod (tables_list_lock.path, st->opts.default_permissions) <0 ) { err =REFTABLE_IO_ERROR ; goto done;
@@ -1456,7 +1448,7 @@ static int stack_compact_range(struct reftable_stack *st, goto done; } - err =write_in_full (get_lock_file_fd (&tables_list_lock), + err =write_in_full (tables_list_lock.fd, tables_list_buf.buf, tables_list_buf.len);if (err <0 ) { err =REFTABLE_IO_ERROR ;
@@ -1464,14 +1456,14 @@ static int stack_compact_range(struct reftable_stack *st, goto done; } - err =stack_fsync (st,get_lock_file_fd (&tables_list_lock)); + err =stack_fsync (st, tables_list_lock.fd);if (err <0 ) { err =REFTABLE_IO_ERROR ;unlink (new_table_path.buf); goto done; } - err =commit_lock_file (&tables_list_lock); + err =flock_commit (&tables_list_lock);if (err <0 ) { err =REFTABLE_IO_ERROR ;unlink (new_table_path.buf);
@@ -1492,12 +1484,11 @@ static int stack_compact_range(struct reftable_stack *st,* readers, so it is expected that unlinking tables may fail. */ for (i =0 ; i < nlocks; i++) { -struct lock_file *table_lock = &table_locks[i]; -const char *lock_path =get_lock_file_path (table_lock); +struct reftable_flock *table_lock = &table_locks[i];reftable_buf_reset (&table_name); - err =reftable_buf_add (&table_name, lock_path, -strlen (lock_path) -strlen (".lock" )); + err =reftable_buf_add (&table_name, table_lock->path, +strlen (table_lock->path) -strlen (".lock" ));if (err)continue ;
@@ -1505,9 +1496,9 @@ static int stack_compact_range(struct reftable_stack *st, } done: -rollback_lock_file (&tables_list_lock); +flock_release (&tables_list_lock);for (i =0 ; table_locks && i < nlocks; i++) -rollback_lock_file (&table_locks[i]); +flock_release (&table_locks[i]);reftable_free (table_locks);tmpfile_delete (&new_table);
reftable/system.c +77 -0
86 lines not on this page · raw message
reftable/system.h +44 -1
55 lines not on this page · raw message
t/unit-tests/lib-reftable.c +1 -0
@@ -2,6 +2,7 @@#include "test-lib.h" #include "reftable/constants.h" #include "reftable/writer.h" +#include "strbuf.h" void t_reftable_set_hash (uint8_t *p,int i,enum reftable_hash id) {
t/unit-tests/t-reftable-block.c +1 -0
@@ -11,6 +11,7 @@ license that can be found in the LICENSE file or at#include "reftable/blocksource.h" #include "reftable/constants.h" #include "reftable/reftable-error.h" +#include "strbuf.h" static void t_ref_block_read_write (void ) {
t/unit-tests/t-reftable-pq.c +1 -0
@@ -9,6 +9,7 @@ license that can be found in the LICENSE file or at#include "test-lib.h" #include "reftable/constants.h" #include "reftable/pq.h" +#include "strbuf.h" static void merged_iter_pqueue_check (const struct merged_iter_pqueue *pq) {
t/unit-tests/t-reftable-readwrite.c +1 -0
@@ -13,6 +13,7 @@ license that can be found in the LICENSE file or at#include "reftable/reader.h" #include "reftable/reftable-error.h" #include "reftable/reftable-writer.h" +#include "strbuf.h" static const int update_index =5 ;
t/unit-tests/t-reftable-stack.c +2 -0
@@ -13,6 +13,8 @@ license that can be found in the LICENSE file or at#include "reftable/reader.h" #include "reftable/reftable-error.h" #include "reftable/stack.h" +#include "strbuf.h" +#include "tempfile.h" #include <dirent.h> static void clear_dir (const char *dirname)
Signature · 1 line
2.47.0.118.gfd3785337b.dirty
v2 cover reftable: stop using Git subsystems
In reply to Patrick SteinhardtHi,
this is the second version of my patch series that continues to detangle the reftable library from the Git codebase.
Changes compared to v1:
- Fix a commit message typo.
- Document the values of the newly introduced reftable format IDs.
- Include "reftable-basics.h" instead of "basics.h".
- Adapt stack_fsync() to take write options as input instead of the whole stack.
Thanks!
Patrick
Patrick Steinhardt (7):
reftable/system: move "dir.h" to its only user
reftable: explicitly handle hash format IDs reftable/system: stop depending on "hash.h" reftable/stack: stop using fsync_component() directly reftable/system: provide thin wrapper for tempfile subsystem reftable/stack: drop only use of get_locked_file_path() reftable/system: provide thin wrapper for lockfile subsystem
Makefile | 1 + refs/reftable-backend.c | 19 +++- reftable/basics.c | 13 ++- reftable/basics.h | 10 +- reftable/merged.c | 4 +- reftable/merged.h | 3 +- reftable/reader.c | 14 ++- reftable/reader.h | 4 +- reftable/reftable-basics.h | 13 +++ reftable/reftable-merged.h | 4 +- reftable/reftable-reader.h | 2 +- reftable/reftable-record.h | 12 +- reftable/reftable-writer.h | 8 +- reftable/stack.c | 171 ++++++++++++++-------------- reftable/system.c | 126 ++++++++++++++++++++ reftable/system.h | 88 +++++++++++++- reftable/writer.c | 20 +++- t/helper/test-reftable.c | 4 +- t/unit-tests/lib-reftable.c | 5 +- t/unit-tests/lib-reftable.h | 2 +- t/unit-tests/t-reftable-block.c | 41 +++---- t/unit-tests/t-reftable-merged.c | 26 ++--- t/unit-tests/t-reftable-pq.c | 3 +- t/unit-tests/t-reftable-reader.c | 4 +- t/unit-tests/t-reftable-readwrite.c | 41 +++---- t/unit-tests/t-reftable-record.c | 59 +++++----- t/unit-tests/t-reftable-stack.c | 37 +++--- 27 files changed, 505 insertions(+), 229 deletions(-) create mode 100644 reftable/system.c
Range-diff against v1:
1: 036cc8f9d60 ! 1: 2b7d4e28529 reftable/system: move "dir.h" to its only user
@@ Metadata
## Commit message ##
reftable/system: move "dir.h" to its only user
- We still include "dir.h" in "reftable/system.h" evne though it is not
+ We still include "dir.h" in "reftable/system.h" even though it is not
used by anything but by a single unit test. Move it over into that unit test so that we don't accidentally use any functionality provided by it in the reftable codebase.
2: c1bd8e2b3c4 ! 2: 38cfe85bf5b reftable: explicitly handle hash format IDs
@@ reftable/basics.h: int common_prefix_size(struct reftable_buf *a, struct reftabl
+/*
+ * Format IDs that identify the hash function used by a reftable. Note that
-+ * these constants end up on disk and thus mustn't change.
++ * these constants end up on disk and thus mustn't change. The format IDs are ++ * "sha1" and "s256" in big endian, respectively.
+ */
+#define REFTABLE_FORMAT_ID_SHA1 ((uint32_t) 0x73686131)
+#define REFTABLE_FORMAT_ID_SHA256 ((uint32_t) 0x73323536)
3: b595668a5cd ! 3: 745c1a070dd reftable/system: stop depending on "hash.h"
@@ reftable/merged.h: license that can be found in the LICENSE file or at
#define MERGED_H
#include "system.h"
-+#include "basics.h"
++#include "reftable-basics.h"
struct reftable_merged_table {
struct reftable_reader **readers;
4: 86269fc4fca ! 4: 7782652b975 reftable/stack: stop using `fsync_component()` directly
@@ reftable/stack.c: static int stack_filename(struct reftable_buf *dest, struct re
}
-static ssize_t reftable_fd_write(void *arg, const void *data, size_t sz)
-+static int stack_fsync(struct reftable_stack *st, int fd)
++static int stack_fsync(const struct reftable_write_options *opts, int fd)
{
- int *fdp = (int *)arg;
- return write_in_full(*fdp, data, sz);
-+ if (st->opts.fsync)
-+ return st->opts.fsync(fd);
++ if (opts->fsync)
++ return opts->fsync(fd);
+ return fsync(fd);
}
-static int reftable_fd_flush(void *arg)
+struct fd_writer {
-+ struct reftable_stack *stack;
++ const struct reftable_write_options *opts;
+ int fd;
+};
+
@@ reftable/stack.c: static int stack_filename(struct reftable_buf *dest, struct re
+static int fd_writer_flush(void *arg)
+{
+ struct fd_writer *writer = arg;
-+ return stack_fsync(writer->stack, writer->fd);
++ return stack_fsync(writer->opts, writer->fd);
}
int reftable_new_stack(struct reftable_stack **dest, const char *dir,
@@ reftable/stack.c: int reftable_addition_commit(struct reftable_addition *add)
}
- err = fsync_component(FSYNC_COMPONENT_REFERENCE, lock_file_fd);
-+ err = stack_fsync(add->stack, lock_file_fd);
++ err = stack_fsync(&add->stack->opts, lock_file_fd);
if (err < 0) {
err = REFTABLE_IO_ERROR;
goto done;
@@ reftable/stack.c: int reftable_addition_add(struct reftable_addition *add,
struct reftable_writer *wr = NULL;
struct tempfile *tab_file = NULL;
+ struct fd_writer writer = {
-+ .stack = add->stack,
++ .opts = &add->stack->opts,
+ };
int err = 0;
- int tab_fd;
@@ reftable/stack.c: static int stack_compact_locked(struct reftable_stack *st,
struct reftable_buf tab_file_path = REFTABLE_BUF_INIT;
struct reftable_writer *wr = NULL;
+ struct fd_writer writer= {
-+ .stack = st,
++ .opts = &st->opts,
+ };
struct tempfile *tab_file;
- int tab_fd, err = 0;
@@ reftable/stack.c: static int stack_compact_range(struct reftable_stack *st,
}
- err = fsync_component(FSYNC_COMPONENT_REFERENCE, get_lock_file_fd(&tables_list_lock));
-+ err = stack_fsync(st, get_lock_file_fd(&tables_list_lock));
++ err = stack_fsync(&st->opts, get_lock_file_fd(&tables_list_lock));
if (err < 0) {
err = REFTABLE_IO_ERROR;
unlink(new_table_path.buf);
5: aca19955560 ! 5: b15daefbc83 reftable/system: provide thin wrapper for tempfile subsystem
@@ reftable/stack.c: int reftable_addition_add(struct reftable_addition *add,
- struct tempfile *tab_file = NULL;
+ struct reftable_tmpfile tab_file = REFTABLE_TMPFILE_INIT;
struct fd_writer writer = {
- .stack = add->stack,
+ .opts = &add->stack->opts,
};
@@ reftable/stack.c: int reftable_addition_add(struct reftable_addition *add,
if (err < 0)
@@ reftable/stack.c: uint64_t reftable_stack_next_update_index(struct reftable_stac
struct reftable_buf tab_file_path = REFTABLE_BUF_INIT;
@@ reftable/stack.c: static int stack_compact_locked(struct reftable_stack *st,
struct fd_writer writer= {
- .stack = st,
+ .opts = &st->opts,
};
- struct tempfile *tab_file;
+ struct reftable_tmpfile tab_file = REFTABLE_TMPFILE_INIT;
6: 74afe30974d = 6: 83949837a29 reftable/stack: drop only use of `get_locked_file_path()`
7: 71b213d6f8a ! 7: 80fe5bc5e10 reftable/system: provide thin wrapper for lockfile subsystem
@@ reftable/stack.c: int reftable_addition_commit(struct reftable_addition *add)
goto done;
}
-- err = stack_fsync(add->stack, lock_file_fd);
-+ err = stack_fsync(add->stack, add->tables_list_lock.fd);
+- err = stack_fsync(&add->stack->opts, lock_file_fd);
++ err = stack_fsync(&add->stack->opts, add->tables_list_lock.fd);
if (err < 0) {
err = REFTABLE_IO_ERROR;
goto done;
@@ reftable/stack.c: static int stack_compact_range(struct reftable_stack *st,
goto done;
}
-- err = stack_fsync(st, get_lock_file_fd(&tables_list_lock));
-+ err = stack_fsync(st, tables_list_lock.fd);
+- err = stack_fsync(&st->opts, get_lock_file_fd(&tables_list_lock));
++ err = stack_fsync(&st->opts, tables_list_lock.fd);
if (err < 0) {
err = REFTABLE_IO_ERROR;
unlink(new_table_path.buf);Signature · 1 line
2.47.0.229.g8f8d6eee53.dirty
PATCH v2 1/7 reftable/system: move "dir.h" to its only user
We still include "dir.h" in "reftable/system.h" even though it is not used by anything but by a single unit test. Move it over into that unit test so that we don't accidentally use any functionality provided by it in the reftable codebase.
--- reftable/system.h | 1 - t/unit-tests/t-reftable-stack.c | 1 + 2 files changed, 1 insertion(+), 1 deletion(-)
reftable/system.h +0 -1
@@ -15,7 +15,6 @@ license that can be found in the LICENSE file or at#include "lockfile.h" #include "tempfile.h" #include "hash.h" /* hash ID, sizes.*/ -#include "dir.h" /* remove_dir_recursively, for tests.*/ int hash_size (uint32_t id);
t/unit-tests/t-reftable-stack.c +1 -0
@@ -8,6 +8,7 @@ license that can be found in the LICENSE file or at#include "test-lib.h" #include "lib-reftable.h" +#include "dir.h" #include "reftable/merged.h" #include "reftable/reader.h" #include "reftable/reftable-error.h"
Signature · 1 line
2.47.0.229.g8f8d6eee53.dirty
PATCH v2 2/7 reftable: explicitly handle hash format IDs
In reply to Patrick SteinhardtThe hash format IDs are used for two different things across the reftable codebase:
- They are used as a 32 bit unsigned integer when reading and writing the header in order to identify the hash function.
- They are used internally to identify which hash function is in use.
When one only considers the second usecase one might think that one can easily change the representation of those hash IDs. But because those IDs end up in the reftable header and footer on disk it is important that those never change.
Create separate constants REFTABLE_FORMAT_ID_* and use them in contexts where we read or write reftable headers. This serves multiple purposes:
- It allows us to more easily discern cases where we actually use those constants for the on-disk format.
- It detangles us from the same constants that are defined in libgit.a, which is another required step to convert the reftable library to become standalone.
- It makes the next step easier where we stop using GIT_*_FORMAT_ID constants in favor of a custom enum.
--- reftable/basics.h | 8 ++++++++ reftable/reader.c | 10 ++++++---- reftable/writer.c | 16 +++++++++++++++- 3 files changed, 29 insertions(+), 5 deletions(-)
reftable/basics.h +8 -0
@@ -150,4 +150,12 @@ int common_prefix_size(struct reftable_buf *a, struct reftable_buf *b);int hash_size (uint32_t id); +/* +* Format IDs that identify the hash function used by a reftable. Note that +* these constants end up on disk and thus mustn't change. The format IDs are +* "sha1" and "s256" in big endian, respectively. +*/ +#define REFTABLE_FORMAT_ID_SHA1 ((uint32_t) 0x73686131) +#define REFTABLE_FORMAT_ID_SHA256 ((uint32_t) 0x73323536) +#endif
reftable/reader.c +6 -4
@@ -109,16 +109,18 @@ static int parse_footer(struct reftable_reader *r, uint8_t *footer,if (r->version ==1 ) { r->hash_id =GIT_SHA1_FORMAT_ID ; }else { - r->hash_id =get_be32 (f); -switch (r->hash_id) { -case GIT_SHA1_FORMAT_ID : +switch (get_be32 (f)) { +case REFTABLE_FORMAT_ID_SHA1 : + r->hash_id =GIT_SHA1_FORMAT_ID ;break ; -case GIT_SHA256_FORMAT_ID : +case REFTABLE_FORMAT_ID_SHA256 : + r->hash_id =GIT_SHA256_FORMAT_ID ;break ;default : err =REFTABLE_FORMAT_ERROR ; goto done; } + f +=4 ; }
reftable/writer.c +15 -1
@@ -103,8 +103,22 @@ static int writer_write_header(struct reftable_writer *w, uint8_t *dest)put_be64 (dest +8 , w->min_update_index);put_be64 (dest +16 , w->max_update_index);if (writer_version (w) ==2 ) { -put_be32 (dest +24 , w->opts.hash_id); +uint32_t hash_id; + +switch (w->opts.hash_id) { +case GIT_SHA1_FORMAT_ID : + hash_id =REFTABLE_FORMAT_ID_SHA1 ; +break ; +case GIT_SHA256_FORMAT_ID : + hash_id =REFTABLE_FORMAT_ID_SHA256 ; +break ; +default : +return -1 ; + } + +put_be32 (dest +24 , hash_id); } +return header_size (writer_version (w)); }
Signature · 1 line
2.47.0.229.g8f8d6eee53.dirty
Re: [PATCH v2 2/7] reftable: explicitly handle hash format IDs
Quoted 20 lines · Patrick Steinhardt
The hash format IDs are used for two different things across the reftable codebase:
- They are used as a 32 bit unsigned integer when reading and writing the header in order to identify the hash function.
- They are used internally to identify which hash function is in use.
When one only considers the second usecase one might think that one can easily change the representation of those hash IDs. But because those IDs end up in the reftable header and footer on disk it is important that those never change.
Create separate constants
REFTABLE_FORMAT_ID_*and use them in contexts where we read or write reftable headers. This serves multiple purposes:- It allows us to more easily discern cases where we actually use those constants for the on-disk format.
Quoted 7 lines
- It detangles us from the same constants that are defined in libgit.a, which is another required step to convert the reftable library to become standalone.
- It makes the next step easier where we stop using
GIT_*_FORMAT_IDconstants in favor of a custom enum.
So this patch tackles the first scenario mentioned above (for reading/writing the header), changing the second scenario (identification of which hash function) is handled in the next step by introducing a custom enum. Makes sense.
- Karthik
PATCH v2 3/7 reftable/system: stop depending on "hash.h"
In reply to Patrick SteinhardtWe include "hash.h" in "reftable/system.h" such that we can use hash format IDs as well as the raw size of SHA1 and SHA256. As we are in the process of converting the reftable library to become standalone we of course cannot rely on those constants anymore.
Introduce a new enum reftable_hash to replace internal uses of the hash format IDs and new constants that replace internal uses of the hash size. Adapt the reftable backend to set up the correct hash function.
--- refs/reftable-backend.c | 12 +++++- reftable/basics.c | 13 ++++--- reftable/basics.h | 2 +- reftable/merged.c | 4 +- reftable/merged.h | 3 +- reftable/reader.c | 8 ++-- reftable/reader.h | 4 +- reftable/reftable-basics.h | 13 +++++++ reftable/reftable-merged.h | 4 +- reftable/reftable-reader.h | 2 +- reftable/reftable-record.h | 12 +++--- reftable/reftable-writer.h | 2 +- reftable/stack.c | 4 +- reftable/system.h | 3 -- reftable/writer.c | 8 ++-- t/helper/test-reftable.c | 4 +- t/unit-tests/lib-reftable.c | 4 +- t/unit-tests/lib-reftable.h | 2 +- t/unit-tests/t-reftable-block.c | 40 +++++++++---------- t/unit-tests/t-reftable-merged.c | 26 ++++++------- t/unit-tests/t-reftable-pq.c | 2 +- t/unit-tests/t-reftable-reader.c | 4 +- t/unit-tests/t-reftable-readwrite.c | 40 +++++++++---------- t/unit-tests/t-reftable-record.c | 59 +++++++++++++++-------------- t/unit-tests/t-reftable-stack.c | 34 ++++++++--------- 25 files changed, 166 insertions(+), 143 deletions(-)
refs/reftable-backend.c +11 -1
@@ -15,6 +15,7 @@#include "../object.h" #include "../path.h" #include "../refs.h" +#include "../reftable/reftable-basics.h" #include "../reftable/reftable-stack.h" #include "../reftable/reftable-record.h" #include "../reftable/reftable-error.h"
@@ -289,7 +290,16 @@ static struct ref_store *reftable_be_init(struct repository *repo, refs->store_flags = store_flags; refs->log_all_ref_updates =repo_settings_get_log_all_ref_updates (repo); - refs->write_options.hash_id = repo->hash_algo->format_id; +switch (repo->hash_algo->format_id) { +case GIT_SHA1_FORMAT_ID : + refs->write_options.hash_id =REFTABLE_HASH_SHA1 ; +break ; +case GIT_SHA256_FORMAT_ID : + refs->write_options.hash_id =REFTABLE_HASH_SHA256 ; +break ; +default : +BUG ("unknown hash algorithm %d" , repo->hash_algo->format_id); + } refs->write_options.default_permissions =calc_shared_perm (0666 & ~mask); refs->write_options.disable_auto_compact = !git_env_bool ("GIT_TEST_REFTABLE_AUTOCOMPACTION" ,1 );
reftable/basics.c +7 -6
@@ -271,14 +271,15 @@ int common_prefix_size(struct reftable_buf *a, struct reftable_buf *b)return p; } -int hash_size (uint32_t id) +int hash_size (enum reftable_hash id) { +if (!id) +return REFTABLE_HASH_SIZE_SHA1 ;switch (id) { -case 0 : -case GIT_SHA1_FORMAT_ID : -return GIT_SHA1_RAWSZ ; -case GIT_SHA256_FORMAT_ID : -return GIT_SHA256_RAWSZ ; +case REFTABLE_HASH_SHA1 : +return REFTABLE_HASH_SIZE_SHA1 ; +case REFTABLE_HASH_SHA256 : +return REFTABLE_HASH_SIZE_SHA256 ; }abort (); }
reftable/basics.h +1 -1
@@ -148,7 +148,7 @@ char *reftable_strdup(const char *str);/* Find the longest shared prefix size of `a` and `b` */ int common_prefix_size (struct reftable_buf *a,struct reftable_buf *b); -int hash_size (uint32_t id); +int hash_size (enum reftable_hash id);/* * Format IDs that identify the hash function used by a reftable. Note that
reftable/merged.c +2 -2
@@ -181,7 +181,7 @@ static void iterator_from_merged_iter(struct reftable_iterator *it,int reftable_merged_table_new (struct reftable_merged_table **dest,struct reftable_reader **readers,size_t n, -uint32_t hash_id) +enum reftable_hash hash_id) {struct reftable_merged_table *m =NULL ;uint64_t last_max =0 ;
@@ -293,7 +293,7 @@ int reftable_merged_table_init_log_iterator(struct reftable_merged_table *mt,return merged_table_init_iter (mt, it,BLOCK_TYPE_LOG ); } -uint32_t reftable_merged_table_hash_id (struct reftable_merged_table *mt) +enum reftable_hash reftable_merged_table_hash_id (struct reftable_merged_table *mt) {return mt->hash_id; }
reftable/merged.h +2 -1
@@ -10,11 +10,12 @@ license that can be found in the LICENSE file or at#define MERGED_H #include "system.h" +#include "reftable-basics.h" struct reftable_merged_table {struct reftable_reader **readers;size_t readers_len; -uint32_t hash_id; +enum reftable_hash hash_id;/* If unset, produce deletions. This is useful for compaction. For the * full stack, deletions should be produced. */
reftable/reader.c +4 -4
@@ -67,7 +67,7 @@ static int reader_get_block(struct reftable_reader *r,return block_source_read_block (&r->source, dest, off, sz); } -uint32_t reftable_reader_hash_id (struct reftable_reader *r) +enum reftable_hash reftable_reader_hash_id (struct reftable_reader *r) {return r->hash_id; }
@@ -107,14 +107,14 @@ static int parse_footer(struct reftable_reader *r, uint8_t *footer, f +=8 ;if (r->version ==1 ) { - r->hash_id =GIT_SHA1_FORMAT_ID ; + r->hash_id =REFTABLE_HASH_SHA1 ; }else {switch (get_be32 (f)) {case REFTABLE_FORMAT_ID_SHA1 : - r->hash_id =GIT_SHA1_FORMAT_ID ; + r->hash_id =REFTABLE_HASH_SHA1 ;break ;case REFTABLE_FORMAT_ID_SHA256 : - r->hash_id =GIT_SHA256_FORMAT_ID ; + r->hash_id =REFTABLE_HASH_SHA256 ;break ;default : err =REFTABLE_FORMAT_ERROR ;
reftable/reader.h +2 -2
@@ -37,8 +37,8 @@ struct reftable_reader {/* Size of the file, excluding the footer. */ uint64_t size; -/* 'sha1' for SHA1, 's256' for SHA-256 */ -uint32_t hash_id; +/* The hash function used for ref records. */ +enum reftable_hash hash_id;uint32_t block_size;uint64_t min_update_index;
reftable/reftable-basics.h +13 -0
@@ -11,6 +11,19 @@#include <stddef.h> +/* +* Hash functions understood by the reftable library. Note that the values are +* arbitrary and somewhat random such that we can easily detect cases where the +* hash hasn't been properly set up. +*/ +enum reftable_hash { +REFTABLE_HASH_SHA1 =89 , +REFTABLE_HASH_SHA256 =247 , +}; +#define REFTABLE_HASH_SIZE_SHA1 20 +#define REFTABLE_HASH_SIZE_SHA256 32 +#define REFTABLE_HASH_SIZE_MAX REFTABLE_HASH_SIZE_SHA256 +/* Overrides the functions to use for memory management. */ void reftable_set_alloc (void *(*malloc)(size_t ),void *(*realloc)(void *,size_t ),void (*free)(void *));
reftable/reftable-merged.h +2 -2
@@ -34,7 +34,7 @@ struct reftable_reader;*/ int reftable_merged_table_new (struct reftable_merged_table **dest,struct reftable_reader **readers,size_t n, -uint32_t hash_id); +enum reftable_hash hash_id);/* Initialize a merged table iterator for reading refs. */ int reftable_merged_table_init_ref_iterator(struct reftable_merged_table *mt,
@@ -56,6 +56,6 @@ reftable_merged_table_min_update_index(struct reftable_merged_table *mt);void reftable_merged_table_free (struct reftable_merged_table *m);/* return the hash ID of the merged table. */ -uint32_t reftable_merged_table_hash_id (struct reftable_merged_table *m); +enum reftable_hash reftable_merged_table_hash_id (struct reftable_merged_table *m);#endif
reftable/reftable-reader.h +1 -1
@@ -54,7 +54,7 @@ int reftable_reader_init_log_iterator(struct reftable_reader *r, struct reftable_iterator *it);/* returns the hash ID used in this table. */ -uint32_t reftable_reader_hash_id (struct reftable_reader *r); +enum reftable_hash reftable_reader_hash_id (struct reftable_reader *r);/* return an iterator for the refs pointing to `oid`. */ int reftable_reader_refs_for(struct reftable_reader *r,
reftable/reftable-record.h +6 -6
@@ -9,7 +9,7 @@ license that can be found in the LICENSE file or at#ifndef REFTABLE_RECORD_H #define REFTABLE_RECORD_H -#include "hash.h" +#include "reftable-basics.h" #include <stdint.h> /*
@@ -40,10 +40,10 @@ struct reftable_ref_record {#define REFTABLE_NR_REF_VALUETYPES 4 } value_type;union { -unsigned char val1[GIT_MAX_RAWSZ ]; +unsigned char val1[REFTABLE_HASH_SIZE_MAX ];struct { -unsigned char value[GIT_MAX_RAWSZ ];/* first hash */ -unsigned char target_value[GIT_MAX_RAWSZ ];/* second hash */ +unsigned char value[REFTABLE_HASH_SIZE_MAX ];/* first hash */ +unsigned char target_value[REFTABLE_HASH_SIZE_MAX ];/* second hash */ } val2;char *symref;/* referent, malloced 0-terminated string */ } value;
@@ -85,8 +85,8 @@ struct reftable_log_record {union {struct { -unsigned char new_hash[GIT_MAX_RAWSZ ]; -unsigned char old_hash[GIT_MAX_RAWSZ ]; +unsigned char new_hash[REFTABLE_HASH_SIZE_MAX ]; +unsigned char old_hash[REFTABLE_HASH_SIZE_MAX ];char *name;char *email;uint64_t time;
reftable/reftable-writer.h +1 -1
@@ -33,7 +33,7 @@ struct reftable_write_options {/* 4-byte identifier ("sha1", "s256") of the hash. * Defaults to SHA1 if unset */ -uint32_t hash_id; +enum reftable_hash hash_id;/* Default mode for creating files. If unset, use 0666 (+umask) */ unsigned int default_permissions;
reftable/stack.c +2 -2
@@ -73,7 +73,7 @@ int reftable_new_stack(struct reftable_stack **dest, const char *dir,if (_opts) opts = *_opts;if (opts.hash_id ==0 ) - opts.hash_id =GIT_SHA1_FORMAT_ID ; + opts.hash_id =REFTABLE_HASH_SHA1 ; *dest =NULL ;
@@ -1603,7 +1603,7 @@ struct segment suggest_compaction_segment(uint64_t *sizes, size_t n,static uint64_t *stack_table_sizes_for_compaction (struct reftable_stack *st) { -int version = (st->opts.hash_id ==GIT_SHA1_FORMAT_ID ) ?1 :2 ; +int version = (st->opts.hash_id ==REFTABLE_HASH_SHA1 ) ?1 :2 ;int overhead =header_size (version) -1 ;uint64_t *sizes;
reftable/system.h +0 -3
@@ -14,8 +14,5 @@ license that can be found in the LICENSE file or at#include "git-compat-util.h" #include "lockfile.h" #include "tempfile.h" -#include "hash.h" /* hash ID, sizes.*/ - -int hash_size (uint32_t id);#endif
reftable/writer.c +4 -4
@@ -79,7 +79,7 @@ static void options_set_defaults(struct reftable_write_options *opts) }if (opts->hash_id ==0 ) { - opts->hash_id =GIT_SHA1_FORMAT_ID ; + opts->hash_id =REFTABLE_HASH_SHA1 ; }if (opts->block_size ==0 ) { opts->block_size =DEFAULT_BLOCK_SIZE ;
@@ -88,7 +88,7 @@ static void options_set_defaults(struct reftable_write_options *opts)static int writer_version (struct reftable_writer *w) { -return (w->opts.hash_id ==0 || w->opts.hash_id ==GIT_SHA1_FORMAT_ID ) ? +return (w->opts.hash_id ==0 || w->opts.hash_id ==REFTABLE_HASH_SHA1 ) ?1 :2 ; }
@@ -106,10 +106,10 @@ static int writer_write_header(struct reftable_writer *w, uint8_t *dest)uint32_t hash_id;switch (w->opts.hash_id) { -case GIT_SHA1_FORMAT_ID : +case REFTABLE_HASH_SHA1 : hash_id =REFTABLE_FORMAT_ID_SHA1 ;break ; -case GIT_SHA256_FORMAT_ID : +case REFTABLE_HASH_SHA256 : hash_id =REFTABLE_FORMAT_ID_SHA256 ;break ;default :
t/helper/test-reftable.c +2 -2
@@ -156,7 +156,7 @@ int cmd__dump_reftable(int argc, const char **argv)int opt_dump_blocks =0 ;int opt_dump_table =0 ;int opt_dump_stack =0 ; -uint32_t opt_hash_id =GIT_SHA1_FORMAT_ID ; +uint32_t opt_hash_id =REFTABLE_HASH_SHA1 ;const char *arg =NULL , *argv0 = argv[0 ];for (; argc >1 ; argv++, argc--)
@@ -167,7 +167,7 @@ int cmd__dump_reftable(int argc, const char **argv)else if (!strcmp ("-t" , argv[1 ])) opt_dump_table =1 ;else if (!strcmp ("-6" , argv[1 ])) - opt_hash_id =GIT_SHA256_FORMAT_ID ; + opt_hash_id =REFTABLE_HASH_SHA256 ;else if (!strcmp ("-s" , argv[1 ])) opt_dump_stack =1 ;else if (!strcmp ("-?" , argv[1 ]) || !strcmp ("-h" , argv[1 ])) {
t/unit-tests/lib-reftable.c +2 -2
@@ -3,7 +3,7 @@#include "reftable/constants.h" #include "reftable/writer.h" -void t_reftable_set_hash (uint8_t *p,int i,uint32_t id) +void t_reftable_set_hash (uint8_t *p,int i,enum reftable_hash id) {memset (p, (uint8_t )i,hash_size (id)); }
@@ -82,7 +82,7 @@ void t_reftable_write_to_buf(struct reftable_buf *buf,size_t off = i * (opts.block_size ? opts.block_size :DEFAULT_BLOCK_SIZE );if (!off) - off =header_size (opts.hash_id ==GIT_SHA256_FORMAT_ID ?2 :1 ); + off =header_size (opts.hash_id ==REFTABLE_HASH_SHA256 ?2 :1 ); check_char(buf->buf[off], ==, 'r'); }
t/unit-tests/lib-reftable.h +1 -1
@@ -6,7 +6,7 @@struct reftable_buf ; -void t_reftable_set_hash (uint8_t *p,int i,uint32_t id); +void t_reftable_set_hash (uint8_t *p,int i,enum reftable_hash id);struct reftable_writer *t_reftable_strbuf_writer (struct reftable_buf *buf,struct reftable_write_options *opts);
t/unit-tests/t-reftable-block.c +20 -20
@@ -36,7 +36,7 @@ static void t_ref_block_read_write(void) block.len = block_size;block_source_from_buf (&block.source ,&buf); ret =block_writer_init (&bw,BLOCK_TYPE_REF , block.data, block_size, - header_off,hash_size (GIT_SHA1_FORMAT_ID )); + header_off,hash_size (REFTABLE_HASH_SHA1 ));check (!ret); rec.u.ref.refname = (char *)"" ;
@@ -47,7 +47,7 @@ static void t_ref_block_read_write(void)for (i =0 ; i <N ; i++) { rec.u.ref.refname =xstrfmt ("branch%02" PRIuMAX, (uintmax_t )i); rec.u.ref.value_type =REFTABLE_REF_VAL1 ; -memset (rec.u.ref.value.val1, i,GIT_SHA1_RAWSZ ); +memset (rec.u.ref.value.val1, i,REFTABLE_HASH_SIZE_SHA1 ); recs[i] = rec; ret =block_writer_add (&bw, &rec);
@@ -61,7 +61,7 @@ static void t_ref_block_read_write(void)block_writer_release (&bw); -block_reader_init (&br, &block, header_off, block_size,GIT_SHA1_RAWSZ ); +block_reader_init (&br, &block, header_off, block_size,REFTABLE_HASH_SIZE_SHA1 );block_iter_seek_start (&it, &br);
@@ -72,7 +72,7 @@ static void t_ref_block_read_write(void) check_int(i, ==, N);break ; } -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); }for (i =0 ; i <N ; i++) {
@@ -85,7 +85,7 @@ static void t_ref_block_read_write(void) ret =block_iter_next (&it, &rec); check_int(ret, ==, 0); -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); want.len--; ret =block_iter_seek_key (&it, &br, &want);
@@ -93,7 +93,7 @@ static void t_ref_block_read_write(void) ret =block_iter_next (&it, &rec); check_int(ret, ==, 0); -check (reftable_record_equal (&recs[10 * (i /10 )], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[10 * (i /10 )], &rec,REFTABLE_HASH_SIZE_SHA1 )); }block_reader_release (&br);
@@ -130,7 +130,7 @@ static void t_log_block_read_write(void) block.len = block_size;block_source_from_buf (&block.source ,&buf); ret =block_writer_init (&bw,BLOCK_TYPE_LOG , block.data, block_size, - header_off,hash_size (GIT_SHA1_FORMAT_ID )); + header_off,hash_size (REFTABLE_HASH_SHA1 ));check (!ret);for (i =0 ; i <N ; i++) {
@@ -150,7 +150,7 @@ static void t_log_block_read_write(void)block_writer_release (&bw); -block_reader_init (&br, &block, header_off, block_size,GIT_SHA1_RAWSZ ); +block_reader_init (&br, &block, header_off, block_size,REFTABLE_HASH_SIZE_SHA1 );block_iter_seek_start (&it, &br);
@@ -161,7 +161,7 @@ static void t_log_block_read_write(void) check_int(i, ==, N);break ; } -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); }for (i =0 ; i <N ; i++) {
@@ -175,7 +175,7 @@ static void t_log_block_read_write(void) ret =block_iter_next (&it, &rec); check_int(ret, ==, 0); -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); want.len--; ret =block_iter_seek_key (&it, &br, &want);
@@ -183,7 +183,7 @@ static void t_log_block_read_write(void) ret =block_iter_next (&it, &rec); check_int(ret, ==, 0); -check (reftable_record_equal (&recs[10 * (i /10 )], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[10 * (i /10 )], &rec,REFTABLE_HASH_SIZE_SHA1 )); }block_reader_release (&br);
@@ -220,7 +220,7 @@ static void t_obj_block_read_write(void) block.len = block_size;block_source_from_buf (&block.source, &buf); ret =block_writer_init (&bw,BLOCK_TYPE_OBJ , block.data, block_size, - header_off,hash_size (GIT_SHA1_FORMAT_ID )); + header_off,hash_size (REFTABLE_HASH_SHA1 ));check (!ret);for (i =0 ; i <N ; i++) {
@@ -242,7 +242,7 @@ static void t_obj_block_read_write(void)block_writer_release (&bw); -block_reader_init (&br, &block, header_off, block_size,GIT_SHA1_RAWSZ ); +block_reader_init (&br, &block, header_off, block_size,REFTABLE_HASH_SIZE_SHA1 );block_iter_seek_start (&it, &br);
@@ -253,7 +253,7 @@ static void t_obj_block_read_write(void) check_int(i, ==, N);break ; } -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); }for (i =0 ; i <N ; i++) {
@@ -266,7 +266,7 @@ static void t_obj_block_read_write(void) ret =block_iter_next (&it, &rec); check_int(ret, ==, 0); -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); }block_reader_release (&br);
@@ -304,7 +304,7 @@ static void t_index_block_read_write(void) block.len = block_size;block_source_from_buf (&block.source, &buf); ret =block_writer_init (&bw,BLOCK_TYPE_INDEX , block.data, block_size, - header_off,hash_size (GIT_SHA1_FORMAT_ID )); + header_off,hash_size (REFTABLE_HASH_SHA1 ));check (!ret);for (i =0 ; i <N ; i++) {
@@ -326,7 +326,7 @@ static void t_index_block_read_write(void)block_writer_release (&bw); -block_reader_init (&br, &block, header_off, block_size,GIT_SHA1_RAWSZ ); +block_reader_init (&br, &block, header_off, block_size,REFTABLE_HASH_SIZE_SHA1 );block_iter_seek_start (&it, &br);
@@ -337,7 +337,7 @@ static void t_index_block_read_write(void) check_int(i, ==, N);break ; } -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); }for (i =0 ; i <N ; i++) {
@@ -350,7 +350,7 @@ static void t_index_block_read_write(void) ret =block_iter_next (&it, &rec); check_int(ret, ==, 0); -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); want.len--; ret =block_iter_seek_key (&it, &br, &want);
@@ -358,7 +358,7 @@ static void t_index_block_read_write(void) ret =block_iter_next (&it, &rec); check_int(ret, ==, 0); -check (reftable_record_equal (&recs[10 * (i /10 )], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[10 * (i /10 )], &rec,REFTABLE_HASH_SIZE_SHA1 )); }block_reader_release (&br);
t/unit-tests/t-reftable-merged.c +13 -13
@@ -42,7 +42,7 @@ merged_table_from_records(struct reftable_ref_record **refs,check (!err); } - err =reftable_merged_table_new (&mt, *readers, n,GIT_SHA1_FORMAT_ID ); + err =reftable_merged_table_new (&mt, *readers, n,REFTABLE_HASH_SHA1 );check (!err);return mt; }
@@ -91,7 +91,7 @@ static void t_merged_single_record(void) err =reftable_iterator_next_ref (&it, &ref);check (!err); -check (reftable_ref_record_equal (&r2[0 ], &ref,GIT_SHA1_RAWSZ )); +check (reftable_ref_record_equal (&r2[0 ], &ref,REFTABLE_HASH_SIZE_SHA1 ));reftable_ref_record_release (&ref);reftable_iterator_destroy (&it);readers_destroy (readers,3 );
@@ -168,7 +168,7 @@ static void t_merged_refs(void)check (!err); err =reftable_iterator_seek_ref (&it,"a" );check (!err); - check_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID); + check_int(reftable_merged_table_hash_id(mt), ==, REFTABLE_HASH_SHA1); check_int(reftable_merged_table_min_update_index(mt), ==, 1); check_int(reftable_merged_table_max_update_index(mt), ==, 3);
@@ -186,7 +186,7 @@ static void t_merged_refs(void) check_int(ARRAY_SIZE(want), ==, len);for (i =0 ; i < len; i++)check (reftable_ref_record_equal (want[i], &out[i], -GIT_SHA1_RAWSZ )); +REFTABLE_HASH_SIZE_SHA1 ));for (i =0 ; i < len; i++)reftable_ref_record_release (&out[i]);reftable_free (out);
@@ -252,12 +252,12 @@ static void t_merged_seek_multiple_times(void) err =reftable_iterator_next_ref (&it, &rec);check (!err); - err =reftable_ref_record_equal (&rec, &r1[1 ],GIT_SHA1_RAWSZ ); + err =reftable_ref_record_equal (&rec, &r1[1 ],REFTABLE_HASH_SIZE_SHA1 );check (err ==1 ); err =reftable_iterator_next_ref (&it, &rec);check (!err); - err =reftable_ref_record_equal (&rec, &r2[1 ],GIT_SHA1_RAWSZ ); + err =reftable_ref_record_equal (&rec, &r2[1 ],REFTABLE_HASH_SIZE_SHA1 );check (err ==1 ); err =reftable_iterator_next_ref (&it, &rec);
@@ -300,7 +300,7 @@ merged_table_from_log_records(struct reftable_log_record **logs,check (!err); } - err =reftable_merged_table_new (&mt, *readers, n,GIT_SHA1_FORMAT_ID ); + err =reftable_merged_table_new (&mt, *readers, n,REFTABLE_HASH_SHA1 );check (!err);return mt; }
@@ -377,7 +377,7 @@ static void t_merged_logs(void)check (!err); err =reftable_iterator_seek_log (&it,"a" );check (!err); - check_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID); + check_int(reftable_merged_table_hash_id(mt), ==, REFTABLE_HASH_SHA1); check_int(reftable_merged_table_min_update_index(mt), ==, 1); check_int(reftable_merged_table_max_update_index(mt), ==, 3);
@@ -395,7 +395,7 @@ static void t_merged_logs(void) check_int(ARRAY_SIZE(want), ==, len);for (i =0 ; i < len; i++)check (reftable_log_record_equal (want[i], &out[i], -GIT_SHA1_RAWSZ )); +REFTABLE_HASH_SIZE_SHA1 )); err =merged_table_init_iter (mt, &it,BLOCK_TYPE_LOG );check (!err);
@@ -404,7 +404,7 @@ static void t_merged_logs(void)reftable_log_record_release (&out[0 ]); err =reftable_iterator_next_log (&it, &out[0 ]);check (!err); -check (reftable_log_record_equal (&out[0 ], &r3[0 ],GIT_SHA1_RAWSZ )); +check (reftable_log_record_equal (&out[0 ], &r3[0 ],REFTABLE_HASH_SIZE_SHA1 ));reftable_iterator_destroy (&it);for (i =0 ; i < len; i++)
@@ -448,11 +448,11 @@ static void t_default_write_opts(void)check (!err); hash_id =reftable_reader_hash_id (rd); - check_int(hash_id, ==, GIT_SHA1_FORMAT_ID); + check_int(hash_id, ==, REFTABLE_HASH_SHA1); - err =reftable_merged_table_new (&merged, &rd,1 ,GIT_SHA256_FORMAT_ID ); + err =reftable_merged_table_new (&merged, &rd,1 ,REFTABLE_HASH_SHA256 ); check_int(err, ==, REFTABLE_FORMAT_ERROR); - err =reftable_merged_table_new (&merged, &rd,1 ,GIT_SHA1_FORMAT_ID ); + err =reftable_merged_table_new (&merged, &rd,1 ,REFTABLE_HASH_SHA1 );check (!err);reftable_reader_decref (rd);
t/unit-tests/t-reftable-pq.c +1 -1
@@ -132,7 +132,7 @@ static void t_merged_iter_pqueue_top(void)merged_iter_pqueue_check (&pq);check (pq_entry_equal (&top, &e)); -check (reftable_record_equal (top.rec, &recs[i],GIT_SHA1_RAWSZ )); +check (reftable_record_equal (top.rec, &recs[i],REFTABLE_HASH_SIZE_SHA1 ));for (size_t j =0 ; i < pq.len; j++) {check (pq_less (&top, &pq.heap[j])); check_int(top.index, >, j);
t/unit-tests/t-reftable-reader.c +2 -2
@@ -31,7 +31,7 @@ static int t_reader_seek_once(void) ret =reftable_iterator_next_ref (&it, &ref);check (!ret); - ret =reftable_ref_record_equal (&ref, &records[0 ],GIT_SHA1_RAWSZ ); + ret =reftable_ref_record_equal (&ref, &records[0 ],REFTABLE_HASH_SIZE_SHA1 ); check_int(ret, ==, 1); ret =reftable_iterator_next_ref (&it, &ref);
@@ -74,7 +74,7 @@ static int t_reader_reseek(void) ret =reftable_iterator_next_ref (&it, &ref);check (!ret); - ret =reftable_ref_record_equal (&ref, &records[0 ],GIT_SHA1_RAWSZ ); + ret =reftable_ref_record_equal (&ref, &records[0 ],REFTABLE_HASH_SIZE_SHA1 ); check_int(ret, ==, 1); ret =reftable_iterator_next_ref (&it, &ref);
t/unit-tests/t-reftable-readwrite.c +20 -20
@@ -41,7 +41,7 @@ static void t_buffer(void) }static void write_table (char ***names,struct reftable_buf *buf,int N , -int block_size,uint32_t hash_id) +int block_size,enum reftable_hash hash_id) {struct reftable_write_options opts = { .block_size = block_size,
@@ -62,7 +62,7 @@ static void write_table(char ***names, struct reftable_buf *buf, int N, refs[i].refname = (*names)[i] =xstrfmt ("refs/heads/branch%02d" , i); refs[i].update_index = update_index; refs[i].value_type =REFTABLE_REF_VAL1 ; -t_reftable_set_hash (refs[i].value.val1, i,GIT_SHA1_FORMAT_ID ); +t_reftable_set_hash (refs[i].value.val1, i,REFTABLE_HASH_SHA1 ); }for (i =0 ; i <N ; i++) {
@@ -70,7 +70,7 @@ static void write_table(char ***names, struct reftable_buf *buf, int N, logs[i].update_index = update_index; logs[i].value_type =REFTABLE_LOG_UPDATE ;t_reftable_set_hash (logs[i].value.update.new_hash, i, -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 ); logs[i].value.update.message = (char *)"message" ; }
@@ -104,7 +104,7 @@ static void t_log_buffer_size(void)/* This tests buffer extension for log compression. Must use a random hash, to ensure that the compressed part is larger than the original. */ -for (i =0 ; i <GIT_SHA1_RAWSZ ; i++) { +for (i =0 ; i <REFTABLE_HASH_SIZE_SHA1 ; i++) { log.value.update.old_hash[i] = (uint8_t )(git_rand () %256 ); log.value.update.new_hash[i] = (uint8_t )(git_rand () %256 ); }
@@ -191,9 +191,9 @@ static void t_log_write_read(void) log.update_index = i; log.value_type =REFTABLE_LOG_UPDATE ;t_reftable_set_hash (log.value.update.old_hash, i, -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 );t_reftable_set_hash (log.value.update.new_hash, i +1 , -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 ); err =reftable_writer_add_log (w, &log);check (!err);
@@ -326,7 +326,7 @@ static void t_table_read_write_sequential(void)int err =0 ;int j =0 ; -write_table (&names, &buf,N ,256 ,GIT_SHA1_FORMAT_ID ); +write_table (&names, &buf,N ,256 ,REFTABLE_HASH_SHA1 );block_source_from_buf (&source, &buf);
@@ -361,7 +361,7 @@ static void t_table_write_small_table(void)char **names;struct reftable_buf buf =REFTABLE_BUF_INIT ;int N =1 ; -write_table (&names, &buf,N ,4096 ,GIT_SHA1_FORMAT_ID ); +write_table (&names, &buf,N ,4096 ,REFTABLE_HASH_SHA1 ); check_int(buf.len, <, 200);reftable_buf_release (&buf);free_names (names);
@@ -378,7 +378,7 @@ static void t_table_read_api(void)struct reftable_log_record log = {0 };struct reftable_iterator it = {0 }; -write_table (&names, &buf,N ,256 ,GIT_SHA1_FORMAT_ID ); +write_table (&names, &buf,N ,256 ,REFTABLE_HASH_SHA1 );block_source_from_buf (&source, &buf);
@@ -400,7 +400,7 @@ static void t_table_read_api(void)reftable_buf_release (&buf); } -static void t_table_read_write_seek (int index,int hash_id) +static void t_table_read_write_seek (int index,enum reftable_hash hash_id) {char **names;struct reftable_buf buf =REFTABLE_BUF_INIT ;
@@ -467,24 +467,24 @@ static void t_table_read_write_seek(int index, int hash_id)static void t_table_read_write_seek_linear (void ) { -t_table_read_write_seek (0 ,GIT_SHA1_FORMAT_ID ); +t_table_read_write_seek (0 ,REFTABLE_HASH_SHA1 ); }static void t_table_read_write_seek_linear_sha256 (void ) { -t_table_read_write_seek (0 ,GIT_SHA256_FORMAT_ID ); +t_table_read_write_seek (0 ,REFTABLE_HASH_SHA256 ); }static void t_table_read_write_seek_index (void ) { -t_table_read_write_seek (1 ,GIT_SHA1_FORMAT_ID ); +t_table_read_write_seek (1 ,REFTABLE_HASH_SHA1 ); }static void t_table_refs_for (int indexed) {char **want_names;int want_names_len =0 ; -uint8_t want_hash[GIT_SHA1_RAWSZ ]; +uint8_t want_hash[REFTABLE_HASH_SIZE_SHA1 ];struct reftable_write_options opts = { .block_size =256 ,
@@ -500,10 +500,10 @@ static void t_table_refs_for(int indexed) want_names =reftable_calloc (N +1 ,sizeof (*want_names));check (want_names !=NULL ); -t_reftable_set_hash (want_hash,4 ,GIT_SHA1_FORMAT_ID ); +t_reftable_set_hash (want_hash,4 ,REFTABLE_HASH_SHA1 );for (i =0 ; i <N ; i++) { -uint8_t hash[GIT_SHA1_RAWSZ ]; +uint8_t hash[REFTABLE_HASH_SIZE_SHA1 ];char fill[51 ] = {0 };char name[100 ];struct reftable_ref_record ref = {0 };
@@ -517,9 +517,9 @@ static void t_table_refs_for(int indexed) ref.value_type =REFTABLE_REF_VAL2 ;t_reftable_set_hash (ref.value.val2.value, i /4 , -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 );t_reftable_set_hash (ref.value.val2.target_value,3 + i /4 , -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 );/* 80 bytes / entry, so 3 entries per block. Yields 17 */
@@ -527,8 +527,8 @@ static void t_table_refs_for(int indexed) n =reftable_writer_add_ref (w, &ref); check_int(n, ==, 0); -if (!memcmp (ref.value.val2.value, want_hash,GIT_SHA1_RAWSZ ) || - !memcmp (ref.value.val2.target_value, want_hash,GIT_SHA1_RAWSZ )) +if (!memcmp (ref.value.val2.value, want_hash,REFTABLE_HASH_SIZE_SHA1 ) || + !memcmp (ref.value.val2.target_value, want_hash,REFTABLE_HASH_SIZE_SHA1 )) want_names[want_names_len++] =xstrdup (name); }
t/unit-tests/t-reftable-record.c +30 -29
175 lines not on this page · raw message
t/unit-tests/t-reftable-stack.c +17 -17
@@ -121,7 +121,7 @@ static void write_n_ref_tables(struct reftable_stack *st,snprintf (buf,sizeof (buf),"refs/heads/branch-%04" PRIuMAX, (uintmax_t )i); ref.refname = buf; -t_reftable_set_hash (ref.value.val1, i,GIT_SHA1_FORMAT_ID ); +t_reftable_set_hash (ref.value.val1, i,REFTABLE_HASH_SHA1 ); err =reftable_stack_add (st, &write_test_ref, &ref);check (!err);
@@ -169,7 +169,7 @@ static void t_reftable_stack_add_one(void) err =reftable_stack_read_ref (st, ref.refname, &dest);check (!err); -check (reftable_ref_record_equal (&ref, &dest,GIT_SHA1_RAWSZ )); +check (reftable_ref_record_equal (&ref, &dest,REFTABLE_HASH_SIZE_SHA1 )); check_int(st->readers_len, >, 0);#ifndef GIT_WINDOWS_NATIVE
@@ -280,7 +280,7 @@ static void t_reftable_stack_transaction_api(void) err =reftable_stack_read_ref (st, ref.refname, &dest);check (!err); check_int(REFTABLE_REF_SYMREF, ==, dest.value_type); -check (reftable_ref_record_equal (&ref, &dest,GIT_SHA1_RAWSZ )); +check (reftable_ref_record_equal (&ref, &dest,REFTABLE_HASH_SIZE_SHA1 ));reftable_ref_record_release (&dest);reftable_stack_destroy (st);
@@ -340,7 +340,7 @@ static void t_reftable_stack_transaction_with_reload(void)for (size_t i =0 ; i <ARRAY_SIZE (refs); i++) { err =reftable_stack_read_ref (st2, refs[i].refname, &ref);check (!err); -check (reftable_ref_record_equal (&refs[i], &ref,GIT_SHA1_RAWSZ )); +check (reftable_ref_record_equal (&refs[i], &ref,REFTABLE_HASH_SIZE_SHA1 )); }reftable_ref_record_release (&ref);
@@ -530,13 +530,13 @@ static void t_reftable_stack_add(void) refs[i].refname =xstrdup (buf); refs[i].update_index = i +1 ; refs[i].value_type =REFTABLE_REF_VAL1 ; -t_reftable_set_hash (refs[i].value.val1, i,GIT_SHA1_FORMAT_ID ); +t_reftable_set_hash (refs[i].value.val1, i,REFTABLE_HASH_SHA1 ); logs[i].refname =xstrdup (buf); logs[i].update_index =N + i +1 ; logs[i].value_type =REFTABLE_LOG_UPDATE ; logs[i].value.update.email =xstrdup ("identity@invalid" ); -t_reftable_set_hash (logs[i].value.update.new_hash, i,GIT_SHA1_FORMAT_ID ); +t_reftable_set_hash (logs[i].value.update.new_hash, i,REFTABLE_HASH_SHA1 ); }for (i =0 ; i <N ; i++) {
@@ -562,7 +562,7 @@ static void t_reftable_stack_add(void)int err =reftable_stack_read_ref (st, refs[i].refname, &dest);check (!err);check (reftable_ref_record_equal (&dest, refs + i, -GIT_SHA1_RAWSZ )); +REFTABLE_HASH_SIZE_SHA1 ));reftable_ref_record_release (&dest); }
@@ -571,7 +571,7 @@ static void t_reftable_stack_add(void)int err =reftable_stack_read_log (st, refs[i].refname, &dest);check (!err);check (reftable_log_record_equal (&dest, logs + i, -GIT_SHA1_RAWSZ )); +REFTABLE_HASH_SIZE_SHA1 ));reftable_log_record_release (&dest); }
@@ -622,14 +622,14 @@ static void t_reftable_stack_iterator(void) refs[i].refname =xstrfmt ("branch%02" PRIuMAX, (uintmax_t )i); refs[i].update_index = i +1 ; refs[i].value_type =REFTABLE_REF_VAL1 ; -t_reftable_set_hash (refs[i].value.val1, i,GIT_SHA1_FORMAT_ID ); +t_reftable_set_hash (refs[i].value.val1, i,REFTABLE_HASH_SHA1 ); logs[i].refname =xstrfmt ("branch%02" PRIuMAX, (uintmax_t )i); logs[i].update_index = i +1 ; logs[i].value_type =REFTABLE_LOG_UPDATE ; logs[i].value.update.email =xstrdup ("johndoe@invalid" ); logs[i].value.update.message =xstrdup ("commit\n" ); -t_reftable_set_hash (logs[i].value.update.new_hash, i,GIT_SHA1_FORMAT_ID ); +t_reftable_set_hash (logs[i].value.update.new_hash, i,REFTABLE_HASH_SHA1 ); }for (i =0 ; i <N ; i++) {
@@ -656,7 +656,7 @@ static void t_reftable_stack_iterator(void)if (err >0 )break ;check (!err); -check (reftable_ref_record_equal (&ref, &refs[i],GIT_SHA1_RAWSZ )); +check (reftable_ref_record_equal (&ref, &refs[i],REFTABLE_HASH_SIZE_SHA1 ));reftable_ref_record_release (&ref); } check_int(i, ==, N);
@@ -674,7 +674,7 @@ static void t_reftable_stack_iterator(void)if (err >0 )break ;check (!err); -check (reftable_log_record_equal (&log, &logs[i],GIT_SHA1_RAWSZ )); +check (reftable_log_record_equal (&log, &logs[i],REFTABLE_HASH_SIZE_SHA1 ));reftable_log_record_release (&log); } check_int(i, ==, N);
@@ -767,7 +767,7 @@ static void t_reftable_stack_tombstone(void)if (i %2 ==0 ) { refs[i].value_type =REFTABLE_REF_VAL1 ;t_reftable_set_hash (refs[i].value.val1, i, -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 ); } logs[i].refname =xstrdup (buf);
@@ -776,7 +776,7 @@ static void t_reftable_stack_tombstone(void)if (i %2 ==0 ) { logs[i].value_type =REFTABLE_LOG_UPDATE ;t_reftable_set_hash (logs[i].value.update.new_hash, i, -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 ); logs[i].value.update.email =xstrdup ("identity@invalid" ); }
@@ -836,7 +836,7 @@ static void t_reftable_stack_hash_id(void) .value.symref = (char *) "target", .update_index = 1, }; -struct reftable_write_options opts32 = { .hash_id =GIT_SHA256_FORMAT_ID }; +struct reftable_write_options opts32 = { .hash_id =REFTABLE_HASH_SHA256 };struct reftable_stack *st32 =NULL ;struct reftable_write_options opts_default = {0 };struct reftable_stack *st_default =NULL ;
@@ -859,7 +859,7 @@ static void t_reftable_stack_hash_id(void) err =reftable_stack_read_ref (st_default,"master" , &dest);check (!err); -check (reftable_ref_record_equal (&ref, &dest,GIT_SHA1_RAWSZ )); +check (reftable_ref_record_equal (&ref, &dest,REFTABLE_HASH_SIZE_SHA1 ));reftable_ref_record_release (&dest);reftable_stack_destroy (st);reftable_stack_destroy (st_default);
@@ -909,7 +909,7 @@ static void t_reflog_expire(void) logs[i].value.update.time = i; logs[i].value.update.email =xstrdup ("identity@invalid" );t_reftable_set_hash (logs[i].value.update.new_hash, i, -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 ); }for (i =1 ; i <=N ; i++) {
Signature · 1 line
2.47.0.229.g8f8d6eee53.dirty
Re: [PATCH v2 3/7] reftable/system: stop depending on "hash.h"
Quoted 20 lines · Patrick Steinhardt
refs/reftable-backend.c +4
-1@@ -15,6 +15,7 @@#include "../object.h" #include "../path.h" #include "../refs.h" +#include "../reftable/reftable-basics.h" #include "../reftable/reftable-stack.h" #include "../reftable/reftable-record.h" #include "../reftable/reftable-error.h" @@ -289,7 +290,16 @@ static struct ref_store *reftable_be_init(struct repository *repo, refs->store_flags = store_flags; refs->log_all_ref_updates =repo_settings_get_log_all_ref_updates (repo); - refs->write_options.hash_id = repo->hash_algo->format_id; +switch (repo->hash_algo->format_id) { +case GIT_SHA1_FORMAT_ID : + refs->write_options.hash_id =REFTABLE_HASH_SHA1 ;
refs/reftable-backend.c static struct ref_store *reftable_be_init(struct repository *repo, · line 305
Quoted 10 lines
refs/reftable-backend.c +7
-0+break ; +case GIT_SHA256_FORMAT_ID : + refs->write_options.hash_id =REFTABLE_HASH_SHA256 ; +break ; +default : +BUG ("unknown hash algorithm %d" , repo->hash_algo->format_id); + } refs->write_options.default_permissions =calc_shared_perm (0666 & ~mask); refs->write_options.disable_auto_compact = !git_env_bool ("GIT_TEST_REFTABLE_AUTOCOMPACTION" ,1 );
OK. With this step, together with the previous one, we let Git side to use GIT_SHA{1,256}_FORMAT_ID to specify what algorithm is in use in the repository, and reftable-backend.c layer is responsible for translating them into REFTABLE_HASH_SHA{1,256}, which is internally used in reftable library to convey the same thing.
Which makes sense.
Quoted 7 lines
reftable/reftable-basics.h +0
-0@@ -11,6 +11,19 @@#include <stddef.h>
reftable/reftable-basics.h line 22
Quoted 10 lines
reftable/reftable-basics.h +9
-0+/* +* Hash functions understood by the reftable library. Note that the values are +* arbitrary and somewhat random such that we can easily detect cases where the +* hash hasn't been properly set up. +*/ +enum reftable_hash { +REFTABLE_HASH_SHA1 =89 , +REFTABLE_HASH_SHA256 =247 , +};
;-).
reftable/reftable-basics.h line 29
Quoted 7 lines
reftable/reftable-basics.h +4
-0+#define REFTABLE_HASH_SIZE_SHA1 20 +#define REFTABLE_HASH_SIZE_SHA256 32 +#define REFTABLE_HASH_SIZE_MAX REFTABLE_HASH_SIZE_SHA256 +/* Overrides the functions to use for memory management. */ void reftable_set_alloc (void *(*malloc)(size_t ),void *(*realloc)(void *,size_t ),void (*free)(void *));
OK.
Re: [PATCH v2 3/7] reftable/system: stop depending on "hash.h"
In reply to Patrick SteinhardtQuoted 9 lines · Patrick Steinhardt
We include "
hash.h" in "reftable/system.h" such that we can use hash format IDs as well as the raw size of SHA1 and SHA256. As we are in the process of converting the reftable library to become standalone we of course cannot rely on those constants anymore.Introduce a new
enum reftable_hashto replace internal uses of the hash format IDs and new constants that replace internal uses of the hash size. Adapt the reftable backend to set up the correct hash function.
So with this patch, we now have:
- REFTABLE_FORMAT_ID_SHA1 & REFTABLE_FORMAT_ID_SHA256 which is what is stored on disk and used while reading/writing.
- enum reftable_hash which is used to identify the hash function internally in the reftable code.
Both of which are separated. Nice!
- Karthik
PATCH v2 4/7 reftable/stack: stop using `fsync_component()` directly
In reply to Patrick SteinhardtWe're executing fsync_component() directly in the reftable library so that we can fsync data to disk depending on "core.fsync". But as we're in the process of converting the reftable library to become standalone we cannot use that function in the library anymore.
Refactor the code such that users of the library can inject a custom fsync function via the write options. This allows us to get rid of the dependency on "write-or-die.h".
--- refs/reftable-backend.c | 7 ++++++ reftable/reftable-writer.h | 6 +++++ reftable/stack.c | 49 +++++++++++++++++++++++++------------- 3 files changed, 45 insertions(+), 17 deletions(-)
refs/reftable-backend.c +7 -0
@@ -24,6 +24,7 @@#include "../setup.h" #include "../strmap.h" #include "../trace2.h" +#include "../write-or-die.h" #include "parse.h" #include "refs-internal.h"
@@ -273,6 +274,11 @@ static int reftable_be_config(const char *var, const char *value,return 0 ; } +static int reftable_be_fsync (int fd) +{ +return fsync_component (FSYNC_COMPONENT_REFERENCE , fd); +} +static struct ref_store *reftable_be_init (struct repository *repo,const char *gitdir,unsigned int store_flags)
@@ -304,6 +310,7 @@ static struct ref_store *reftable_be_init(struct repository *repo, refs->write_options.disable_auto_compact = !git_env_bool ("GIT_TEST_REFTABLE_AUTOCOMPACTION" ,1 ); refs->write_options.lock_timeout_ms =100 ; + refs->write_options.fsync = reftable_be_fsync;git_config (reftable_be_config, &refs->write_options);
reftable/reftable-writer.h +6 -0
@@ -62,6 +62,12 @@ struct reftable_write_options {* negative value will cause us to block indefinitely. */ long lock_timeout_ms; + +/* +* Optional callback used to fsync files to disk. Falls back to using +* fsync(3P) when unset. +*/ +int (*fsync)(int fd); };/* reftable_block_stats holds statistics for a single block type */
reftable/stack.c +32 -17
111 lines not on this page · raw message
Signature · 1 line
2.47.0.229.g8f8d6eee53.dirty
Re: [PATCH v2 4/7] reftable/stack: stop using `fsync_component()` directly
Patrick Steinhardt <[email protected]> writes:
[snip]
Quoted 15 lines
reftable/stack.c +4
-4@@ -43,17 +42,28 @@ static int stack_filename(struct reftable_buf *dest, struct reftable_stack *st,return 0 ; } -static ssize_t reftable_fd_write (void *arg,const void *data,size_t sz) +static int stack_fsync (const struct reftable_write_options *opts,int fd) { -int *fdp = (int *)arg; -return write_in_full (*fdp, data, sz); +if (opts->fsync) +return opts->fsync (fd); +return fsync (fd); } -static int reftable_fd_flush (void *arg)
reftable/stack.c static int stack_filename(struct reftable_buf *dest, struct reftable_stack *st, · line 60
Quoted 10 lines
reftable/stack.c +8
-1+struct fd_writer { +const struct reftable_write_options *opts; +int fd; +}; + +static ssize_t fd_writer_write (void *arg,const void *data,size_t sz) { -int *fdp = (int *)arg; +struct fd_writer *writer = arg; +return write_in_full (writer->fd, data, sz);
Tangent: eventually we'd also want to get rid of write_in_full no? Since it is also an internal Git implementation.
The patch itself looks good to me.
[snip]
- Karthik
Re: [PATCH v2 4/7] reftable/stack: stop using `fsync_component()` directly
Quoted 30 lines · karthik nayak
Quoted 15 lines · Patrick Steinhardt
reftable/stack.c +4
-4@@ -43,17 +42,28 @@ static int stack_filename(struct reftable_buf *dest, struct reftable_stack *st,return 0 ; } -static ssize_t reftable_fd_write (void *arg,const void *data,size_t sz) +static int stack_fsync (const struct reftable_write_options *opts,int fd) { -int *fdp = (int *)arg; -return write_in_full (*fdp, data, sz); +if (opts->fsync) +return opts->fsync (fd); +return fsync (fd); } -static int reftable_fd_flush (void *arg)reftable/stack.c static int stack_filename(struct reftable_buf *dest, struct reftable_stack *st, · line 60
Quoted 10 lines
reftable/stack.c +8
-1+struct fd_writer { +const struct reftable_write_options *opts; +int fd; +}; + +static ssize_t fd_writer_write (void *arg,const void *data,size_t sz) { -int *fdp = (int *)arg; +struct fd_writer *writer = arg; +return write_in_full (writer->fd, data, sz);Tangent: eventually we'd also want to get rid of
write_in_fullno? Since it is also an internal Git implementation.
Quoted 1 line
The patch itself looks good to me.
Yup. This is part of the next and final patch series that I'll send once this series here has landed.
Patrick
PATCH v2 5/7 reftable/system: provide thin wrapper for tempfile subsystem
In reply to Patrick SteinhardtWe use the tempfile subsystem to write temporary tables, but given that we're in the process of converting the reftable library to become standalone we cannot use this subsystem directly anymore. While we could in theory convert the code to use mkstemp(3p) instead, we'd lose access to our infrastructure that automatically prunes tempfiles via atexit(3p) or signal handlers.
Provide a thin wrapper for the tempfile subsystem instead. Like this, the compatibility shim is fully self-contained in "reftable/system.c". Downstream users of the reftable library would have to implement their own tempfile shims by replacing "system.c" with a custom version.
--- Makefile | 1 + reftable/stack.c | 57 +++++++++++++++++++---------------------------- reftable/system.c | 49 ++++++++++++++++++++++++++++++++++++++++ reftable/system.h | 41 +++++++++++++++++++++++++++++++++- 4 files changed, 113 insertions(+), 35 deletions(-) create mode 100644 reftable/system.c
Makefile +1 -0
@@ -2722,6 +2722,7 @@ REFTABLE_OBJS += reftable/pq.o REFTABLE_OBJS += reftable/reader.o REFTABLE_OBJS += reftable/record.o REFTABLE_OBJS += reftable/stack.o +REFTABLE_OBJS += reftable/system.o REFTABLE_OBJS += reftable/tree.o REFTABLE_OBJS += reftable/writer.o
reftable/stack.c +23 -34
159 lines not on this page · raw message
reftable/system.c new file +49 -0
49 lines not on this page · raw message
reftable/system.h +40 -1
46 lines not on this page · raw message
Signature · 1 line
2.47.0.229.g8f8d6eee53.dirty
Re: [PATCH v2 5/7] reftable/system: provide thin wrapper for tempfile subsystem
Patrick Steinhardt <[email protected]> writes:
[snip]
reftable/system.c line 25
Quoted 6 lines
reftable/system.c new file +6
-0+ +int tmpfile_close (struct reftable_tmpfile *t) +{ +struct tempfile *tempfile = t->priv; +int ret =close_tempfile_gently (tempfile); + t->fd =-1 ;
Nit: below we do *t = REFTABLE_TMPFILE_INIT should we keep things consistent and do the same here?
Quoted 5 lines
reftable/system.c new file +5
-0+if (ret <0 ) +return REFTABLE_IO_ERROR ; +return 0 ; +} +
[snip]
reftable/system.h license that can be found in the LICENSE file or at · line 54
Quoted 11 lines
reftable/system.h +11
-0+/* +* Close the temporary file and delete it. This is a no-op in case the file has +* already been deleted or renamed beforehand. Returns 0 on success, a reftable +* error code on error. +*/ +int tmpfile_delete (struct reftable_tmpfile *t); + +/* +* Rename the temporary file to the provided path. The temporary file must be +* active. Return 0 on success, a reftable error code on error. +*/
Nit: would be nice to mention that the t is reset here.
Quoted 5 lines
+int
tmpfile_rename(struct reftable_tmpfile *t, const char *path);#endif
--
2.47.0.229.g8f8d6eee53.dirty
Re: [PATCH v2 5/7] reftable/system: provide thin wrapper for tempfile subsystem
Quoted 4 lines · karthik nayak
Patrick Steinhardt <[email protected]> writes:
[snip]
Quoted 10 lines
reftable/system.c line 25
Quoted 6 lines
reftable/system.c new file +6
-0+ +int tmpfile_close (struct reftable_tmpfile *t) +{ +struct tempfile *tempfile = t->priv; +int ret =close_tempfile_gently (tempfile); + t->fd =-1 ;Nit: below we do
*t = REFTABLE_TMPFILE_INITshould we keep things consistent and do the same here?
My bad, we want this to still hold onto t->priv which is cleared when we call tmpfile_delete(). So all good here.
PATCH v2 6/7 reftable/stack: drop only use of `get_locked_file_path()`
In reply to Patrick SteinhardtWe've got a single callsite where we call get_locked_file_path(). As we're about to convert our usage of the lockfile subsystem to instead be used via a compatibility shim we'd have to implement more logic for this single callsite. While that would be okay if Git was the only supposed user of the reftable library, it's a bit more awkward when considering that we have to reimplement this functionality for every user of the library eventually.
Refactor the code such that we don't call get_locked_file_path() anymore.
--- reftable/stack.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
reftable/stack.c +9 -3
@@ -1493,9 +1493,15 @@ static int stack_compact_range(struct reftable_stack *st,*/ for (i =0 ; i < nlocks; i++) {struct lock_file *table_lock = &table_locks[i]; -char *table_path =get_locked_file_path (table_lock); -unlink (table_path); -reftable_free (table_path); +const char *lock_path =get_lock_file_path (table_lock); + +reftable_buf_reset (&table_name); + err =reftable_buf_add (&table_name, lock_path, +strlen (lock_path) -strlen (".lock" )); +if (err) +continue ; + +unlink (table_name.buf); } done:
Signature · 1 line
2.47.0.229.g8f8d6eee53.dirty
PATCH v2 7/7 reftable/system: provide thin wrapper for lockfile subsystem
In reply to Patrick SteinhardtWe use the lockfile subsystem to write lockfiles for "tables.list". As with the tempfile subsystem, the lockfile subsystem also hooks into our infrastructure to prune stale locks via atexit(3p) or signal handlers.
Furthermore, the lockfile subsystem also handles locking timeouts, which do add quite a bit of logic. Having to reimplement that in the context of Git wouldn't make a whole lot of sense, and it is quite likely that downstream users of the reftable library may have a better idea for how exactly to implement timeouts.
So again, provide a thin wrapper for the lockfile subsystem instead such that the compatibility shim is fully self-contained.
--- reftable/stack.c | 63 ++++++++++------------- reftable/system.c | 77 +++++++++++++++++++++++++++++ reftable/system.h | 45 ++++++++++++++++- t/unit-tests/lib-reftable.c | 1 + t/unit-tests/t-reftable-block.c | 1 + t/unit-tests/t-reftable-pq.c | 1 + t/unit-tests/t-reftable-readwrite.c | 1 + t/unit-tests/t-reftable-stack.c | 2 + 8 files changed, 154 insertions(+), 37 deletions(-)
reftable/stack.c +27 -36
@@ -657,7 +657,7 @@ static int format_name(struct reftable_buf *dest, uint64_t min, uint64_t max) }struct reftable_addition { -struct lock_file tables_list_lock; +struct reftable_flock tables_list_lock;struct reftable_stack *stack;char **new_tables;
@@ -676,10 +676,8 @@ static int reftable_stack_init_addition(struct reftable_addition *add, add->stack = st; - err =hold_lock_file_for_update_timeout (&add->tables_list_lock, - st->list_file, -LOCK_NO_DEREF , - st->opts.lock_timeout_ms); + err =flock_acquire (&add->tables_list_lock, st->list_file, + st->opts.lock_timeout_ms);if (err <0 ) {if (errno ==EEXIST ) { err =REFTABLE_LOCK_ERROR ;
@@ -689,7 +687,7 @@ static int reftable_stack_init_addition(struct reftable_addition *add, goto done; }if (st->opts.default_permissions) { -if (chmod (get_lock_file_path (&add->tables_list_lock), +if (chmod (add->tables_list_lock.path, st->opts.default_permissions) <0 ) { err =REFTABLE_IO_ERROR ; goto done;
@@ -733,7 +731,7 @@ static void reftable_addition_close(struct reftable_addition *add) add->new_tables_len =0 ; add->new_tables_cap =0 ; -rollback_lock_file (&add->tables_list_lock); +flock_release (&add->tables_list_lock);reftable_buf_release (&nm); }
@@ -749,7 +747,6 @@ void reftable_addition_destroy(struct reftable_addition *add)int reftable_addition_commit (struct reftable_addition *add) {struct reftable_buf table_list =REFTABLE_BUF_INIT ; -int lock_file_fd =get_lock_file_fd (&add->tables_list_lock);int err =0 ;size_t i;
@@ -767,20 +764,20 @@ int reftable_addition_commit(struct reftable_addition *add) goto done; } - err =write_in_full (lock_file_fd, table_list.buf, table_list.len); + err =write_in_full (add->tables_list_lock.fd, table_list.buf, table_list.len);reftable_buf_release (&table_list);if (err <0 ) { err =REFTABLE_IO_ERROR ; goto done; } - err =stack_fsync (&add->stack->opts, lock_file_fd); + err =stack_fsync (&add->stack->opts, add->tables_list_lock.fd);if (err <0 ) { err =REFTABLE_IO_ERROR ; goto done; } - err =commit_lock_file (&add->tables_list_lock); + err =flock_commit (&add->tables_list_lock);if (err <0 ) { err =REFTABLE_IO_ERROR ; goto done;
@@ -1160,8 +1157,8 @@ static int stack_compact_range(struct reftable_stack *st,struct reftable_buf new_table_name =REFTABLE_BUF_INIT ;struct reftable_buf new_table_path =REFTABLE_BUF_INIT ;struct reftable_buf table_name =REFTABLE_BUF_INIT ; -struct lock_file tables_list_lock =LOCK_INIT ; -struct lock_file *table_locks =NULL ; +struct reftable_flock tables_list_lock =REFTABLE_FLOCK_INIT ; +struct reftable_flock *table_locks =NULL ;struct reftable_tmpfile new_table =REFTABLE_TMPFILE_INIT ;int is_empty_table =0 , err =0 ;size_t first_to_replace, last_to_replace;
@@ -1179,10 +1176,7 @@ static int stack_compact_range(struct reftable_stack *st,* Hold the lock so that we can read "tables.list" and lock all tables * which are part of the user-specified range. */ - err =hold_lock_file_for_update_timeout (&tables_list_lock, - st->list_file, -LOCK_NO_DEREF , - st->opts.lock_timeout_ms); + err =flock_acquire (&tables_list_lock, st->list_file, st->opts.lock_timeout_ms);if (err <0 ) {if (errno ==EEXIST ) err =REFTABLE_LOCK_ERROR ;
@@ -1205,19 +1199,20 @@ static int stack_compact_range(struct reftable_stack *st,* older process is still busy compacting tables which are preexisting * from the point of view of the newer process. */ -REFTABLE_CALLOC_ARRAY (table_locks, last - first +1 ); +REFTABLE_ALLOC_ARRAY (table_locks, last - first +1 );if (!table_locks) { err =REFTABLE_OUT_OF_MEMORY_ERROR ; goto done; } +for (i =0 ; i < last - first +1 ; i++) + table_locks[i] =REFTABLE_FLOCK_INIT ;for (i = last +1 ; i > first; i--) { err =stack_filename (&table_name, st,reader_name (st->readers[i -1 ]));if (err <0 ) goto done; - err =hold_lock_file_for_update (&table_locks[nlocks], - table_name.buf,LOCK_NO_DEREF ); + err =flock_acquire (&table_locks[nlocks], table_name.buf,0 );if (err <0 ) {/* * When the table is locked already we may do a
@@ -1253,7 +1248,7 @@ static int stack_compact_range(struct reftable_stack *st,* run into file descriptor exhaustion when we compress a lot * of tables. */ - err =close_lock_file_gently (&table_locks[nlocks++]); + err =flock_close (&table_locks[nlocks++]);if (err <0 ) { err =REFTABLE_IO_ERROR ; goto done;
@@ -1265,7 +1260,7 @@ static int stack_compact_range(struct reftable_stack *st,* "tables.list" lock while compacting the locked tables. This allows * concurrent updates to the stack to proceed. */ - err =rollback_lock_file (&tables_list_lock); + err =flock_release (&tables_list_lock);if (err <0 ) { err =REFTABLE_IO_ERROR ; goto done;
@@ -1288,10 +1283,7 @@ static int stack_compact_range(struct reftable_stack *st,* "tables.list". We'll then replace the compacted range of tables with * the new table. */ - err =hold_lock_file_for_update_timeout (&tables_list_lock, - st->list_file, -LOCK_NO_DEREF , - st->opts.lock_timeout_ms); + err =flock_acquire (&tables_list_lock, st->list_file, st->opts.lock_timeout_ms);if (err <0 ) {if (errno ==EEXIST ) err =REFTABLE_LOCK_ERROR ;
@@ -1301,7 +1293,7 @@ static int stack_compact_range(struct reftable_stack *st, }if (st->opts.default_permissions) { -if (chmod (get_lock_file_path (&tables_list_lock), +if (chmod (tables_list_lock.path, st->opts.default_permissions) <0 ) { err =REFTABLE_IO_ERROR ; goto done;
@@ -1456,7 +1448,7 @@ static int stack_compact_range(struct reftable_stack *st, goto done; } - err =write_in_full (get_lock_file_fd (&tables_list_lock), + err =write_in_full (tables_list_lock.fd, tables_list_buf.buf, tables_list_buf.len);if (err <0 ) { err =REFTABLE_IO_ERROR ;
@@ -1464,14 +1456,14 @@ static int stack_compact_range(struct reftable_stack *st, goto done; } - err =stack_fsync (&st->opts,get_lock_file_fd (&tables_list_lock)); + err =stack_fsync (&st->opts, tables_list_lock.fd);if (err <0 ) { err =REFTABLE_IO_ERROR ;unlink (new_table_path.buf); goto done; } - err =commit_lock_file (&tables_list_lock); + err =flock_commit (&tables_list_lock);if (err <0 ) { err =REFTABLE_IO_ERROR ;unlink (new_table_path.buf);
@@ -1492,12 +1484,11 @@ static int stack_compact_range(struct reftable_stack *st,* readers, so it is expected that unlinking tables may fail. */ for (i =0 ; i < nlocks; i++) { -struct lock_file *table_lock = &table_locks[i]; -const char *lock_path =get_lock_file_path (table_lock); +struct reftable_flock *table_lock = &table_locks[i];reftable_buf_reset (&table_name); - err =reftable_buf_add (&table_name, lock_path, -strlen (lock_path) -strlen (".lock" )); + err =reftable_buf_add (&table_name, table_lock->path, +strlen (table_lock->path) -strlen (".lock" ));if (err)continue ;
@@ -1505,9 +1496,9 @@ static int stack_compact_range(struct reftable_stack *st, } done: -rollback_lock_file (&tables_list_lock); +flock_release (&tables_list_lock);for (i =0 ; table_locks && i < nlocks; i++) -rollback_lock_file (&table_locks[i]); +flock_release (&table_locks[i]);reftable_free (table_locks);tmpfile_delete (&new_table);
reftable/system.c +77 -0
86 lines not on this page · raw message
reftable/system.h +44 -1
55 lines not on this page · raw message
t/unit-tests/lib-reftable.c +1 -0
@@ -2,6 +2,7 @@#include "test-lib.h" #include "reftable/constants.h" #include "reftable/writer.h" +#include "strbuf.h" void t_reftable_set_hash (uint8_t *p,int i,enum reftable_hash id) {
t/unit-tests/t-reftable-block.c +1 -0
@@ -11,6 +11,7 @@ license that can be found in the LICENSE file or at#include "reftable/blocksource.h" #include "reftable/constants.h" #include "reftable/reftable-error.h" +#include "strbuf.h" static void t_ref_block_read_write (void ) {
t/unit-tests/t-reftable-pq.c +1 -0
@@ -9,6 +9,7 @@ license that can be found in the LICENSE file or at#include "test-lib.h" #include "reftable/constants.h" #include "reftable/pq.h" +#include "strbuf.h" static void merged_iter_pqueue_check (const struct merged_iter_pqueue *pq) {
t/unit-tests/t-reftable-readwrite.c +1 -0
@@ -13,6 +13,7 @@ license that can be found in the LICENSE file or at#include "reftable/reader.h" #include "reftable/reftable-error.h" #include "reftable/reftable-writer.h" +#include "strbuf.h" static const int update_index =5 ;
t/unit-tests/t-reftable-stack.c +2 -0
@@ -13,6 +13,8 @@ license that can be found in the LICENSE file or at#include "reftable/reader.h" #include "reftable/reftable-error.h" #include "reftable/stack.h" +#include "strbuf.h" +#include "tempfile.h" #include <dirent.h> static void clear_dir (const char *dirname)
Signature · 1 line
2.47.0.229.g8f8d6eee53.dirty
Quoted 11 lines · Patrick Steinhardt
Hi,
this is the second version of my patch series that continues to detangle the reftable library from the Git codebase.
Changes compared to v1:
- Fix a commit message typo.
- Document the values of the newly introduced reftable format IDs.
Quoted 8 lines
- Include "
reftable-basics.h" instead of "basics.h".- Adapt
stack_fsync()to take write options as input instead of the whole stack.Thanks!
Patrick
I've reviewed the range-diff and this version looks good to me. :)
-Justin
Quoted 14 lines · Justin Tobler
Quoted 13 lines · Patrick Steinhardt
Hi,
this is the second version of my patch series that continues to detangle the reftable library from the Git codebase.
Changes compared to v1:
- Fix a commit message typo.
- Document the values of the newly introduced reftable format IDs.
- Include "
reftable-basics.h" instead of "basics.h".
Quoted 8 lines
Quoted 6 lines
- Adapt
stack_fsync()to take write options as input instead of the whole stack.Thanks!
Patrick
I've reviewed the range-diff and this version looks good to me. :)
Thanks, both of you.
Quoted 9 lines · Patrick Steinhardt
Hi,
this is the second version of my patch series that continues to detangle the reftable library from the Git codebase.
Changes compared to v1:
- Fix a commit message typo.
Quoted 9 lines
- Document the values of the newly introduced reftable format IDs.
- Include "
reftable-basics.h" instead of "basics.h".- Adapt
stack_fsync()to take write options as input instead of the whole stack.Thanks!
[snip]
I went through the patches and I think it looks great!
- Karthik
Quoted 18 lines · karthik nayak
Quoted 16 lines · Patrick Steinhardt
Hi,
this is the second version of my patch series that continues to detangle the reftable library from the Git codebase.
Changes compared to v1:
- Fix a commit message typo.
- Document the values of the newly introduced reftable format IDs.
- Include "
reftable-basics.h" instead of "basics.h".- Adapt
stack_fsync()to take write options as input instead of the whole stack.
Quoted 6 lines
Quoted 2 lines
Thanks!
[snip]
I went through the patches and I think it looks great!
Thanks for your review!
Patrick
v3 cover reftable: stop using Git subsystems
In reply to Patrick SteinhardtHi,
this is the third version of my patch series that continues to detangle the reftable library from the Git codebase.
There is only a single change compared to v2, which is a clarification that tmpfile_rename() also deactivates the underlying tmpfile.
Thanks!
Patrick
Patrick Steinhardt (7):
reftable/system: move "dir.h" to its only user
reftable: explicitly handle hash format IDs reftable/system: stop depending on "hash.h" reftable/stack: stop using fsync_component() directly reftable/system: provide thin wrapper for tempfile subsystem reftable/stack: drop only use of get_locked_file_path() reftable/system: provide thin wrapper for lockfile subsystem
Makefile | 1 + refs/reftable-backend.c | 19 +++- reftable/basics.c | 13 ++- reftable/basics.h | 10 +- reftable/merged.c | 4 +- reftable/merged.h | 3 +- reftable/reader.c | 14 ++- reftable/reader.h | 4 +- reftable/reftable-basics.h | 13 +++ reftable/reftable-merged.h | 4 +- reftable/reftable-reader.h | 2 +- reftable/reftable-record.h | 12 +- reftable/reftable-writer.h | 8 +- reftable/stack.c | 171 ++++++++++++++-------------- reftable/system.c | 126 ++++++++++++++++++++ reftable/system.h | 89 ++++++++++++++- reftable/writer.c | 20 +++- t/helper/test-reftable.c | 4 +- t/unit-tests/lib-reftable.c | 5 +- t/unit-tests/lib-reftable.h | 2 +- t/unit-tests/t-reftable-block.c | 41 +++---- t/unit-tests/t-reftable-merged.c | 26 ++--- t/unit-tests/t-reftable-pq.c | 3 +- t/unit-tests/t-reftable-reader.c | 4 +- t/unit-tests/t-reftable-readwrite.c | 41 +++---- t/unit-tests/t-reftable-record.c | 59 +++++----- t/unit-tests/t-reftable-stack.c | 37 +++--- 27 files changed, 506 insertions(+), 229 deletions(-) create mode 100644 reftable/system.c
Range-diff against v2:
1: 2b7d4e28529 = 1: 2b7d4e28529 reftable/system: move "dir.h" to its only user 2: 38cfe85bf5b = 2: 38cfe85bf5b reftable: explicitly handle hash format IDs 3: 745c1a070dd = 3: 745c1a070dd reftable/system: stop depending on "hash.h" 4: 7782652b975 = 4: 7782652b975 reftable/stack: stop using `fsync_component()` directly 5: b15daefbc83 ! 5: 430be1045d6 reftable/system: provide thin wrapper for tempfile subsystem
@@ reftable/system.h: license that can be found in the LICENSE file or at
+
+/*
+ * Rename the temporary file to the provided path. The temporary file must be
-+ * active. Return 0 on success, a reftable error code on error. ++ * active. Return 0 on success, a reftable error code on error. Deactivates the ++ * temporary file.
+ */
+int tmpfile_rename(struct reftable_tmpfile *t, const char *path);
6: 83949837a29 = 6: b9ffdc605b9 reftable/stack: drop only use of `get_locked_file_path()` 7: 80fe5bc5e10 = 7: e1ac1cc2e67 reftable/system: provide thin wrapper for lockfile subsystem
Signature · 1 line
2.47.0.274.g962d0b743d.dirty
PATCH v3 1/7 reftable/system: move "dir.h" to its only user
We still include "dir.h" in "reftable/system.h" even though it is not used by anything but by a single unit test. Move it over into that unit test so that we don't accidentally use any functionality provided by it in the reftable codebase.
--- reftable/system.h | 1 - t/unit-tests/t-reftable-stack.c | 1 + 2 files changed, 1 insertion(+), 1 deletion(-)
reftable/system.h +0 -1
@@ -15,7 +15,6 @@ license that can be found in the LICENSE file or at#include "lockfile.h" #include "tempfile.h" #include "hash.h" /* hash ID, sizes.*/ -#include "dir.h" /* remove_dir_recursively, for tests.*/ int hash_size (uint32_t id);
t/unit-tests/t-reftable-stack.c +1 -0
@@ -8,6 +8,7 @@ license that can be found in the LICENSE file or at#include "test-lib.h" #include "lib-reftable.h" +#include "dir.h" #include "reftable/merged.h" #include "reftable/reader.h" #include "reftable/reftable-error.h"
Signature · 1 line
2.47.0.274.g962d0b743d.dirty
PATCH v3 2/7 reftable: explicitly handle hash format IDs
In reply to Patrick SteinhardtThe hash format IDs are used for two different things across the reftable codebase:
- They are used as a 32 bit unsigned integer when reading and writing the header in order to identify the hash function.
- They are used internally to identify which hash function is in use.
When one only considers the second usecase one might think that one can easily change the representation of those hash IDs. But because those IDs end up in the reftable header and footer on disk it is important that those never change.
Create separate constants REFTABLE_FORMAT_ID_* and use them in contexts where we read or write reftable headers. This serves multiple purposes:
- It allows us to more easily discern cases where we actually use those constants for the on-disk format.
- It detangles us from the same constants that are defined in libgit.a, which is another required step to convert the reftable library to become standalone.
- It makes the next step easier where we stop using GIT_*_FORMAT_ID constants in favor of a custom enum.
--- reftable/basics.h | 8 ++++++++ reftable/reader.c | 10 ++++++---- reftable/writer.c | 16 +++++++++++++++- 3 files changed, 29 insertions(+), 5 deletions(-)
reftable/basics.h +8 -0
@@ -150,4 +150,12 @@ int common_prefix_size(struct reftable_buf *a, struct reftable_buf *b);int hash_size (uint32_t id); +/* +* Format IDs that identify the hash function used by a reftable. Note that +* these constants end up on disk and thus mustn't change. The format IDs are +* "sha1" and "s256" in big endian, respectively. +*/ +#define REFTABLE_FORMAT_ID_SHA1 ((uint32_t) 0x73686131) +#define REFTABLE_FORMAT_ID_SHA256 ((uint32_t) 0x73323536) +#endif
reftable/reader.c +6 -4
@@ -109,16 +109,18 @@ static int parse_footer(struct reftable_reader *r, uint8_t *footer,if (r->version ==1 ) { r->hash_id =GIT_SHA1_FORMAT_ID ; }else { - r->hash_id =get_be32 (f); -switch (r->hash_id) { -case GIT_SHA1_FORMAT_ID : +switch (get_be32 (f)) { +case REFTABLE_FORMAT_ID_SHA1 : + r->hash_id =GIT_SHA1_FORMAT_ID ;break ; -case GIT_SHA256_FORMAT_ID : +case REFTABLE_FORMAT_ID_SHA256 : + r->hash_id =GIT_SHA256_FORMAT_ID ;break ;default : err =REFTABLE_FORMAT_ERROR ; goto done; } + f +=4 ; }
reftable/writer.c +15 -1
@@ -103,8 +103,22 @@ static int writer_write_header(struct reftable_writer *w, uint8_t *dest)put_be64 (dest +8 , w->min_update_index);put_be64 (dest +16 , w->max_update_index);if (writer_version (w) ==2 ) { -put_be32 (dest +24 , w->opts.hash_id); +uint32_t hash_id; + +switch (w->opts.hash_id) { +case GIT_SHA1_FORMAT_ID : + hash_id =REFTABLE_FORMAT_ID_SHA1 ; +break ; +case GIT_SHA256_FORMAT_ID : + hash_id =REFTABLE_FORMAT_ID_SHA256 ; +break ; +default : +return -1 ; + } + +put_be32 (dest +24 , hash_id); } +return header_size (writer_version (w)); }
Signature · 1 line
2.47.0.274.g962d0b743d.dirty
PATCH v3 3/7 reftable/system: stop depending on "hash.h"
In reply to Patrick SteinhardtWe include "hash.h" in "reftable/system.h" such that we can use hash format IDs as well as the raw size of SHA1 and SHA256. As we are in the process of converting the reftable library to become standalone we of course cannot rely on those constants anymore.
Introduce a new enum reftable_hash to replace internal uses of the hash format IDs and new constants that replace internal uses of the hash size. Adapt the reftable backend to set up the correct hash function.
--- refs/reftable-backend.c | 12 +++++- reftable/basics.c | 13 ++++--- reftable/basics.h | 2 +- reftable/merged.c | 4 +- reftable/merged.h | 3 +- reftable/reader.c | 8 ++-- reftable/reader.h | 4 +- reftable/reftable-basics.h | 13 +++++++ reftable/reftable-merged.h | 4 +- reftable/reftable-reader.h | 2 +- reftable/reftable-record.h | 12 +++--- reftable/reftable-writer.h | 2 +- reftable/stack.c | 4 +- reftable/system.h | 3 -- reftable/writer.c | 8 ++-- t/helper/test-reftable.c | 4 +- t/unit-tests/lib-reftable.c | 4 +- t/unit-tests/lib-reftable.h | 2 +- t/unit-tests/t-reftable-block.c | 40 +++++++++---------- t/unit-tests/t-reftable-merged.c | 26 ++++++------- t/unit-tests/t-reftable-pq.c | 2 +- t/unit-tests/t-reftable-reader.c | 4 +- t/unit-tests/t-reftable-readwrite.c | 40 +++++++++---------- t/unit-tests/t-reftable-record.c | 59 +++++++++++++++-------------- t/unit-tests/t-reftable-stack.c | 34 ++++++++--------- 25 files changed, 166 insertions(+), 143 deletions(-)
refs/reftable-backend.c +11 -1
@@ -15,6 +15,7 @@#include "../object.h" #include "../path.h" #include "../refs.h" +#include "../reftable/reftable-basics.h" #include "../reftable/reftable-stack.h" #include "../reftable/reftable-record.h" #include "../reftable/reftable-error.h"
@@ -289,7 +290,16 @@ static struct ref_store *reftable_be_init(struct repository *repo, refs->store_flags = store_flags; refs->log_all_ref_updates =repo_settings_get_log_all_ref_updates (repo); - refs->write_options.hash_id = repo->hash_algo->format_id; +switch (repo->hash_algo->format_id) { +case GIT_SHA1_FORMAT_ID : + refs->write_options.hash_id =REFTABLE_HASH_SHA1 ; +break ; +case GIT_SHA256_FORMAT_ID : + refs->write_options.hash_id =REFTABLE_HASH_SHA256 ; +break ; +default : +BUG ("unknown hash algorithm %d" , repo->hash_algo->format_id); + } refs->write_options.default_permissions =calc_shared_perm (0666 & ~mask); refs->write_options.disable_auto_compact = !git_env_bool ("GIT_TEST_REFTABLE_AUTOCOMPACTION" ,1 );
reftable/basics.c +7 -6
@@ -271,14 +271,15 @@ int common_prefix_size(struct reftable_buf *a, struct reftable_buf *b)return p; } -int hash_size (uint32_t id) +int hash_size (enum reftable_hash id) { +if (!id) +return REFTABLE_HASH_SIZE_SHA1 ;switch (id) { -case 0 : -case GIT_SHA1_FORMAT_ID : -return GIT_SHA1_RAWSZ ; -case GIT_SHA256_FORMAT_ID : -return GIT_SHA256_RAWSZ ; +case REFTABLE_HASH_SHA1 : +return REFTABLE_HASH_SIZE_SHA1 ; +case REFTABLE_HASH_SHA256 : +return REFTABLE_HASH_SIZE_SHA256 ; }abort (); }
reftable/basics.h +1 -1
@@ -148,7 +148,7 @@ char *reftable_strdup(const char *str);/* Find the longest shared prefix size of `a` and `b` */ int common_prefix_size (struct reftable_buf *a,struct reftable_buf *b); -int hash_size (uint32_t id); +int hash_size (enum reftable_hash id);/* * Format IDs that identify the hash function used by a reftable. Note that
reftable/merged.c +2 -2
@@ -181,7 +181,7 @@ static void iterator_from_merged_iter(struct reftable_iterator *it,int reftable_merged_table_new (struct reftable_merged_table **dest,struct reftable_reader **readers,size_t n, -uint32_t hash_id) +enum reftable_hash hash_id) {struct reftable_merged_table *m =NULL ;uint64_t last_max =0 ;
@@ -293,7 +293,7 @@ int reftable_merged_table_init_log_iterator(struct reftable_merged_table *mt,return merged_table_init_iter (mt, it,BLOCK_TYPE_LOG ); } -uint32_t reftable_merged_table_hash_id (struct reftable_merged_table *mt) +enum reftable_hash reftable_merged_table_hash_id (struct reftable_merged_table *mt) {return mt->hash_id; }
reftable/merged.h +2 -1
@@ -10,11 +10,12 @@ license that can be found in the LICENSE file or at#define MERGED_H #include "system.h" +#include "reftable-basics.h" struct reftable_merged_table {struct reftable_reader **readers;size_t readers_len; -uint32_t hash_id; +enum reftable_hash hash_id;/* If unset, produce deletions. This is useful for compaction. For the * full stack, deletions should be produced. */
reftable/reader.c +4 -4
@@ -67,7 +67,7 @@ static int reader_get_block(struct reftable_reader *r,return block_source_read_block (&r->source, dest, off, sz); } -uint32_t reftable_reader_hash_id (struct reftable_reader *r) +enum reftable_hash reftable_reader_hash_id (struct reftable_reader *r) {return r->hash_id; }
@@ -107,14 +107,14 @@ static int parse_footer(struct reftable_reader *r, uint8_t *footer, f +=8 ;if (r->version ==1 ) { - r->hash_id =GIT_SHA1_FORMAT_ID ; + r->hash_id =REFTABLE_HASH_SHA1 ; }else {switch (get_be32 (f)) {case REFTABLE_FORMAT_ID_SHA1 : - r->hash_id =GIT_SHA1_FORMAT_ID ; + r->hash_id =REFTABLE_HASH_SHA1 ;break ;case REFTABLE_FORMAT_ID_SHA256 : - r->hash_id =GIT_SHA256_FORMAT_ID ; + r->hash_id =REFTABLE_HASH_SHA256 ;break ;default : err =REFTABLE_FORMAT_ERROR ;
reftable/reader.h +2 -2
@@ -37,8 +37,8 @@ struct reftable_reader {/* Size of the file, excluding the footer. */ uint64_t size; -/* 'sha1' for SHA1, 's256' for SHA-256 */ -uint32_t hash_id; +/* The hash function used for ref records. */ +enum reftable_hash hash_id;uint32_t block_size;uint64_t min_update_index;
reftable/reftable-basics.h +13 -0
@@ -11,6 +11,19 @@#include <stddef.h> +/* +* Hash functions understood by the reftable library. Note that the values are +* arbitrary and somewhat random such that we can easily detect cases where the +* hash hasn't been properly set up. +*/ +enum reftable_hash { +REFTABLE_HASH_SHA1 =89 , +REFTABLE_HASH_SHA256 =247 , +}; +#define REFTABLE_HASH_SIZE_SHA1 20 +#define REFTABLE_HASH_SIZE_SHA256 32 +#define REFTABLE_HASH_SIZE_MAX REFTABLE_HASH_SIZE_SHA256 +/* Overrides the functions to use for memory management. */ void reftable_set_alloc (void *(*malloc)(size_t ),void *(*realloc)(void *,size_t ),void (*free)(void *));
reftable/reftable-merged.h +2 -2
@@ -34,7 +34,7 @@ struct reftable_reader;*/ int reftable_merged_table_new (struct reftable_merged_table **dest,struct reftable_reader **readers,size_t n, -uint32_t hash_id); +enum reftable_hash hash_id);/* Initialize a merged table iterator for reading refs. */ int reftable_merged_table_init_ref_iterator(struct reftable_merged_table *mt,
@@ -56,6 +56,6 @@ reftable_merged_table_min_update_index(struct reftable_merged_table *mt);void reftable_merged_table_free (struct reftable_merged_table *m);/* return the hash ID of the merged table. */ -uint32_t reftable_merged_table_hash_id (struct reftable_merged_table *m); +enum reftable_hash reftable_merged_table_hash_id (struct reftable_merged_table *m);#endif
reftable/reftable-reader.h +1 -1
@@ -54,7 +54,7 @@ int reftable_reader_init_log_iterator(struct reftable_reader *r, struct reftable_iterator *it);/* returns the hash ID used in this table. */ -uint32_t reftable_reader_hash_id (struct reftable_reader *r); +enum reftable_hash reftable_reader_hash_id (struct reftable_reader *r);/* return an iterator for the refs pointing to `oid`. */ int reftable_reader_refs_for(struct reftable_reader *r,
reftable/reftable-record.h +6 -6
@@ -9,7 +9,7 @@ license that can be found in the LICENSE file or at#ifndef REFTABLE_RECORD_H #define REFTABLE_RECORD_H -#include "hash.h" +#include "reftable-basics.h" #include <stdint.h> /*
@@ -40,10 +40,10 @@ struct reftable_ref_record {#define REFTABLE_NR_REF_VALUETYPES 4 } value_type;union { -unsigned char val1[GIT_MAX_RAWSZ ]; +unsigned char val1[REFTABLE_HASH_SIZE_MAX ];struct { -unsigned char value[GIT_MAX_RAWSZ ];/* first hash */ -unsigned char target_value[GIT_MAX_RAWSZ ];/* second hash */ +unsigned char value[REFTABLE_HASH_SIZE_MAX ];/* first hash */ +unsigned char target_value[REFTABLE_HASH_SIZE_MAX ];/* second hash */ } val2;char *symref;/* referent, malloced 0-terminated string */ } value;
@@ -85,8 +85,8 @@ struct reftable_log_record {union {struct { -unsigned char new_hash[GIT_MAX_RAWSZ ]; -unsigned char old_hash[GIT_MAX_RAWSZ ]; +unsigned char new_hash[REFTABLE_HASH_SIZE_MAX ]; +unsigned char old_hash[REFTABLE_HASH_SIZE_MAX ];char *name;char *email;uint64_t time;
reftable/reftable-writer.h +1 -1
@@ -33,7 +33,7 @@ struct reftable_write_options {/* 4-byte identifier ("sha1", "s256") of the hash. * Defaults to SHA1 if unset */ -uint32_t hash_id; +enum reftable_hash hash_id;/* Default mode for creating files. If unset, use 0666 (+umask) */ unsigned int default_permissions;
reftable/stack.c +2 -2
@@ -73,7 +73,7 @@ int reftable_new_stack(struct reftable_stack **dest, const char *dir,if (_opts) opts = *_opts;if (opts.hash_id ==0 ) - opts.hash_id =GIT_SHA1_FORMAT_ID ; + opts.hash_id =REFTABLE_HASH_SHA1 ; *dest =NULL ;
@@ -1603,7 +1603,7 @@ struct segment suggest_compaction_segment(uint64_t *sizes, size_t n,static uint64_t *stack_table_sizes_for_compaction (struct reftable_stack *st) { -int version = (st->opts.hash_id ==GIT_SHA1_FORMAT_ID ) ?1 :2 ; +int version = (st->opts.hash_id ==REFTABLE_HASH_SHA1 ) ?1 :2 ;int overhead =header_size (version) -1 ;uint64_t *sizes;
reftable/system.h +0 -3
@@ -14,8 +14,5 @@ license that can be found in the LICENSE file or at#include "git-compat-util.h" #include "lockfile.h" #include "tempfile.h" -#include "hash.h" /* hash ID, sizes.*/ - -int hash_size (uint32_t id);#endif
reftable/writer.c +4 -4
@@ -79,7 +79,7 @@ static void options_set_defaults(struct reftable_write_options *opts) }if (opts->hash_id ==0 ) { - opts->hash_id =GIT_SHA1_FORMAT_ID ; + opts->hash_id =REFTABLE_HASH_SHA1 ; }if (opts->block_size ==0 ) { opts->block_size =DEFAULT_BLOCK_SIZE ;
@@ -88,7 +88,7 @@ static void options_set_defaults(struct reftable_write_options *opts)static int writer_version (struct reftable_writer *w) { -return (w->opts.hash_id ==0 || w->opts.hash_id ==GIT_SHA1_FORMAT_ID ) ? +return (w->opts.hash_id ==0 || w->opts.hash_id ==REFTABLE_HASH_SHA1 ) ?1 :2 ; }
@@ -106,10 +106,10 @@ static int writer_write_header(struct reftable_writer *w, uint8_t *dest)uint32_t hash_id;switch (w->opts.hash_id) { -case GIT_SHA1_FORMAT_ID : +case REFTABLE_HASH_SHA1 : hash_id =REFTABLE_FORMAT_ID_SHA1 ;break ; -case GIT_SHA256_FORMAT_ID : +case REFTABLE_HASH_SHA256 : hash_id =REFTABLE_FORMAT_ID_SHA256 ;break ;default :
t/helper/test-reftable.c +2 -2
@@ -156,7 +156,7 @@ int cmd__dump_reftable(int argc, const char **argv)int opt_dump_blocks =0 ;int opt_dump_table =0 ;int opt_dump_stack =0 ; -uint32_t opt_hash_id =GIT_SHA1_FORMAT_ID ; +uint32_t opt_hash_id =REFTABLE_HASH_SHA1 ;const char *arg =NULL , *argv0 = argv[0 ];for (; argc >1 ; argv++, argc--)
@@ -167,7 +167,7 @@ int cmd__dump_reftable(int argc, const char **argv)else if (!strcmp ("-t" , argv[1 ])) opt_dump_table =1 ;else if (!strcmp ("-6" , argv[1 ])) - opt_hash_id =GIT_SHA256_FORMAT_ID ; + opt_hash_id =REFTABLE_HASH_SHA256 ;else if (!strcmp ("-s" , argv[1 ])) opt_dump_stack =1 ;else if (!strcmp ("-?" , argv[1 ]) || !strcmp ("-h" , argv[1 ])) {
t/unit-tests/lib-reftable.c +2 -2
@@ -3,7 +3,7 @@#include "reftable/constants.h" #include "reftable/writer.h" -void t_reftable_set_hash (uint8_t *p,int i,uint32_t id) +void t_reftable_set_hash (uint8_t *p,int i,enum reftable_hash id) {memset (p, (uint8_t )i,hash_size (id)); }
@@ -82,7 +82,7 @@ void t_reftable_write_to_buf(struct reftable_buf *buf,size_t off = i * (opts.block_size ? opts.block_size :DEFAULT_BLOCK_SIZE );if (!off) - off =header_size (opts.hash_id ==GIT_SHA256_FORMAT_ID ?2 :1 ); + off =header_size (opts.hash_id ==REFTABLE_HASH_SHA256 ?2 :1 ); check_char(buf->buf[off], ==, 'r'); }
t/unit-tests/lib-reftable.h +1 -1
@@ -6,7 +6,7 @@struct reftable_buf ; -void t_reftable_set_hash (uint8_t *p,int i,uint32_t id); +void t_reftable_set_hash (uint8_t *p,int i,enum reftable_hash id);struct reftable_writer *t_reftable_strbuf_writer (struct reftable_buf *buf,struct reftable_write_options *opts);
t/unit-tests/t-reftable-block.c +20 -20
@@ -36,7 +36,7 @@ static void t_ref_block_read_write(void) block.len = block_size;block_source_from_buf (&block.source ,&buf); ret =block_writer_init (&bw,BLOCK_TYPE_REF , block.data, block_size, - header_off,hash_size (GIT_SHA1_FORMAT_ID )); + header_off,hash_size (REFTABLE_HASH_SHA1 ));check (!ret); rec.u.ref.refname = (char *)"" ;
@@ -47,7 +47,7 @@ static void t_ref_block_read_write(void)for (i =0 ; i <N ; i++) { rec.u.ref.refname =xstrfmt ("branch%02" PRIuMAX, (uintmax_t )i); rec.u.ref.value_type =REFTABLE_REF_VAL1 ; -memset (rec.u.ref.value.val1, i,GIT_SHA1_RAWSZ ); +memset (rec.u.ref.value.val1, i,REFTABLE_HASH_SIZE_SHA1 ); recs[i] = rec; ret =block_writer_add (&bw, &rec);
@@ -61,7 +61,7 @@ static void t_ref_block_read_write(void)block_writer_release (&bw); -block_reader_init (&br, &block, header_off, block_size,GIT_SHA1_RAWSZ ); +block_reader_init (&br, &block, header_off, block_size,REFTABLE_HASH_SIZE_SHA1 );block_iter_seek_start (&it, &br);
@@ -72,7 +72,7 @@ static void t_ref_block_read_write(void) check_int(i, ==, N);break ; } -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); }for (i =0 ; i <N ; i++) {
@@ -85,7 +85,7 @@ static void t_ref_block_read_write(void) ret =block_iter_next (&it, &rec); check_int(ret, ==, 0); -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); want.len--; ret =block_iter_seek_key (&it, &br, &want);
@@ -93,7 +93,7 @@ static void t_ref_block_read_write(void) ret =block_iter_next (&it, &rec); check_int(ret, ==, 0); -check (reftable_record_equal (&recs[10 * (i /10 )], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[10 * (i /10 )], &rec,REFTABLE_HASH_SIZE_SHA1 )); }block_reader_release (&br);
@@ -130,7 +130,7 @@ static void t_log_block_read_write(void) block.len = block_size;block_source_from_buf (&block.source ,&buf); ret =block_writer_init (&bw,BLOCK_TYPE_LOG , block.data, block_size, - header_off,hash_size (GIT_SHA1_FORMAT_ID )); + header_off,hash_size (REFTABLE_HASH_SHA1 ));check (!ret);for (i =0 ; i <N ; i++) {
@@ -150,7 +150,7 @@ static void t_log_block_read_write(void)block_writer_release (&bw); -block_reader_init (&br, &block, header_off, block_size,GIT_SHA1_RAWSZ ); +block_reader_init (&br, &block, header_off, block_size,REFTABLE_HASH_SIZE_SHA1 );block_iter_seek_start (&it, &br);
@@ -161,7 +161,7 @@ static void t_log_block_read_write(void) check_int(i, ==, N);break ; } -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); }for (i =0 ; i <N ; i++) {
@@ -175,7 +175,7 @@ static void t_log_block_read_write(void) ret =block_iter_next (&it, &rec); check_int(ret, ==, 0); -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); want.len--; ret =block_iter_seek_key (&it, &br, &want);
@@ -183,7 +183,7 @@ static void t_log_block_read_write(void) ret =block_iter_next (&it, &rec); check_int(ret, ==, 0); -check (reftable_record_equal (&recs[10 * (i /10 )], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[10 * (i /10 )], &rec,REFTABLE_HASH_SIZE_SHA1 )); }block_reader_release (&br);
@@ -220,7 +220,7 @@ static void t_obj_block_read_write(void) block.len = block_size;block_source_from_buf (&block.source, &buf); ret =block_writer_init (&bw,BLOCK_TYPE_OBJ , block.data, block_size, - header_off,hash_size (GIT_SHA1_FORMAT_ID )); + header_off,hash_size (REFTABLE_HASH_SHA1 ));check (!ret);for (i =0 ; i <N ; i++) {
@@ -242,7 +242,7 @@ static void t_obj_block_read_write(void)block_writer_release (&bw); -block_reader_init (&br, &block, header_off, block_size,GIT_SHA1_RAWSZ ); +block_reader_init (&br, &block, header_off, block_size,REFTABLE_HASH_SIZE_SHA1 );block_iter_seek_start (&it, &br);
@@ -253,7 +253,7 @@ static void t_obj_block_read_write(void) check_int(i, ==, N);break ; } -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); }for (i =0 ; i <N ; i++) {
@@ -266,7 +266,7 @@ static void t_obj_block_read_write(void) ret =block_iter_next (&it, &rec); check_int(ret, ==, 0); -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); }block_reader_release (&br);
@@ -304,7 +304,7 @@ static void t_index_block_read_write(void) block.len = block_size;block_source_from_buf (&block.source, &buf); ret =block_writer_init (&bw,BLOCK_TYPE_INDEX , block.data, block_size, - header_off,hash_size (GIT_SHA1_FORMAT_ID )); + header_off,hash_size (REFTABLE_HASH_SHA1 ));check (!ret);for (i =0 ; i <N ; i++) {
@@ -326,7 +326,7 @@ static void t_index_block_read_write(void)block_writer_release (&bw); -block_reader_init (&br, &block, header_off, block_size,GIT_SHA1_RAWSZ ); +block_reader_init (&br, &block, header_off, block_size,REFTABLE_HASH_SIZE_SHA1 );block_iter_seek_start (&it, &br);
@@ -337,7 +337,7 @@ static void t_index_block_read_write(void) check_int(i, ==, N);break ; } -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); }for (i =0 ; i <N ; i++) {
@@ -350,7 +350,7 @@ static void t_index_block_read_write(void) ret =block_iter_next (&it, &rec); check_int(ret, ==, 0); -check (reftable_record_equal (&recs[i], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[i], &rec,REFTABLE_HASH_SIZE_SHA1 )); want.len--; ret =block_iter_seek_key (&it, &br, &want);
@@ -358,7 +358,7 @@ static void t_index_block_read_write(void) ret =block_iter_next (&it, &rec); check_int(ret, ==, 0); -check (reftable_record_equal (&recs[10 * (i /10 )], &rec,GIT_SHA1_RAWSZ )); +check (reftable_record_equal (&recs[10 * (i /10 )], &rec,REFTABLE_HASH_SIZE_SHA1 )); }block_reader_release (&br);
t/unit-tests/t-reftable-merged.c +13 -13
@@ -42,7 +42,7 @@ merged_table_from_records(struct reftable_ref_record **refs,check (!err); } - err =reftable_merged_table_new (&mt, *readers, n,GIT_SHA1_FORMAT_ID ); + err =reftable_merged_table_new (&mt, *readers, n,REFTABLE_HASH_SHA1 );check (!err);return mt; }
@@ -91,7 +91,7 @@ static void t_merged_single_record(void) err =reftable_iterator_next_ref (&it, &ref);check (!err); -check (reftable_ref_record_equal (&r2[0 ], &ref,GIT_SHA1_RAWSZ )); +check (reftable_ref_record_equal (&r2[0 ], &ref,REFTABLE_HASH_SIZE_SHA1 ));reftable_ref_record_release (&ref);reftable_iterator_destroy (&it);readers_destroy (readers,3 );
@@ -168,7 +168,7 @@ static void t_merged_refs(void)check (!err); err =reftable_iterator_seek_ref (&it,"a" );check (!err); - check_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID); + check_int(reftable_merged_table_hash_id(mt), ==, REFTABLE_HASH_SHA1); check_int(reftable_merged_table_min_update_index(mt), ==, 1); check_int(reftable_merged_table_max_update_index(mt), ==, 3);
@@ -186,7 +186,7 @@ static void t_merged_refs(void) check_int(ARRAY_SIZE(want), ==, len);for (i =0 ; i < len; i++)check (reftable_ref_record_equal (want[i], &out[i], -GIT_SHA1_RAWSZ )); +REFTABLE_HASH_SIZE_SHA1 ));for (i =0 ; i < len; i++)reftable_ref_record_release (&out[i]);reftable_free (out);
@@ -252,12 +252,12 @@ static void t_merged_seek_multiple_times(void) err =reftable_iterator_next_ref (&it, &rec);check (!err); - err =reftable_ref_record_equal (&rec, &r1[1 ],GIT_SHA1_RAWSZ ); + err =reftable_ref_record_equal (&rec, &r1[1 ],REFTABLE_HASH_SIZE_SHA1 );check (err ==1 ); err =reftable_iterator_next_ref (&it, &rec);check (!err); - err =reftable_ref_record_equal (&rec, &r2[1 ],GIT_SHA1_RAWSZ ); + err =reftable_ref_record_equal (&rec, &r2[1 ],REFTABLE_HASH_SIZE_SHA1 );check (err ==1 ); err =reftable_iterator_next_ref (&it, &rec);
@@ -300,7 +300,7 @@ merged_table_from_log_records(struct reftable_log_record **logs,check (!err); } - err =reftable_merged_table_new (&mt, *readers, n,GIT_SHA1_FORMAT_ID ); + err =reftable_merged_table_new (&mt, *readers, n,REFTABLE_HASH_SHA1 );check (!err);return mt; }
@@ -377,7 +377,7 @@ static void t_merged_logs(void)check (!err); err =reftable_iterator_seek_log (&it,"a" );check (!err); - check_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID); + check_int(reftable_merged_table_hash_id(mt), ==, REFTABLE_HASH_SHA1); check_int(reftable_merged_table_min_update_index(mt), ==, 1); check_int(reftable_merged_table_max_update_index(mt), ==, 3);
@@ -395,7 +395,7 @@ static void t_merged_logs(void) check_int(ARRAY_SIZE(want), ==, len);for (i =0 ; i < len; i++)check (reftable_log_record_equal (want[i], &out[i], -GIT_SHA1_RAWSZ )); +REFTABLE_HASH_SIZE_SHA1 )); err =merged_table_init_iter (mt, &it,BLOCK_TYPE_LOG );check (!err);
@@ -404,7 +404,7 @@ static void t_merged_logs(void)reftable_log_record_release (&out[0 ]); err =reftable_iterator_next_log (&it, &out[0 ]);check (!err); -check (reftable_log_record_equal (&out[0 ], &r3[0 ],GIT_SHA1_RAWSZ )); +check (reftable_log_record_equal (&out[0 ], &r3[0 ],REFTABLE_HASH_SIZE_SHA1 ));reftable_iterator_destroy (&it);for (i =0 ; i < len; i++)
@@ -448,11 +448,11 @@ static void t_default_write_opts(void)check (!err); hash_id =reftable_reader_hash_id (rd); - check_int(hash_id, ==, GIT_SHA1_FORMAT_ID); + check_int(hash_id, ==, REFTABLE_HASH_SHA1); - err =reftable_merged_table_new (&merged, &rd,1 ,GIT_SHA256_FORMAT_ID ); + err =reftable_merged_table_new (&merged, &rd,1 ,REFTABLE_HASH_SHA256 ); check_int(err, ==, REFTABLE_FORMAT_ERROR); - err =reftable_merged_table_new (&merged, &rd,1 ,GIT_SHA1_FORMAT_ID ); + err =reftable_merged_table_new (&merged, &rd,1 ,REFTABLE_HASH_SHA1 );check (!err);reftable_reader_decref (rd);
t/unit-tests/t-reftable-pq.c +1 -1
@@ -132,7 +132,7 @@ static void t_merged_iter_pqueue_top(void)merged_iter_pqueue_check (&pq);check (pq_entry_equal (&top, &e)); -check (reftable_record_equal (top.rec, &recs[i],GIT_SHA1_RAWSZ )); +check (reftable_record_equal (top.rec, &recs[i],REFTABLE_HASH_SIZE_SHA1 ));for (size_t j =0 ; i < pq.len; j++) {check (pq_less (&top, &pq.heap[j])); check_int(top.index, >, j);
t/unit-tests/t-reftable-reader.c +2 -2
@@ -31,7 +31,7 @@ static int t_reader_seek_once(void) ret =reftable_iterator_next_ref (&it, &ref);check (!ret); - ret =reftable_ref_record_equal (&ref, &records[0 ],GIT_SHA1_RAWSZ ); + ret =reftable_ref_record_equal (&ref, &records[0 ],REFTABLE_HASH_SIZE_SHA1 ); check_int(ret, ==, 1); ret =reftable_iterator_next_ref (&it, &ref);
@@ -74,7 +74,7 @@ static int t_reader_reseek(void) ret =reftable_iterator_next_ref (&it, &ref);check (!ret); - ret =reftable_ref_record_equal (&ref, &records[0 ],GIT_SHA1_RAWSZ ); + ret =reftable_ref_record_equal (&ref, &records[0 ],REFTABLE_HASH_SIZE_SHA1 ); check_int(ret, ==, 1); ret =reftable_iterator_next_ref (&it, &ref);
t/unit-tests/t-reftable-readwrite.c +20 -20
@@ -41,7 +41,7 @@ static void t_buffer(void) }static void write_table (char ***names,struct reftable_buf *buf,int N , -int block_size,uint32_t hash_id) +int block_size,enum reftable_hash hash_id) {struct reftable_write_options opts = { .block_size = block_size,
@@ -62,7 +62,7 @@ static void write_table(char ***names, struct reftable_buf *buf, int N, refs[i].refname = (*names)[i] =xstrfmt ("refs/heads/branch%02d" , i); refs[i].update_index = update_index; refs[i].value_type =REFTABLE_REF_VAL1 ; -t_reftable_set_hash (refs[i].value.val1, i,GIT_SHA1_FORMAT_ID ); +t_reftable_set_hash (refs[i].value.val1, i,REFTABLE_HASH_SHA1 ); }for (i =0 ; i <N ; i++) {
@@ -70,7 +70,7 @@ static void write_table(char ***names, struct reftable_buf *buf, int N, logs[i].update_index = update_index; logs[i].value_type =REFTABLE_LOG_UPDATE ;t_reftable_set_hash (logs[i].value.update.new_hash, i, -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 ); logs[i].value.update.message = (char *)"message" ; }
@@ -104,7 +104,7 @@ static void t_log_buffer_size(void)/* This tests buffer extension for log compression. Must use a random hash, to ensure that the compressed part is larger than the original. */ -for (i =0 ; i <GIT_SHA1_RAWSZ ; i++) { +for (i =0 ; i <REFTABLE_HASH_SIZE_SHA1 ; i++) { log.value.update.old_hash[i] = (uint8_t )(git_rand () %256 ); log.value.update.new_hash[i] = (uint8_t )(git_rand () %256 ); }
@@ -191,9 +191,9 @@ static void t_log_write_read(void) log.update_index = i; log.value_type =REFTABLE_LOG_UPDATE ;t_reftable_set_hash (log.value.update.old_hash, i, -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 );t_reftable_set_hash (log.value.update.new_hash, i +1 , -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 ); err =reftable_writer_add_log (w, &log);check (!err);
@@ -326,7 +326,7 @@ static void t_table_read_write_sequential(void)int err =0 ;int j =0 ; -write_table (&names, &buf,N ,256 ,GIT_SHA1_FORMAT_ID ); +write_table (&names, &buf,N ,256 ,REFTABLE_HASH_SHA1 );block_source_from_buf (&source, &buf);
@@ -361,7 +361,7 @@ static void t_table_write_small_table(void)char **names;struct reftable_buf buf =REFTABLE_BUF_INIT ;int N =1 ; -write_table (&names, &buf,N ,4096 ,GIT_SHA1_FORMAT_ID ); +write_table (&names, &buf,N ,4096 ,REFTABLE_HASH_SHA1 ); check_int(buf.len, <, 200);reftable_buf_release (&buf);free_names (names);
@@ -378,7 +378,7 @@ static void t_table_read_api(void)struct reftable_log_record log = {0 };struct reftable_iterator it = {0 }; -write_table (&names, &buf,N ,256 ,GIT_SHA1_FORMAT_ID ); +write_table (&names, &buf,N ,256 ,REFTABLE_HASH_SHA1 );block_source_from_buf (&source, &buf);
@@ -400,7 +400,7 @@ static void t_table_read_api(void)reftable_buf_release (&buf); } -static void t_table_read_write_seek (int index,int hash_id) +static void t_table_read_write_seek (int index,enum reftable_hash hash_id) {char **names;struct reftable_buf buf =REFTABLE_BUF_INIT ;
@@ -467,24 +467,24 @@ static void t_table_read_write_seek(int index, int hash_id)static void t_table_read_write_seek_linear (void ) { -t_table_read_write_seek (0 ,GIT_SHA1_FORMAT_ID ); +t_table_read_write_seek (0 ,REFTABLE_HASH_SHA1 ); }static void t_table_read_write_seek_linear_sha256 (void ) { -t_table_read_write_seek (0 ,GIT_SHA256_FORMAT_ID ); +t_table_read_write_seek (0 ,REFTABLE_HASH_SHA256 ); }static void t_table_read_write_seek_index (void ) { -t_table_read_write_seek (1 ,GIT_SHA1_FORMAT_ID ); +t_table_read_write_seek (1 ,REFTABLE_HASH_SHA1 ); }static void t_table_refs_for (int indexed) {char **want_names;int want_names_len =0 ; -uint8_t want_hash[GIT_SHA1_RAWSZ ]; +uint8_t want_hash[REFTABLE_HASH_SIZE_SHA1 ];struct reftable_write_options opts = { .block_size =256 ,
@@ -500,10 +500,10 @@ static void t_table_refs_for(int indexed) want_names =reftable_calloc (N +1 ,sizeof (*want_names));check (want_names !=NULL ); -t_reftable_set_hash (want_hash,4 ,GIT_SHA1_FORMAT_ID ); +t_reftable_set_hash (want_hash,4 ,REFTABLE_HASH_SHA1 );for (i =0 ; i <N ; i++) { -uint8_t hash[GIT_SHA1_RAWSZ ]; +uint8_t hash[REFTABLE_HASH_SIZE_SHA1 ];char fill[51 ] = {0 };char name[100 ];struct reftable_ref_record ref = {0 };
@@ -517,9 +517,9 @@ static void t_table_refs_for(int indexed) ref.value_type =REFTABLE_REF_VAL2 ;t_reftable_set_hash (ref.value.val2.value, i /4 , -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 );t_reftable_set_hash (ref.value.val2.target_value,3 + i /4 , -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 );/* 80 bytes / entry, so 3 entries per block. Yields 17 */
@@ -527,8 +527,8 @@ static void t_table_refs_for(int indexed) n =reftable_writer_add_ref (w, &ref); check_int(n, ==, 0); -if (!memcmp (ref.value.val2.value, want_hash,GIT_SHA1_RAWSZ ) || - !memcmp (ref.value.val2.target_value, want_hash,GIT_SHA1_RAWSZ )) +if (!memcmp (ref.value.val2.value, want_hash,REFTABLE_HASH_SIZE_SHA1 ) || + !memcmp (ref.value.val2.target_value, want_hash,REFTABLE_HASH_SIZE_SHA1 )) want_names[want_names_len++] =xstrdup (name); }
t/unit-tests/t-reftable-record.c +30 -29
175 lines not on this page · raw message
t/unit-tests/t-reftable-stack.c +17 -17
@@ -121,7 +121,7 @@ static void write_n_ref_tables(struct reftable_stack *st,snprintf (buf,sizeof (buf),"refs/heads/branch-%04" PRIuMAX, (uintmax_t )i); ref.refname = buf; -t_reftable_set_hash (ref.value.val1, i,GIT_SHA1_FORMAT_ID ); +t_reftable_set_hash (ref.value.val1, i,REFTABLE_HASH_SHA1 ); err =reftable_stack_add (st, &write_test_ref, &ref);check (!err);
@@ -169,7 +169,7 @@ static void t_reftable_stack_add_one(void) err =reftable_stack_read_ref (st, ref.refname, &dest);check (!err); -check (reftable_ref_record_equal (&ref, &dest,GIT_SHA1_RAWSZ )); +check (reftable_ref_record_equal (&ref, &dest,REFTABLE_HASH_SIZE_SHA1 )); check_int(st->readers_len, >, 0);#ifndef GIT_WINDOWS_NATIVE
@@ -280,7 +280,7 @@ static void t_reftable_stack_transaction_api(void) err =reftable_stack_read_ref (st, ref.refname, &dest);check (!err); check_int(REFTABLE_REF_SYMREF, ==, dest.value_type); -check (reftable_ref_record_equal (&ref, &dest,GIT_SHA1_RAWSZ )); +check (reftable_ref_record_equal (&ref, &dest,REFTABLE_HASH_SIZE_SHA1 ));reftable_ref_record_release (&dest);reftable_stack_destroy (st);
@@ -340,7 +340,7 @@ static void t_reftable_stack_transaction_with_reload(void)for (size_t i =0 ; i <ARRAY_SIZE (refs); i++) { err =reftable_stack_read_ref (st2, refs[i].refname, &ref);check (!err); -check (reftable_ref_record_equal (&refs[i], &ref,GIT_SHA1_RAWSZ )); +check (reftable_ref_record_equal (&refs[i], &ref,REFTABLE_HASH_SIZE_SHA1 )); }reftable_ref_record_release (&ref);
@@ -530,13 +530,13 @@ static void t_reftable_stack_add(void) refs[i].refname =xstrdup (buf); refs[i].update_index = i +1 ; refs[i].value_type =REFTABLE_REF_VAL1 ; -t_reftable_set_hash (refs[i].value.val1, i,GIT_SHA1_FORMAT_ID ); +t_reftable_set_hash (refs[i].value.val1, i,REFTABLE_HASH_SHA1 ); logs[i].refname =xstrdup (buf); logs[i].update_index =N + i +1 ; logs[i].value_type =REFTABLE_LOG_UPDATE ; logs[i].value.update.email =xstrdup ("identity@invalid" ); -t_reftable_set_hash (logs[i].value.update.new_hash, i,GIT_SHA1_FORMAT_ID ); +t_reftable_set_hash (logs[i].value.update.new_hash, i,REFTABLE_HASH_SHA1 ); }for (i =0 ; i <N ; i++) {
@@ -562,7 +562,7 @@ static void t_reftable_stack_add(void)int err =reftable_stack_read_ref (st, refs[i].refname, &dest);check (!err);check (reftable_ref_record_equal (&dest, refs + i, -GIT_SHA1_RAWSZ )); +REFTABLE_HASH_SIZE_SHA1 ));reftable_ref_record_release (&dest); }
@@ -571,7 +571,7 @@ static void t_reftable_stack_add(void)int err =reftable_stack_read_log (st, refs[i].refname, &dest);check (!err);check (reftable_log_record_equal (&dest, logs + i, -GIT_SHA1_RAWSZ )); +REFTABLE_HASH_SIZE_SHA1 ));reftable_log_record_release (&dest); }
@@ -622,14 +622,14 @@ static void t_reftable_stack_iterator(void) refs[i].refname =xstrfmt ("branch%02" PRIuMAX, (uintmax_t )i); refs[i].update_index = i +1 ; refs[i].value_type =REFTABLE_REF_VAL1 ; -t_reftable_set_hash (refs[i].value.val1, i,GIT_SHA1_FORMAT_ID ); +t_reftable_set_hash (refs[i].value.val1, i,REFTABLE_HASH_SHA1 ); logs[i].refname =xstrfmt ("branch%02" PRIuMAX, (uintmax_t )i); logs[i].update_index = i +1 ; logs[i].value_type =REFTABLE_LOG_UPDATE ; logs[i].value.update.email =xstrdup ("johndoe@invalid" ); logs[i].value.update.message =xstrdup ("commit\n" ); -t_reftable_set_hash (logs[i].value.update.new_hash, i,GIT_SHA1_FORMAT_ID ); +t_reftable_set_hash (logs[i].value.update.new_hash, i,REFTABLE_HASH_SHA1 ); }for (i =0 ; i <N ; i++) {
@@ -656,7 +656,7 @@ static void t_reftable_stack_iterator(void)if (err >0 )break ;check (!err); -check (reftable_ref_record_equal (&ref, &refs[i],GIT_SHA1_RAWSZ )); +check (reftable_ref_record_equal (&ref, &refs[i],REFTABLE_HASH_SIZE_SHA1 ));reftable_ref_record_release (&ref); } check_int(i, ==, N);
@@ -674,7 +674,7 @@ static void t_reftable_stack_iterator(void)if (err >0 )break ;check (!err); -check (reftable_log_record_equal (&log, &logs[i],GIT_SHA1_RAWSZ )); +check (reftable_log_record_equal (&log, &logs[i],REFTABLE_HASH_SIZE_SHA1 ));reftable_log_record_release (&log); } check_int(i, ==, N);
@@ -767,7 +767,7 @@ static void t_reftable_stack_tombstone(void)if (i %2 ==0 ) { refs[i].value_type =REFTABLE_REF_VAL1 ;t_reftable_set_hash (refs[i].value.val1, i, -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 ); } logs[i].refname =xstrdup (buf);
@@ -776,7 +776,7 @@ static void t_reftable_stack_tombstone(void)if (i %2 ==0 ) { logs[i].value_type =REFTABLE_LOG_UPDATE ;t_reftable_set_hash (logs[i].value.update.new_hash, i, -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 ); logs[i].value.update.email =xstrdup ("identity@invalid" ); }
@@ -836,7 +836,7 @@ static void t_reftable_stack_hash_id(void) .value.symref = (char *) "target", .update_index = 1, }; -struct reftable_write_options opts32 = { .hash_id =GIT_SHA256_FORMAT_ID }; +struct reftable_write_options opts32 = { .hash_id =REFTABLE_HASH_SHA256 };struct reftable_stack *st32 =NULL ;struct reftable_write_options opts_default = {0 };struct reftable_stack *st_default =NULL ;
@@ -859,7 +859,7 @@ static void t_reftable_stack_hash_id(void) err =reftable_stack_read_ref (st_default,"master" , &dest);check (!err); -check (reftable_ref_record_equal (&ref, &dest,GIT_SHA1_RAWSZ )); +check (reftable_ref_record_equal (&ref, &dest,REFTABLE_HASH_SIZE_SHA1 ));reftable_ref_record_release (&dest);reftable_stack_destroy (st);reftable_stack_destroy (st_default);
@@ -909,7 +909,7 @@ static void t_reflog_expire(void) logs[i].value.update.time = i; logs[i].value.update.email =xstrdup ("identity@invalid" );t_reftable_set_hash (logs[i].value.update.new_hash, i, -GIT_SHA1_FORMAT_ID ); +REFTABLE_HASH_SHA1 ); }for (i =1 ; i <=N ; i++) {
Signature · 1 line
2.47.0.274.g962d0b743d.dirty
PATCH v3 4/7 reftable/stack: stop using `fsync_component()` directly
In reply to Patrick SteinhardtWe're executing fsync_component() directly in the reftable library so that we can fsync data to disk depending on "core.fsync". But as we're in the process of converting the reftable library to become standalone we cannot use that function in the library anymore.
Refactor the code such that users of the library can inject a custom fsync function via the write options. This allows us to get rid of the dependency on "write-or-die.h".
--- refs/reftable-backend.c | 7 ++++++ reftable/reftable-writer.h | 6 +++++ reftable/stack.c | 49 +++++++++++++++++++++++++------------- 3 files changed, 45 insertions(+), 17 deletions(-)
refs/reftable-backend.c +7 -0
@@ -24,6 +24,7 @@#include "../setup.h" #include "../strmap.h" #include "../trace2.h" +#include "../write-or-die.h" #include "parse.h" #include "refs-internal.h"
@@ -273,6 +274,11 @@ static int reftable_be_config(const char *var, const char *value,return 0 ; } +static int reftable_be_fsync (int fd) +{ +return fsync_component (FSYNC_COMPONENT_REFERENCE , fd); +} +static struct ref_store *reftable_be_init (struct repository *repo,const char *gitdir,unsigned int store_flags)
@@ -304,6 +310,7 @@ static struct ref_store *reftable_be_init(struct repository *repo, refs->write_options.disable_auto_compact = !git_env_bool ("GIT_TEST_REFTABLE_AUTOCOMPACTION" ,1 ); refs->write_options.lock_timeout_ms =100 ; + refs->write_options.fsync = reftable_be_fsync;git_config (reftable_be_config, &refs->write_options);
reftable/reftable-writer.h +6 -0
@@ -62,6 +62,12 @@ struct reftable_write_options {* negative value will cause us to block indefinitely. */ long lock_timeout_ms; + +/* +* Optional callback used to fsync files to disk. Falls back to using +* fsync(3P) when unset. +*/ +int (*fsync)(int fd); };/* reftable_block_stats holds statistics for a single block type */
reftable/stack.c +32 -17
111 lines not on this page · raw message
Signature · 1 line
2.47.0.274.g962d0b743d.dirty
PATCH v3 5/7 reftable/system: provide thin wrapper for tempfile subsystem
In reply to Patrick SteinhardtWe use the tempfile subsystem to write temporary tables, but given that we're in the process of converting the reftable library to become standalone we cannot use this subsystem directly anymore. While we could in theory convert the code to use mkstemp(3p) instead, we'd lose access to our infrastructure that automatically prunes tempfiles via atexit(3p) or signal handlers.
Provide a thin wrapper for the tempfile subsystem instead. Like this, the compatibility shim is fully self-contained in "reftable/system.c". Downstream users of the reftable library would have to implement their own tempfile shims by replacing "system.c" with a custom version.
--- Makefile | 1 + reftable/stack.c | 57 +++++++++++++++++++---------------------------- reftable/system.c | 49 ++++++++++++++++++++++++++++++++++++++++ reftable/system.h | 42 +++++++++++++++++++++++++++++++++- 4 files changed, 114 insertions(+), 35 deletions(-) create mode 100644 reftable/system.c
Makefile +1 -0
@@ -2722,6 +2722,7 @@ REFTABLE_OBJS += reftable/pq.o REFTABLE_OBJS += reftable/reader.o REFTABLE_OBJS += reftable/record.o REFTABLE_OBJS += reftable/stack.o +REFTABLE_OBJS += reftable/system.o REFTABLE_OBJS += reftable/tree.o REFTABLE_OBJS += reftable/writer.o
reftable/stack.c +23 -34
159 lines not on this page · raw message
reftable/system.c new file +49 -0
49 lines not on this page · raw message
reftable/system.h +41 -1
47 lines not on this page · raw message
Signature · 1 line
2.47.0.274.g962d0b743d.dirty
PATCH v3 6/7 reftable/stack: drop only use of `get_locked_file_path()`
In reply to Patrick SteinhardtWe've got a single callsite where we call get_locked_file_path(). As we're about to convert our usage of the lockfile subsystem to instead be used via a compatibility shim we'd have to implement more logic for this single callsite. While that would be okay if Git was the only supposed user of the reftable library, it's a bit more awkward when considering that we have to reimplement this functionality for every user of the library eventually.
Refactor the code such that we don't call get_locked_file_path() anymore.
--- reftable/stack.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
reftable/stack.c +9 -3
@@ -1493,9 +1493,15 @@ static int stack_compact_range(struct reftable_stack *st,*/ for (i =0 ; i < nlocks; i++) {struct lock_file *table_lock = &table_locks[i]; -char *table_path =get_locked_file_path (table_lock); -unlink (table_path); -reftable_free (table_path); +const char *lock_path =get_lock_file_path (table_lock); + +reftable_buf_reset (&table_name); + err =reftable_buf_add (&table_name, lock_path, +strlen (lock_path) -strlen (".lock" )); +if (err) +continue ; + +unlink (table_name.buf); } done:
Signature · 1 line
2.47.0.274.g962d0b743d.dirty
PATCH v3 7/7 reftable/system: provide thin wrapper for lockfile subsystem
In reply to Patrick SteinhardtWe use the lockfile subsystem to write lockfiles for "tables.list". As with the tempfile subsystem, the lockfile subsystem also hooks into our infrastructure to prune stale locks via atexit(3p) or signal handlers.
Furthermore, the lockfile subsystem also handles locking timeouts, which do add quite a bit of logic. Having to reimplement that in the context of Git wouldn't make a whole lot of sense, and it is quite likely that downstream users of the reftable library may have a better idea for how exactly to implement timeouts.
So again, provide a thin wrapper for the lockfile subsystem instead such that the compatibility shim is fully self-contained.
--- reftable/stack.c | 63 ++++++++++------------- reftable/system.c | 77 +++++++++++++++++++++++++++++ reftable/system.h | 45 ++++++++++++++++- t/unit-tests/lib-reftable.c | 1 + t/unit-tests/t-reftable-block.c | 1 + t/unit-tests/t-reftable-pq.c | 1 + t/unit-tests/t-reftable-readwrite.c | 1 + t/unit-tests/t-reftable-stack.c | 2 + 8 files changed, 154 insertions(+), 37 deletions(-)
reftable/stack.c +27 -36
@@ -657,7 +657,7 @@ static int format_name(struct reftable_buf *dest, uint64_t min, uint64_t max) }struct reftable_addition { -struct lock_file tables_list_lock; +struct reftable_flock tables_list_lock;struct reftable_stack *stack;char **new_tables;
@@ -676,10 +676,8 @@ static int reftable_stack_init_addition(struct reftable_addition *add, add->stack = st; - err =hold_lock_file_for_update_timeout (&add->tables_list_lock, - st->list_file, -LOCK_NO_DEREF , - st->opts.lock_timeout_ms); + err =flock_acquire (&add->tables_list_lock, st->list_file, + st->opts.lock_timeout_ms);if (err <0 ) {if (errno ==EEXIST ) { err =REFTABLE_LOCK_ERROR ;
@@ -689,7 +687,7 @@ static int reftable_stack_init_addition(struct reftable_addition *add, goto done; }if (st->opts.default_permissions) { -if (chmod (get_lock_file_path (&add->tables_list_lock), +if (chmod (add->tables_list_lock.path, st->opts.default_permissions) <0 ) { err =REFTABLE_IO_ERROR ; goto done;
@@ -733,7 +731,7 @@ static void reftable_addition_close(struct reftable_addition *add) add->new_tables_len =0 ; add->new_tables_cap =0 ; -rollback_lock_file (&add->tables_list_lock); +flock_release (&add->tables_list_lock);reftable_buf_release (&nm); }
@@ -749,7 +747,6 @@ void reftable_addition_destroy(struct reftable_addition *add)int reftable_addition_commit (struct reftable_addition *add) {struct reftable_buf table_list =REFTABLE_BUF_INIT ; -int lock_file_fd =get_lock_file_fd (&add->tables_list_lock);int err =0 ;size_t i;
@@ -767,20 +764,20 @@ int reftable_addition_commit(struct reftable_addition *add) goto done; } - err =write_in_full (lock_file_fd, table_list.buf, table_list.len); + err =write_in_full (add->tables_list_lock.fd, table_list.buf, table_list.len);reftable_buf_release (&table_list);if (err <0 ) { err =REFTABLE_IO_ERROR ; goto done; } - err =stack_fsync (&add->stack->opts, lock_file_fd); + err =stack_fsync (&add->stack->opts, add->tables_list_lock.fd);if (err <0 ) { err =REFTABLE_IO_ERROR ; goto done; } - err =commit_lock_file (&add->tables_list_lock); + err =flock_commit (&add->tables_list_lock);if (err <0 ) { err =REFTABLE_IO_ERROR ; goto done;
@@ -1160,8 +1157,8 @@ static int stack_compact_range(struct reftable_stack *st,struct reftable_buf new_table_name =REFTABLE_BUF_INIT ;struct reftable_buf new_table_path =REFTABLE_BUF_INIT ;struct reftable_buf table_name =REFTABLE_BUF_INIT ; -struct lock_file tables_list_lock =LOCK_INIT ; -struct lock_file *table_locks =NULL ; +struct reftable_flock tables_list_lock =REFTABLE_FLOCK_INIT ; +struct reftable_flock *table_locks =NULL ;struct reftable_tmpfile new_table =REFTABLE_TMPFILE_INIT ;int is_empty_table =0 , err =0 ;size_t first_to_replace, last_to_replace;
@@ -1179,10 +1176,7 @@ static int stack_compact_range(struct reftable_stack *st,* Hold the lock so that we can read "tables.list" and lock all tables * which are part of the user-specified range. */ - err =hold_lock_file_for_update_timeout (&tables_list_lock, - st->list_file, -LOCK_NO_DEREF , - st->opts.lock_timeout_ms); + err =flock_acquire (&tables_list_lock, st->list_file, st->opts.lock_timeout_ms);if (err <0 ) {if (errno ==EEXIST ) err =REFTABLE_LOCK_ERROR ;
@@ -1205,19 +1199,20 @@ static int stack_compact_range(struct reftable_stack *st,* older process is still busy compacting tables which are preexisting * from the point of view of the newer process. */ -REFTABLE_CALLOC_ARRAY (table_locks, last - first +1 ); +REFTABLE_ALLOC_ARRAY (table_locks, last - first +1 );if (!table_locks) { err =REFTABLE_OUT_OF_MEMORY_ERROR ; goto done; } +for (i =0 ; i < last - first +1 ; i++) + table_locks[i] =REFTABLE_FLOCK_INIT ;for (i = last +1 ; i > first; i--) { err =stack_filename (&table_name, st,reader_name (st->readers[i -1 ]));if (err <0 ) goto done; - err =hold_lock_file_for_update (&table_locks[nlocks], - table_name.buf,LOCK_NO_DEREF ); + err =flock_acquire (&table_locks[nlocks], table_name.buf,0 );if (err <0 ) {/* * When the table is locked already we may do a
@@ -1253,7 +1248,7 @@ static int stack_compact_range(struct reftable_stack *st,* run into file descriptor exhaustion when we compress a lot * of tables. */ - err =close_lock_file_gently (&table_locks[nlocks++]); + err =flock_close (&table_locks[nlocks++]);if (err <0 ) { err =REFTABLE_IO_ERROR ; goto done;
@@ -1265,7 +1260,7 @@ static int stack_compact_range(struct reftable_stack *st,* "tables.list" lock while compacting the locked tables. This allows * concurrent updates to the stack to proceed. */ - err =rollback_lock_file (&tables_list_lock); + err =flock_release (&tables_list_lock);if (err <0 ) { err =REFTABLE_IO_ERROR ; goto done;
@@ -1288,10 +1283,7 @@ static int stack_compact_range(struct reftable_stack *st,* "tables.list". We'll then replace the compacted range of tables with * the new table. */ - err =hold_lock_file_for_update_timeout (&tables_list_lock, - st->list_file, -LOCK_NO_DEREF , - st->opts.lock_timeout_ms); + err =flock_acquire (&tables_list_lock, st->list_file, st->opts.lock_timeout_ms);if (err <0 ) {if (errno ==EEXIST ) err =REFTABLE_LOCK_ERROR ;
@@ -1301,7 +1293,7 @@ static int stack_compact_range(struct reftable_stack *st, }if (st->opts.default_permissions) { -if (chmod (get_lock_file_path (&tables_list_lock), +if (chmod (tables_list_lock.path, st->opts.default_permissions) <0 ) { err =REFTABLE_IO_ERROR ; goto done;
@@ -1456,7 +1448,7 @@ static int stack_compact_range(struct reftable_stack *st, goto done; } - err =write_in_full (get_lock_file_fd (&tables_list_lock), + err =write_in_full (tables_list_lock.fd, tables_list_buf.buf, tables_list_buf.len);if (err <0 ) { err =REFTABLE_IO_ERROR ;
@@ -1464,14 +1456,14 @@ static int stack_compact_range(struct reftable_stack *st, goto done; } - err =stack_fsync (&st->opts,get_lock_file_fd (&tables_list_lock)); + err =stack_fsync (&st->opts, tables_list_lock.fd);if (err <0 ) { err =REFTABLE_IO_ERROR ;unlink (new_table_path.buf); goto done; } - err =commit_lock_file (&tables_list_lock); + err =flock_commit (&tables_list_lock);if (err <0 ) { err =REFTABLE_IO_ERROR ;unlink (new_table_path.buf);
@@ -1492,12 +1484,11 @@ static int stack_compact_range(struct reftable_stack *st,* readers, so it is expected that unlinking tables may fail. */ for (i =0 ; i < nlocks; i++) { -struct lock_file *table_lock = &table_locks[i]; -const char *lock_path =get_lock_file_path (table_lock); +struct reftable_flock *table_lock = &table_locks[i];reftable_buf_reset (&table_name); - err =reftable_buf_add (&table_name, lock_path, -strlen (lock_path) -strlen (".lock" )); + err =reftable_buf_add (&table_name, table_lock->path, +strlen (table_lock->path) -strlen (".lock" ));if (err)continue ;
@@ -1505,9 +1496,9 @@ static int stack_compact_range(struct reftable_stack *st, } done: -rollback_lock_file (&tables_list_lock); +flock_release (&tables_list_lock);for (i =0 ; table_locks && i < nlocks; i++) -rollback_lock_file (&table_locks[i]); +flock_release (&table_locks[i]);reftable_free (table_locks);tmpfile_delete (&new_table);
reftable/system.c +77 -0
86 lines not on this page · raw message
reftable/system.h +44 -1
55 lines not on this page · raw message
t/unit-tests/lib-reftable.c +1 -0
@@ -2,6 +2,7 @@#include "test-lib.h" #include "reftable/constants.h" #include "reftable/writer.h" +#include "strbuf.h" void t_reftable_set_hash (uint8_t *p,int i,enum reftable_hash id) {
t/unit-tests/t-reftable-block.c +1 -0
@@ -11,6 +11,7 @@ license that can be found in the LICENSE file or at#include "reftable/blocksource.h" #include "reftable/constants.h" #include "reftable/reftable-error.h" +#include "strbuf.h" static void t_ref_block_read_write (void ) {
t/unit-tests/t-reftable-pq.c +1 -0
@@ -9,6 +9,7 @@ license that can be found in the LICENSE file or at#include "test-lib.h" #include "reftable/constants.h" #include "reftable/pq.h" +#include "strbuf.h" static void merged_iter_pqueue_check (const struct merged_iter_pqueue *pq) {
t/unit-tests/t-reftable-readwrite.c +1 -0
@@ -13,6 +13,7 @@ license that can be found in the LICENSE file or at#include "reftable/reader.h" #include "reftable/reftable-error.h" #include "reftable/reftable-writer.h" +#include "strbuf.h" static const int update_index =5 ;
t/unit-tests/t-reftable-stack.c +2 -0
@@ -13,6 +13,8 @@ license that can be found in the LICENSE file or at#include "reftable/reader.h" #include "reftable/reftable-error.h" #include "reftable/stack.h" +#include "strbuf.h" +#include "tempfile.h" #include <dirent.h> static void clear_dir (const char *dirname)
Signature · 1 line
2.47.0.274.g962d0b743d.dirty
Patrick Steinhardt <[email protected]> writes:
[snip]
Quoted 16 lines
Range-diff against v2:
1: 2b7d4e28529 = 1: 2b7d4e28529 reftable/system: move "dir.h" to its only user 2: 38cfe85bf5b = 2: 38cfe85bf5b reftable: explicitly handle hash format IDs 3: 745c1a070dd = 3: 745c1a070dd reftable/system: stop depending on "hash.h" 4: 7782652b975 = 4: 7782652b975 reftable/stack: stop using `fsync_component()` directly 5: b15daefbc83 ! 5: 430be1045d6 reftable/system: provide thin wrapper for tempfile subsystem@@
reftable/system.h: license that can be found in the LICENSE file or at
+
+/*
+ * Rename the temporary file to the provided path. The temporary file must be
-+ * active. Return 0 on success, a reftable error code on error. ++ * active. Return 0 on success, a reftable error code on error. Deactivates the ++ * temporary file.
+ */
+inttmpfile_rename(struct reftable_tmpfile *t, const char *path);
Quoted 2 lines
6: 83949837a29 = 6: b9ffdc605b9 reftable/stack: drop only use of `get_locked_file_path()` 7: 80fe5bc5e10 = 7: e1ac1cc2e67 reftable/system: provide thin wrapper for lockfile subsystem
Post my previous review, this range diff looks good. Thanks
- Karthik
Quoted 20 lines · karthik nayak
Patrick Steinhardt <[email protected]> writes:
[snip]
Quoted 16 lines
Range-diff against v2:
1: 2b7d4e28529 = 1: 2b7d4e28529 reftable/system: move "dir.h" to its only user 2: 38cfe85bf5b = 2: 38cfe85bf5b reftable: explicitly handle hash format IDs 3: 745c1a070dd = 3: 745c1a070dd reftable/system: stop depending on "hash.h" 4: 7782652b975 = 4: 7782652b975 reftable/stack: stop using `fsync_component()` directly 5: b15daefbc83 ! 5: 430be1045d6 reftable/system: provide thin wrapper for tempfile subsystem@@
reftable/system.h: license that can be found in the LICENSE file or at
+
+/*
+ * Rename the temporary file to the provided path. The temporary file must be
-+ * active. Return 0 on success, a reftable error code on error. ++ * active. Return 0 on success, a reftable error code on error. Deactivates the ++ * temporary file.
+ */
+inttmpfile_rename(struct reftable_tmpfile *t, const char *path);
Quoted 4 lines
Quoted 2 lines
6: 83949837a29 = 6: b9ffdc605b9 reftable/stack: drop only use of `get_locked_file_path()` 7: 80fe5bc5e10 = 7: e1ac1cc2e67 reftable/system: provide thin wrapper for lockfile subsystemPost my previous review, this range diff looks good. Thanks
Thanks, both. Queued.