reftable library
Subsystems (2)
Re: [PATCH v6 18/20] Reftable support for git-core
In reply to Han-Wen NienhuysQuoted 16 lines · Han-Wen Nienhuys
On Tue, Apr 13, 2021 at 9:18 AM Ævar Arnfjörð Bjarmason <[email protected]> wrote:
[...]Quoted 13 lines
I understand that getting this series to land has been a pain, but this sort of thing doesn't inspire confidence.
Manually running it under reftable anyway I have 17 out of 81 tests failing. A lot of tests won't run under reftable because they're of the form:
test_when_finished "remove_object $tag" && echo $tag >.git/refs/tags/wrong && test_when_finished "git update-ref -d refs/tags/wrong" &&I.e. we have some hack to get around update-ref guarding things for us.
Quoted 8 lines
Quoted 3 lines
I would think that a better approach here would be to start with some (per-se unrelated) series to teach update-ref some mode like hash-object's --literally, i.e. "YOLO this ref update".
I disagree. I think this would be a job better suited to a test-helper. Then we don't put tools into users' hands that potentially corrupt the repository. I don't understand why hash-object --literally is not a test helper either.
I don't feel too strongly either way, and in any case that's a discussion for any such mass test update series, and separate from the reftable series per-se.
I do lean on the side of shipping a thing like that as part of our plumbing, of course with very prominent warnings, just as we do --literally.
It is useful for not only our test suite, but also for others to reproduce issues locally with the ref and object store going out of sync.
Until now there hasn't been much of a need in practice, the file backend format is trivially understood and you can just echo a bad value to a file.
But if I or anyone else encounters some Heisenbug in the wild where we suspect the ref and object store went out of syncy, and we're using reftable, it would be very handy to test that with an "update-ref --literally", instead of having to locally recompile git, or otherwise munge the opaque reftable format.
Quoted 12 lines
Quoted 7 lines
Then adjust our various tests that move, clobber, or intentionally corrupt things in the refstore to use that helper/option.
At that point we can actually run things like
t1450-fsck.sh, maybe we simply have to skip some of them because we think the sort of corruption we're testing/worried about is impossible with reftable, but that in itself is *very* interesting.One reason for skipping some files wholesale, is that quite a few tests are not isolated in their test functions, so skipping an individual test functions affects follow-on tests. I can't remember if t1450 is one of them, though.
[...]
Quoted 10 lines
Quoted 9 lines
I.e. do we not have to handle certain edge cases at all in fsck and friends because of inherent properties of the reftable backend (e.g. I'd assume, but don't know, that we're not likely to get one corrupt ref entry with it), but of course we can have a ref pointing to a loose object that then gets removed, and is thus corrupt.
Anyway. I'm speculating, but that's also my point, that I'm having to speculate. That comes down to us having a new GIT_TEST_* mode that doesn't pass the test suite._
Quoted 7 lines
The speculation is warranted, but there are currently about 80 files that have some sort of test failure, and some of these are for quite common workflows. I would rather focus on those before looking at tests like fsck. Functional failures may have far reaching implications on the code structure (example: worktrees considerably complicated both the reftable library and its git-core glue), while corruption issues if they occur, will be due to localized bugs.
I'm not sure from reading this if/where we disagree, so let's enumerate and see where we land point-by-point:
1. The test suite must pass 100% for any new GIT_TEST_X mode as it lands.
2. It must do so in a meaningful way (i.e. not skipping entire files, if some/many of the tests are relevant / stress the codepath involved)
3. #2 should be there as the series lands.
My comments on that:
1: I feel strongly that #1 is an absolute prerequisite for integrating
this or any other major core surgery.
E.g. now we can't do any meaningful cross-platform tests for this to see how it breaks on our various supported platforms.
We're also in V6 of this and apparently finding an easily discoverable segfault now, if v7 of this had a GIT_TEST_REFTABLE in the relevant ci/* code ...
I realize that that's hard, e.g. we had to do large refactoring for all of the commit-graph, protocol v2, sha256 etc. modes, but it really pays off. E.g. for protocol v2 upload-pack not paying attention to hideRefs was discovered that way.
2-3: I do think we should have this too, but would be willing to be
convinced otherwise. E.g. we have a "gcov" target, once we have #1 passing do we se that under GIT_TEST_REFTABLE=1 we have the same (or trivially explainable) test coverage, or are we simply skipping a large part of the fsck etc. code (some/much of which will have to do with refs).
Even more than test coverage I think this is interesting to see what we *don't* cover, as I noted upthread. I.e. can we skip large parts of some sanity checking "if (reftable)"? I suspect so, but without the coverage we won't be able to tell.
I realize the above would be a lot of work, probably a >50 patch series to t/* if previous major GIT_TEST_* refactorings are any indication.
But if we agree that's a viable way forward that's not all work that you'd have to do.
Re: [PATCH v6 18/20] Reftable support for git-core
In reply to Han-Wen NienhuysQuoted 8 lines · Han-Wen Nienhuys
Quoted 3 lines
I would think that a better approach here would be to start with some (per-se unrelated) series to teach update-ref some mode like hash-object's --literally, i.e. "YOLO this ref update".
I disagree. I think this would be a job better suited to a test-helper. Then we don't put tools into users' hands that potentially corrupt the repository. I don't understand why hash-object --literally is not a test helper either.
As the person who invented the "--literally" option, I'd have to agree with this assessment. It does make life a little bit easier for those who hack on Git codebase and reimplementations of it, but little practical value for those who use Git every day [*].
If it were a tool to _dump_ the contents of a possibly corrupt object that the "--literally" option would have produced to make it easier to see by humans, I might be pursuaded to say that such a feature may be better in end-user accessible subcommands. But the reason why we invented "--literally" was specifically to create corrupt objects in test repository to see end-user accessible tools behave, and in hindsight, such a use case shouldn't have been a good justification to add the option.
I wasn't following the discussion of this particular patch closely, so I do not know what is being discussed is a tool on the diagnosis side (for which I may say it is OK to give to end-usres) or on the currupting side (for which I would regret to add to end-user tools), but hopefully the above would help guiding the decision between the two.
Thanks.
[Footnote]
* ... for any purpose other than creating a corrupt repository, asking somebody who is or claims to be a Git expert to figure out what is wrong in his or her repository, and either waste expert's time or have fun by watching the expert fail and gets embarrased, that is ;-)
PATCH v6 19/20 git-prompt: prepare for reftable refs backend
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: =?UTF-8?q?SZEDER=20G=C3=A1bor?= <[email protected]>
In our git-prompt script we strive to use Bash builtins wherever possible, because fork()-ing subshells for command substitutions and fork()+exec()-ing Git commands are expensive on some platforms. We even read and parse '.git/HEAD' using Bash builtins to get the name of the current branch [1]. However, the upcoming reftable refs backend won't use '.git/HEAD' at all, but will write an invalid refname as placeholder for backwards compatibility instead, which will break our git-prompt script.
Update the git-prompt script to recognize the placeholder '.git/HEAD' written by the reftable backend (its content is specified in the reftable specs), and then fall back to use 'git symbolic-ref' to get the name of the current branch.
[1] 3a43c4b5bd (bash prompt: use bash builtins to find out current
branch, 2011-03-31)
--- contrib/completion/git-prompt.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
contrib/completion/git-prompt.sh +6 -1
@@ -478,10 +478,15 @@ __git_ps1 () if ! __git_eread "$g/HEAD" head; then return $exit fi - # is it a symbolic ref? b="${head#ref: }" if [ "$head" = "$b" ]; then detached=yes + elif [ "$b" = "refs/heads/.invalid" ]; then + # Reftable + b="$(git symbolic-ref HEAD 2>/dev/null)" || + detached=yes + fi + if [ "$detached" = yes ]; then b="$( case "${GIT_PS1_DESCRIBE_STYLE-}" in (contains)
Signature · 1 line
gitgitgadget
PATCH v6 20/20 Add "test-tool dump-reftable" command.
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
This command dumps individual tables or a stack of of tables.
--- Makefile | 1 + reftable/dump.c | 67 ++++++++++++++++++++-------------------- t/helper/test-reftable.c | 5 +++ t/helper/test-tool.c | 1 + t/helper/test-tool.h | 1 + 5 files changed, 42 insertions(+), 33 deletions(-)
Makefile +1 -0
@@ -2420,6 +2420,7 @@ REFTABLE_OBJS += reftable/zlib-compat.o REFTABLE_TEST_OBJS += reftable/basics_test.o REFTABLE_TEST_OBJS += reftable/block_test.o +REFTABLE_TEST_OBJS += reftable/dump.o REFTABLE_TEST_OBJS += reftable/merged_test.o REFTABLE_TEST_OBJS += reftable/pq_test.o REFTABLE_TEST_OBJS += reftable/record_test.o
reftable/dump.c +34 -33
144 lines not on this page · raw message
t/helper/test-reftable.c +5 -0
@@ -14,3 +14,8 @@ int cmd__reftable(int argc, const char **argv)tree_test_main (argc, argv);return 0 ; } + +int cmd__dump_reftable (int argc,const char **argv) +{ +return reftable_dump_main (argc, (char *const *)argv); +}
t/helper/test-tool.c +1 -0
@@ -57,6 +57,7 @@ static struct test_cmd cmds[] = { { "read-midx", cmd__read_midx }, { "ref-store", cmd__ref_store }, { "reftable", cmd__reftable }, + { "dump-reftable", cmd__dump_reftable }, { "regex", cmd__regex }, { "repository", cmd__repository }, { "revision-walking", cmd__revision_walking },
t/helper/test-tool.h +1 -0
@@ -18,6 +18,7 @@ int cmd__dump_cache_tree(int argc, const char **argv);int cmd__dump_fsmonitor (int argc,const char **argv);int cmd__dump_split_index (int argc,const char **argv);int cmd__dump_untracked_cache (int argc,const char **argv); +int cmd__dump_reftable (int argc,const char **argv);int cmd__example_decorate (int argc,const char **argv);int cmd__fast_rebase (int argc,const char **argv);int cmd__genrandom (int argc,const char **argv);
Signature · 1 line
gitgitgadget
v7 cover reftable library
In reply to Han-Wen Nienhuys via GitGitGadgetThis splits the giant commit from
https://github.com/gitgitgadget/git/pull/539 into a series of smaller commits, which build and have unittests.
Changes relative to last series: version 12 Apr 2021:
* AEvar's comments: zlib into compat/, void* casts, use implicit pointer <-> boolean conversions.
* Plug memory leak in indexed reverse lookup.
* Take SHA1/SHA256 constants from git headers.
* Fix branch rename (branch -M)
* Fix stash (support reflog deletion.)
* Move debug printing logic out of dump.c
* Fix reflog --dry-run expire.
* Implement branch copy (branch -c)
* Fix deletion of 0 branches
* Fix transactions without REF_HAVE_OLD
* Add requirement REFFILES for test cases that are specific to the files backend.
* Fixes for various test files.
The commits up to "hash.h: provide constants for the hash IDs" should be good to merge to 'next'.
There are several test fixups, but I've put them in another series because GGG enforces max 30 commits.
Due to using uncompress2, this build fails on the Linux32 builder; what is the magic incantation to run autoconf? There are other unexplained failures in t0031-reftable.sh on CI that I haven't been able to look into. The test passes for me, locally.
Han-Wen Nienhuys (27):
refs: ref_iterator_peel returns boolean, rather than peel_status
refs: document reflog_expire_fn's flag argument refs/debug: trace into reflog expiry too
hash.h: provide constants for the hash IDs
init-db: set the_repository->hash_algo early on
reftable: add LICENSE
reftable: add error related functionality
reftable: utility functions
reftable: add blocksource, an abstraction for random access reads
reftable: (de)serialization for the polymorphic record type. Provide zlib's uncompress2 from compat/zlib-compat.c
reftable: reading/writing blocks
reftable: a generic binary tree implementation
reftable: write reftable files
reftable: generic interface to tables
reftable: read reftable files
reftable: reftable file level tests
reftable: add a heap-based priority queue for reftable records
reftable: add merged table view
reftable: implement refname validation
reftable: implement stack, a mutable database of reftable files.
reftable: add dump utility
Reftable support for git-core
Add "test-tool dump-reftable" command.
t1301: document what needs to be done for REFTABLE t1401,t2011: parameterize HEAD.lock for REFTABLE
t1404: annotate test cases with REFFILES
SZEDER Gábor (1):
git-prompt: prepare for reftable refs backend
Documentation/config/extensions.txt | 9 + .../technical/repository-version.txt | 7 + Makefile | 54 +- builtin/clone.c | 5 +- builtin/init-db.c | 76 +- builtin/stash.c | 8 +- builtin/worktree.c | 27 +- cache.h | 8 +- compat/.gitattributes | 1 + compat/zlib-uncompress2.c | 92 + config.mak.uname | 2 +- configure.ac | 13 + contrib/buildsystems/CMakeLists.txt | 14 +- contrib/buildsystems/Generators/Vcxproj.pm | 11 +- contrib/completion/git-prompt.sh | 7 +- git-compat-util.h | 4 + hash.h | 6 + object-file.c | 6 +- refs.c | 28 +- refs.h | 3 + refs/debug.c | 47 +- refs/ref-cache.c | 2 +- refs/refs-internal.h | 8 + refs/reftable-backend.c | 1616 +++++++++++++++++ reftable/LICENSE | 31 + reftable/basics.c | 128 ++ reftable/basics.h | 60 + reftable/basics_test.c | 98 + reftable/block.c | 446 +++++ reftable/block.h | 127 ++ reftable/block_test.c | 121 ++ reftable/blocksource.c | 148 ++ reftable/blocksource.h | 22 + reftable/constants.h | 21 + reftable/dump.c | 100 + reftable/error.c | 41 + reftable/generic.c | 169 ++ reftable/generic.h | 32 + reftable/iter.c | 194 ++ reftable/iter.h | 69 + reftable/merged.c | 362 ++++ reftable/merged.h | 35 + reftable/merged_test.c | 292 +++ reftable/pq.c | 115 ++ reftable/pq.h | 32 + reftable/pq_test.c | 72 + reftable/publicbasics.c | 58 + reftable/reader.c | 794 ++++++++ reftable/reader.h | 66 + reftable/readwrite_test.c | 621 +++++++ reftable/record.c | 1202 ++++++++++++ reftable/record.h | 139 ++ reftable/record_test.c | 407 +++++ reftable/refname.c | 209 +++ reftable/refname.h | 29 + reftable/refname_test.c | 102 ++ reftable/reftable-blocksource.h | 49 + reftable/reftable-error.h | 62 + reftable/reftable-generic.h | 47 + reftable/reftable-iterator.h | 39 + reftable/reftable-malloc.h | 18 + reftable/reftable-merged.h | 72 + reftable/reftable-reader.h | 101 ++ reftable/reftable-record.h | 114 ++ reftable/reftable-stack.h | 128 ++ reftable/reftable-tests.h | 23 + reftable/reftable-writer.h | 147 ++ reftable/reftable.c | 115 ++ reftable/stack.c | 1395 ++++++++++++++ reftable/stack.h | 41 + reftable/stack_test.c | 940 ++++++++++ reftable/system.h | 24 + reftable/test_framework.c | 23 + reftable/test_framework.h | 53 + reftable/tree.c | 63 + reftable/tree.h | 34 + reftable/tree_test.c | 61 + reftable/writer.c | 690 +++++++ reftable/writer.h | 50 + repository.c | 2 + repository.h | 3 + setup.c | 9 +- t/helper/test-reftable.c | 21 + t/helper/test-tool.c | 4 +- t/helper/test-tool.h | 2 + t/t0031-reftable.sh | 271 +++ t/t0032-reftable-unittest.sh | 15 + t/t1301-shared-repo.sh | 8 +- t/t1401-symbolic-ref.sh | 11 +- t/t1404-update-ref-errors.sh | 86 +- t/t1409-avoid-packing-refs.sh | 6 + t/t1450-fsck.sh | 6 + t/t2011-checkout-invalid-head.sh | 11 +- t/t3210-pack-refs.sh | 6 + t/test-lib.sh | 5 + 95 files changed, 13065 insertions(+), 86 deletions(-) create mode 100644 compat/.gitattributes create mode 100644 compat/zlib-uncompress2.c create mode 100644 refs/reftable-backend.c create mode 100644 reftable/LICENSE create mode 100644 reftable/basics.c create mode 100644 reftable/basics.h create mode 100644 reftable/basics_test.c create mode 100644 reftable/block.c create mode 100644 reftable/block.h create mode 100644 reftable/block_test.c create mode 100644 reftable/blocksource.c create mode 100644 reftable/blocksource.h create mode 100644 reftable/constants.h create mode 100644 reftable/dump.c create mode 100644 reftable/error.c create mode 100644 reftable/generic.c create mode 100644 reftable/generic.h create mode 100644 reftable/iter.c create mode 100644 reftable/iter.h create mode 100644 reftable/merged.c create mode 100644 reftable/merged.h create mode 100644 reftable/merged_test.c create mode 100644 reftable/pq.c create mode 100644 reftable/pq.h create mode 100644 reftable/pq_test.c create mode 100644 reftable/publicbasics.c create mode 100644 reftable/reader.c create mode 100644 reftable/reader.h create mode 100644 reftable/readwrite_test.c create mode 100644 reftable/record.c create mode 100644 reftable/record.h create mode 100644 reftable/record_test.c create mode 100644 reftable/refname.c create mode 100644 reftable/refname.h create mode 100644 reftable/refname_test.c create mode 100644 reftable/reftable-blocksource.h create mode 100644 reftable/reftable-error.h create mode 100644 reftable/reftable-generic.h create mode 100644 reftable/reftable-iterator.h create mode 100644 reftable/reftable-malloc.h create mode 100644 reftable/reftable-merged.h create mode 100644 reftable/reftable-reader.h create mode 100644 reftable/reftable-record.h create mode 100644 reftable/reftable-stack.h create mode 100644 reftable/reftable-tests.h create mode 100644 reftable/reftable-writer.h create mode 100644 reftable/reftable.c create mode 100644 reftable/stack.c create mode 100644 reftable/stack.h create mode 100644 reftable/stack_test.c create mode 100644 reftable/system.h create mode 100644 reftable/test_framework.c create mode 100644 reftable/test_framework.h create mode 100644 reftable/tree.c create mode 100644 reftable/tree.h create mode 100644 reftable/tree_test.c create mode 100644 reftable/writer.c create mode 100644 reftable/writer.h create mode 100644 t/helper/test-reftable.c create mode 100755 t/t0031-reftable.sh create mode 100755 t/t0032-reftable-unittest.sh
base-commit: 54a391711554ed41b4b0792cfef004abc74893bd
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-847%2Fhanwen%2Flibreftable-v7
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-847/hanwen/libreftable-v7
Pull-Request: https://github.com/git/git/pull/847
Range-diff vs v6:
-: ------------ > 1: 8103a80394ae refs: ref_iterator_peel returns boolean, rather than peel_status
-: ------------ > 2: 554bb1ac3aed refs: document reflog_expire_fn's flag argument
-: ------------ > 3: 9ae5ddff6aed refs/debug: trace into reflog expiry too
-: ------------ > 4: f0216ae20b69 hash.h: provide constants for the hash IDs
1: 15b8306fbe4b = 5: fa64aa9ce7c1 init-db: set the_repository->hash_algo early on
2: 02bd1f40fa4e = 6: 8a960fb77fa9 reftable: add LICENSE
3: bffa33c012ad = 7: 5177919a3fd2 reftable: add error related functionality
4: df8003cb9a7d ! 8: a24e9e661616 reftable: utility functions
@@ reftable/basics.c (new)
+void free_names(char **a)
+{
+ char **p;
-+ if (a == NULL) {
++ if (!a) {
+ return;
+ }
+ for (p = a; *p; p++) {
@@ reftable/basics_test.c (new)
+
+static int binsearch_func(size_t i, void *void_args)
+{
-+ struct binsearch_args *args = (struct binsearch_args *)void_args;
++ struct binsearch_args *args = void_args;
+
+ return args->key < args->arr[i];
+}
@@ reftable/basics_test.c (new)
+ parse_names(in, strlen(in), &out);
+ EXPECT(!strcmp(out[0], "a"));
+ EXPECT(!strcmp(out[1], "b"));
-+ EXPECT(out[2] == NULL);
++ EXPECT(!out[2]);
+ free_names(out);
+}
+
@@ reftable/basics_test.c (new)
+ char **out = NULL;
+ parse_names(in, strlen(in), &out);
+ EXPECT(!strcmp(out[0], "a"));
-+ EXPECT(out[1] == NULL);
++ EXPECT(!out[1]);
+ free_names(out);
+}
+
@@ reftable/publicbasics.c (new)
+{
+ switch (id) {
+ case 0:
-+ case SHA1_ID:
-+ return SHA1_SIZE;
-+ case SHA256_ID:
-+ return SHA256_SIZE;
++ case GIT_SHA1_HASH_ID:
++ return GIT_SHA1_RAWSZ;
++ case GIT_SHA256_HASH_ID:
++ return GIT_SHA256_RAWSZ;
+ }
+ abort();
+}
@@ reftable/system.h (new)
+#ifndef SYSTEM_H
+#define SYSTEM_H
+
++// This header glues the reftable library to the rest of Git
++
+#include "git-compat-util.h"
+#include "strbuf.h"
++#include "hash.h" /* hash ID, sizes.*/
++#include "dir.h" /* remove_dir_recursively, for tests.*/
+
+#include <zlib.h>
+
+struct strbuf;
-+/* In git, this is declared in dir.h */
-+int remove_dir_recursively(struct strbuf *path, int flags);
-+
-+#define SHA1_ID 0x73686131
-+#define SHA256_ID 0x73323536
-+#define SHA1_SIZE 20
-+#define SHA256_SIZE 32
-+
-+/* This is uncompress2, which is only available in zlib as of 2017.
-+ */
-+int uncompress_return_consumed(Bytef *dest, uLongf *destLen,
-+ const Bytef *source, uLong *sourceLen);
+int hash_size(uint32_t id);
+
+#endif
@@ reftable/test_framework.c (new)
+
+void set_test_hash(uint8_t *p, int i)
+{
-+ memset(p, (uint8_t)i, hash_size(SHA1_ID));
++ memset(p, (uint8_t)i, hash_size(GIT_SHA1_HASH_ID));
+}
+
-+int strbuf_add_void(void *b, const void *data, size_t sz)
++ssize_t strbuf_add_void(void *b, const void *data, size_t sz)
+{
-+ strbuf_add((struct strbuf *)b, data, sz);
++ strbuf_add(b, data, sz);
+ return sz;
+}
@@ reftable/test_framework.h (new)
+
+/* Like strbuf_add, but suitable for passing to reftable_new_writer
+ */
-+int strbuf_add_void(void *b, const void *data, size_t sz);
++ssize_t strbuf_add_void(void *b, const void *data, size_t sz);
+
+#endif 5: 6807af53e9a0 ! 9: 5d54e6fac99b reftable: add blocksource, an abstraction for random access reads
@@ reftable/blocksource.c (new)
+static int strbuf_read_block(void *v, struct reftable_block *dest, uint64_t off,
+ uint32_t size)
+{
-+ struct strbuf *b = (struct strbuf *)v;
++ struct strbuf *b = v;
+ assert(off + size <= b->len);
+ dest->data = reftable_calloc(size);
+ memcpy(dest->data, b->buf + off, size);
@@ reftable/blocksource.c (new)
+void block_source_from_strbuf(struct reftable_block_source *bs,
+ struct strbuf *buf)
+{
-+ assert(bs->ops == NULL);
++ assert(!bs->ops);
+ bs->ops = &strbuf_vtable;
+ bs->arg = buf;
+}
@@ reftable/blocksource.c (new)
+static int file_read_block(void *v, struct reftable_block *dest, uint64_t off,
+ uint32_t size)
+{
-+ struct file_block_source *b = (struct file_block_source *)v;
++ struct file_block_source *b = v;
+ assert(off + size <= b->size);
+ dest->data = reftable_malloc(size);
+ if (pread(b->fd, dest->data, size, off) != size)
@@ reftable/blocksource.c (new)
+ p->size = st.st_size;
+ p->fd = fd;
+
-+ assert(bs->ops == NULL);
++ assert(!bs->ops);
+ bs->ops = &file_vtable;
+ bs->arg = p;
+ return 0;
6: 78a352c96f82 ! 10: 028dd13e701b reftable: (de)serialization for the polymorphic record type.
@@ reftable/record.c (new)
+static void reftable_ref_record_copy_from(void *rec, const void *src_rec,
+ int hash_size)
+{
-+ struct reftable_ref_record *ref = (struct reftable_ref_record *)rec;
-+ struct reftable_ref_record *src = (struct reftable_ref_record *)src_rec;
++ struct reftable_ref_record *ref = rec;
++ const struct reftable_ref_record *src = src_rec;
+ assert(hash_size > 0);
+
+ /* This is simple and correct, but we could probably reuse the hash
+ * fields. */
+ reftable_ref_record_release(ref);
-+ if (src->refname != NULL) {
++ if (src->refname) {
+ ref->refname = xstrdup(src->refname);
+ }
+ ref->update_index = src->update_index;
@@ reftable/record.c (new)
+static void hex_format(char *dest, uint8_t *src, int hash_size)
+{
+ assert(hash_size > 0);
-+ if (src != NULL) {
++ if (src) {
+ int i = 0;
+ for (i = 0; i < hash_size; i++) {
+ dest[2 * i] = hexdigit(src[i] >> 4);
@@ reftable/record.c (new)
+void reftable_ref_record_print(struct reftable_ref_record *ref,
+ uint32_t hash_id)
+{
-+ char hex[2 * SHA256_SIZE + 1] = { 0 }; /* BUG */
++ char hex[2 * GIT_SHA256_RAWSZ + 1] = { 0 }; /* BUG */
+ printf("ref{%s(%" PRIu64 ") ", ref->refname, ref->update_index);
+ switch (ref->value_type) {
+ case REFTABLE_REF_SYMREF:
@@ reftable/record.c (new)
+
+static void reftable_ref_record_release_void(void *rec)
+{
-+ reftable_ref_record_release((struct reftable_ref_record *)rec);
++ reftable_ref_record_release(rec);
+}
+
+void reftable_ref_record_release(struct reftable_ref_record *ref)
@@ reftable/record.c (new)
+ uint8_t val_type, struct string_view in,
+ int hash_size)
+{
-+ struct reftable_ref_record *r = (struct reftable_ref_record *)rec;
++ struct reftable_ref_record *r = rec;
+ struct string_view start = in;
+ uint64_t update_index = 0;
+ int n = get_var_int(&update_index, &in);
@@ reftable/record.c (new)
+
+static void reftable_obj_record_release(void *rec)
+{
-+ struct reftable_obj_record *obj = (struct reftable_obj_record *)rec;
++ struct reftable_obj_record *obj = rec;
+ FREE_AND_NULL(obj->hash_prefix);
+ FREE_AND_NULL(obj->offsets);
+ memset(obj, 0, sizeof(struct reftable_obj_record));
@@ reftable/record.c (new)
+static void reftable_obj_record_copy_from(void *rec, const void *src_rec,
+ int hash_size)
+{
-+ struct reftable_obj_record *obj = (struct reftable_obj_record *)rec;
++ struct reftable_obj_record *obj = rec;
+ const struct reftable_obj_record *src =
+ (const struct reftable_obj_record *)src_rec;
+ int olen;
@@ reftable/record.c (new)
+
+static uint8_t reftable_obj_record_val_type(const void *rec)
+{
-+ struct reftable_obj_record *r = (struct reftable_obj_record *)rec;
++ const struct reftable_obj_record *r = rec;
+ if (r->offset_len > 0 && r->offset_len < 8)
+ return r->offset_len;
+ return 0;
@@ reftable/record.c (new)
+static int reftable_obj_record_encode(const void *rec, struct string_view s,
+ int hash_size)
+{
-+ struct reftable_obj_record *r = (struct reftable_obj_record *)rec;
++ const struct reftable_obj_record *r = rec;
+ struct string_view start = s;
+ int i = 0;
+ int n = 0;
@@ reftable/record.c (new)
+ int hash_size)
+{
+ struct string_view start = in;
-+ struct reftable_obj_record *r = (struct reftable_obj_record *)rec;
++ struct reftable_obj_record *r = rec;
+ uint64_t count = val_type;
+ int n = 0;
+ uint64_t last;
@@ reftable/record.c (new)
+void reftable_log_record_print(struct reftable_log_record *log,
+ uint32_t hash_id)
+{
-+ char hex[SHA256_SIZE + 1] = { 0 };
++ char hex[GIT_SHA256_RAWSZ + 1] = { 0 };
+
+ switch (log->value_type) {
+ case REFTABLE_LOG_DELETION:
@@ reftable/record.c (new)
+static void reftable_log_record_copy_from(void *rec, const void *src_rec,
+ int hash_size)
+{
-+ struct reftable_log_record *dst = (struct reftable_log_record *)rec;
++ struct reftable_log_record *dst = rec;
+ const struct reftable_log_record *src =
+ (const struct reftable_log_record *)src_rec;
+
+ reftable_log_record_release(dst);
+ *dst = *src;
-+ if (dst->refname != NULL) {
++ if (dst->refname) {
+ dst->refname = xstrdup(dst->refname);
+ }
+ switch (dst->value_type) {
+ case REFTABLE_LOG_DELETION:
+ break;
+ case REFTABLE_LOG_UPDATE:
-+ if (dst->update.email != NULL) {
++ if (dst->update.email) {
+ dst->update.email = xstrdup(dst->update.email);
+ }
-+ if (dst->update.name != NULL) {
++ if (dst->update.name) {
+ dst->update.name = xstrdup(dst->update.name);
+ }
-+ if (dst->update.message != NULL) {
++ if (dst->update.message) {
+ dst->update.message = xstrdup(dst->update.message);
+ }
+
-+ if (dst->update.new_hash != NULL) {
++ if (dst->update.new_hash) {
+ dst->update.new_hash = reftable_malloc(hash_size);
+ memcpy(dst->update.new_hash, src->update.new_hash,
+ hash_size);
+ }
-+ if (dst->update.old_hash != NULL) {
++ if (dst->update.old_hash) {
+ dst->update.old_hash = reftable_malloc(hash_size);
+ memcpy(dst->update.old_hash, src->update.old_hash,
+ hash_size);
@@ reftable/record.c (new)
+
+static void reftable_log_record_release_void(void *rec)
+{
-+ struct reftable_log_record *r = (struct reftable_log_record *)rec;
++ struct reftable_log_record *r = rec;
+ reftable_log_record_release(r);
+}
+
@@ reftable/record.c (new)
+ return reftable_log_record_is_deletion(log) ? 0 : 1;
+}
+
-+static uint8_t zero[SHA256_SIZE] = { 0 };
++static uint8_t zero[GIT_SHA256_RAWSZ] = { 0 };
+
+static int reftable_log_record_encode(const void *rec, struct string_view s,
+ int hash_size)
+{
-+ struct reftable_log_record *r = (struct reftable_log_record *)rec;
++ const struct reftable_log_record *r = rec;
+ struct string_view start = s;
+ int n = 0;
+ uint8_t *oldh = NULL;
@@ reftable/record.c (new)
+
+ oldh = r->update.old_hash;
+ newh = r->update.new_hash;
-+ if (oldh == NULL) {
++ if (!oldh) {
+ oldh = zero;
+ }
-+ if (newh == NULL) {
++ if (!newh) {
+ newh = zero;
+ }
+
@@ reftable/record.c (new)
+ int hash_size)
+{
+ struct string_view start = in;
-+ struct reftable_log_record *r = (struct reftable_log_record *)rec;
++ struct reftable_log_record *r = rec;
+ uint64_t max = 0;
+ uint64_t ts = 0;
+ struct strbuf dest = STRBUF_INIT;
@@ reftable/record.c (new)
+static int null_streq(char *a, char *b)
+{
+ char *empty = "";
-+ if (a == NULL)
++ if (!a)
+ a = empty;
+
-+ if (b == NULL)
++ if (!b)
+ b = empty;
+
+ return 0 == strcmp(a, b);
@@ reftable/record.c (new)
+
+static int zero_hash_eq(uint8_t *a, uint8_t *b, int sz)
+{
-+ if (a == NULL)
++ if (!a)
+ a = zero;
+
-+ if (b == NULL)
++ if (!b)
+ b = zero;
+
+ return !memcmp(a, b, sz);
@@ reftable/record.c (new)
+
+static void reftable_index_record_key(const void *r, struct strbuf *dest)
+{
-+ struct reftable_index_record *rec = (struct reftable_index_record *)r;
++ const struct reftable_index_record *rec = r;
+ strbuf_reset(dest);
+ strbuf_addbuf(dest, &rec->last_key);
+}
@@ reftable/record.c (new)
+static void reftable_index_record_copy_from(void *rec, const void *src_rec,
+ int hash_size)
+{
-+ struct reftable_index_record *dst = (struct reftable_index_record *)rec;
-+ struct reftable_index_record *src =
-+ (struct reftable_index_record *)src_rec;
++ struct reftable_index_record *dst = rec;
++ const struct reftable_index_record *src = src_rec;
+
+ strbuf_reset(&dst->last_key);
+ strbuf_addbuf(&dst->last_key, &src->last_key);
@@ reftable/record.c (new)
+
+static void reftable_index_record_release(void *rec)
+{
-+ struct reftable_index_record *idx = (struct reftable_index_record *)rec;
++ struct reftable_index_record *idx = rec;
+ strbuf_release(&idx->last_key);
+}
+
@@ reftable/record.c (new)
+ int hash_size)
+{
+ struct string_view start = in;
-+ struct reftable_index_record *r = (struct reftable_index_record *)rec;
++ struct reftable_index_record *r = rec;
+ int n = 0;
+
+ strbuf_reset(&r->last_key);
@@ reftable/record.c (new)
+void reftable_record_from_ref(struct reftable_record *rec,
+ struct reftable_ref_record *ref_rec)
+{
-+ assert(rec->ops == NULL);
++ assert(!rec->ops);
+ rec->data = ref_rec;
+ rec->ops = &reftable_ref_record_vtable;
+}
@@ reftable/record.c (new)
+void reftable_record_from_obj(struct reftable_record *rec,
+ struct reftable_obj_record *obj_rec)
+{
-+ assert(rec->ops == NULL);
++ assert(!rec->ops);
+ rec->data = obj_rec;
+ rec->ops = &reftable_obj_record_vtable;
+}
@@ reftable/record.c (new)
+void reftable_record_from_index(struct reftable_record *rec,
+ struct reftable_index_record *index_rec)
+{
-+ assert(rec->ops == NULL);
++ assert(!rec->ops);
+ rec->data = index_rec;
+ rec->ops = &reftable_index_record_vtable;
+}
@@ reftable/record.c (new)
+void reftable_record_from_log(struct reftable_record *rec,
+ struct reftable_log_record *log_rec)
+{
-+ assert(rec->ops == NULL);
++ assert(!rec->ops);
+ rec->data = log_rec;
+ rec->ops = &reftable_log_record_vtable;
+}
@@ reftable/record.c (new)
+struct reftable_ref_record *reftable_record_as_ref(struct reftable_record *rec)
+{
+ assert(reftable_record_type(rec) == BLOCK_TYPE_REF);
-+ return (struct reftable_ref_record *)rec->data;
++ return rec->data;
+}
+
+struct reftable_log_record *reftable_record_as_log(struct reftable_record *rec)
+{
+ assert(reftable_record_type(rec) == BLOCK_TYPE_LOG);
-+ return (struct reftable_log_record *)rec->data;
++ return rec->data;
+}
+
+static int hash_equal(uint8_t *a, uint8_t *b, int hash_size)
+{
-+ if (a != NULL && b != NULL)
++ if (a && b)
+ return !memcmp(a, b, hash_size);
+
+ return a == b;
@@ reftable/record.c (new)
+
+int reftable_log_record_compare_key(const void *a, const void *b)
+{
-+ struct reftable_log_record *la = (struct reftable_log_record *)a;
-+ struct reftable_log_record *lb = (struct reftable_log_record *)b;
++ const struct reftable_log_record *la = a;
++ const struct reftable_log_record *lb = b;
+
+ int cmp = strcmp(la->refname, lb->refname);
+ if (cmp)
@@ reftable/record_test.c (new)
+{
+ struct reftable_record copy =
+ reftable_new_record(reftable_record_type(rec));
-+ reftable_record_copy_from(©, rec, SHA1_SIZE);
++ reftable_record_copy_from(©, rec, GIT_SHA1_RAWSZ);
+ /* do it twice to catch memory leaks */
-+ reftable_record_copy_from(©, rec, SHA1_SIZE);
++ reftable_record_copy_from(©, rec, GIT_SHA1_RAWSZ);
+ switch (reftable_record_type(©)) {
+ case BLOCK_TYPE_REF:
+ EXPECT(reftable_ref_record_equal(reftable_record_as_ref(©),
+ reftable_record_as_ref(rec),
-+ SHA1_SIZE));
++ GIT_SHA1_RAWSZ));
+ break;
+ case BLOCK_TYPE_LOG:
+ EXPECT(reftable_log_record_equal(reftable_record_as_log(©),
+ reftable_record_as_log(rec),
-+ SHA1_SIZE));
++ GIT_SHA1_RAWSZ));
+ break;
+ }
+ reftable_record_destroy(©);
@@ reftable/record_test.c (new)
+static void set_hash(uint8_t *h, int j)
+{
+ int i = 0;
-+ for (i = 0; i < hash_size(SHA1_ID); i++) {
++ for (i = 0; i < hash_size(GIT_SHA1_HASH_ID); i++) {
+ h[i] = (j >> i) & 0xff;
+ }
+}
@@ reftable/record_test.c (new)
+ case REFTABLE_REF_DELETION:
+ break;
+ case REFTABLE_REF_VAL1:
-+ in.value.val1 = reftable_malloc(SHA1_SIZE);
++ in.value.val1 = reftable_malloc(GIT_SHA1_RAWSZ);
+ set_hash(in.value.val1, 1);
+ break;
+ case REFTABLE_REF_VAL2:
-+ in.value.val2.value = reftable_malloc(SHA1_SIZE);
++ in.value.val2.value = reftable_malloc(GIT_SHA1_RAWSZ);
+ set_hash(in.value.val2.value, 1);
-+ in.value.val2.target_value = reftable_malloc(SHA1_SIZE);
++ in.value.val2.target_value =
++ reftable_malloc(GIT_SHA1_RAWSZ);
+ set_hash(in.value.val2.target_value, 2);
+ break;
+ case REFTABLE_REF_SYMREF:
@@ reftable/record_test.c (new)
+ EXPECT(reftable_record_val_type(&rec) == i);
+
+ reftable_record_key(&rec, &key);
-+ n = reftable_record_encode(&rec, dest, SHA1_SIZE);
++ n = reftable_record_encode(&rec, dest, GIT_SHA1_RAWSZ);
+ EXPECT(n > 0);
+
+ /* decode into a non-zero reftable_record to test for leaks. */
+
+ reftable_record_from_ref(&rec_out, &out);
-+ m = reftable_record_decode(&rec_out, key, i, dest, SHA1_SIZE);
++ m = reftable_record_decode(&rec_out, key, i, dest,
++ GIT_SHA1_RAWSZ);
+ EXPECT(n == m);
+
-+ EXPECT(reftable_ref_record_equal(&in, &out, SHA1_SIZE));
++ EXPECT(reftable_ref_record_equal(&in, &out, GIT_SHA1_RAWSZ));
+ reftable_record_release(&rec_out);
+
+ strbuf_release(&key);
@@ reftable/record_test.c (new)
+ }
+ };
+
-+ EXPECT(!reftable_log_record_equal(&in[0], &in[1], SHA1_SIZE));
++ EXPECT(!reftable_log_record_equal(&in[0], &in[1], GIT_SHA1_RAWSZ));
+ in[1].update_index = in[0].update_index;
-+ EXPECT(reftable_log_record_equal(&in[0], &in[1], SHA1_SIZE));
++ EXPECT(reftable_log_record_equal(&in[0], &in[1], GIT_SHA1_RAWSZ));
+ reftable_log_record_release(&in[0]);
+ reftable_log_record_release(&in[1]);
+}
@@ reftable/record_test.c (new)
+ .update_index = 42,
+ .value_type = REFTABLE_LOG_UPDATE,
+ .update = {
-+ .old_hash = reftable_malloc(SHA1_SIZE),
-+ .new_hash = reftable_malloc(SHA1_SIZE),
++ .old_hash = reftable_malloc(GIT_SHA1_RAWSZ),
++ .new_hash = reftable_malloc(GIT_SHA1_RAWSZ),
+ .name = xstrdup("han-wen"),
+ .email = xstrdup("[email protected]"),
+ .message = xstrdup("test"),
@@ reftable/record_test.c (new)
+ .refname = xstrdup("old name"),
+ .value_type = REFTABLE_LOG_UPDATE,
+ .update = {
-+ .new_hash = reftable_calloc(SHA1_SIZE),
-+ .old_hash = reftable_calloc(SHA1_SIZE),
++ .new_hash = reftable_calloc(GIT_SHA1_RAWSZ),
++ .old_hash = reftable_calloc(GIT_SHA1_RAWSZ),
+ .name = xstrdup("old name"),
+ .email = xstrdup("old@email"),
+ .message = xstrdup("old message"),
@@ reftable/record_test.c (new)
+
+ reftable_record_key(&rec, &key);
+
-+ n = reftable_record_encode(&rec, dest, SHA1_SIZE);
++ n = reftable_record_encode(&rec, dest, GIT_SHA1_RAWSZ);
+ EXPECT(n >= 0);
+ reftable_record_from_log(&rec_out, &out);
+ valtype = reftable_record_val_type(&rec);
+ m = reftable_record_decode(&rec_out, key, valtype, dest,
-+ SHA1_SIZE);
++ GIT_SHA1_RAWSZ);
+ EXPECT(n == m);
+
-+ EXPECT(reftable_log_record_equal(&in[i], &out, SHA1_SIZE));
++ EXPECT(reftable_log_record_equal(&in[i], &out, GIT_SHA1_RAWSZ));
+ reftable_log_record_release(&in[i]);
+ strbuf_release(&key);
+ reftable_record_release(&rec_out);
@@ reftable/record_test.c (new)
+
+static void test_reftable_obj_record_roundtrip(void)
+{
-+ uint8_t testHash1[SHA1_SIZE] = { 1, 2, 3, 4, 0 };
++ uint8_t testHash1[GIT_SHA1_RAWSZ] = { 1, 2, 3, 4, 0 };
+ uint64_t till9[] = { 1, 2, 3, 4, 500, 600, 700, 800, 9000 };
+ struct reftable_obj_record recs[3] = { {
+ .hash_prefix = testHash1,
@@ reftable/record_test.c (new)
+ reftable_record_from_obj(&rec, &in);
+ test_copy(&rec);
+ reftable_record_key(&rec, &key);
-+ n = reftable_record_encode(&rec, dest, SHA1_SIZE);
++ n = reftable_record_encode(&rec, dest, GIT_SHA1_RAWSZ);
+ EXPECT(n > 0);
+ extra = reftable_record_val_type(&rec);
+ reftable_record_from_obj(&rec_out, &out);
+ m = reftable_record_decode(&rec_out, key, extra, dest,
-+ SHA1_SIZE);
++ GIT_SHA1_RAWSZ);
+ EXPECT(n == m);
+
+ EXPECT(in.hash_prefix_len == out.hash_prefix_len);
@@ reftable/record_test.c (new)
+ test_copy(&rec);
+
+ EXPECT(0 == strbuf_cmp(&key, &in.last_key));
-+ n = reftable_record_encode(&rec, dest, SHA1_SIZE);
++ n = reftable_record_encode(&rec, dest, GIT_SHA1_RAWSZ);
+ EXPECT(n > 0);
+
+ extra = reftable_record_val_type(&rec);
+ reftable_record_from_index(&out_rec, &out);
-+ m = reftable_record_decode(&out_rec, key, extra, dest, SHA1_SIZE);
++ m = reftable_record_decode(&out_rec, key, extra, dest, GIT_SHA1_RAWSZ);
+ EXPECT(m == n);
+
+ EXPECT(in.offset == out.offset);
-: ------------ > 11: 0c2604d7e7d4 Provide zlib's uncompress2 from compat/zlib-compat.c
7: 9297b9c363f6 ! 12: 425cd0185aab reftable: reading/writing blocks
@@ Commit message This commit provides the logic to read and write these blocks.
- Includes a code snippet copied from zlib
-
Signed-off-by: Han-Wen Nienhuys <[email protected]>
## Makefile ##
@@ Makefile: xdiff-objs: $(XDIFF_OBJS)
REFTABLE_OBJS += reftable/blocksource.o
REFTABLE_OBJS += reftable/publicbasics.o
REFTABLE_OBJS += reftable/record.o
-+REFTABLE_OBJS += reftable/zlib-compat.o
+REFTABLE_TEST_OBJS += reftable/block_test.o
REFTABLE_TEST_OBJS += reftable/record_test.o
REFTABLE_TEST_OBJS += reftable/test_framework.o
REFTABLE_TEST_OBJS += reftable/basics_test.o
- ## reftable/.gitattributes (new) ##
-@@
-+/zlib-compat.c whitespace=-indent-with-non-tab,-trailing-space
-
## reftable/block.c (new) ##
@@
+/*
@@ reftable/block.c (new)
+#include "record.h"
+#include "reftable-error.h"
+#include "system.h"
-+#include "zlib.h"
++#include <zlib.h>
++
++#ifdef NO_UNCOMPRESS2
++/* This is uncompress2, which is only available in zlib as of 2017.
++ */
++int uncompress2(Bytef *dest, uLongf *destLen, const Bytef *source,
++ uLong *sourceLen);
++#endif
+
+int header_size(int version)
+{
@@ reftable/block.c (new)
+ memcpy(uncompressed, block->data, block_header_skip);
+
+ /* Uncompress */
-+ if (Z_OK != uncompress_return_consumed(
-+ uncompressed + block_header_skip, &dst_len,
-+ block->data + block_header_skip,
-+ &src_len)) {
++ if (Z_OK !=
++ uncompress2(uncompressed + block_header_skip, &dst_len,
++ block->data + block_header_skip, &src_len)) {
+ reftable_free(uncompressed);
+ return REFTABLE_ZLIB_ERROR;
+ }
@@ reftable/block.c (new)
+
+static int restart_key_less(size_t idx, void *args)
+{
-+ struct restart_find_args *a = (struct restart_find_args *)args;
++ struct restart_find_args *a = args;
+ uint32_t off = block_reader_restart_offset(a->r, idx);
+ struct string_view in = {
+ .buf = a->r->block.data + off,
@@ reftable/block.c (new)
+void reftable_block_done(struct reftable_block *blockp)
+{
+ struct reftable_block_source source = blockp->source;
-+ if (blockp != NULL && source.ops != NULL)
++ if (blockp && source.ops)
+ source.ops->return_block(source.arg, blockp);
+ blockp->data = NULL;
+ blockp->len = 0;
@@ reftable/block_test.c (new)
+ block.len = block_size;
+ block.source = malloc_block_source();
+ block_writer_init(&bw, BLOCK_TYPE_REF, block.data, block_size,
-+ header_off, hash_size(SHA1_ID));
++ header_off, hash_size(GIT_SHA1_HASH_ID));
+ reftable_record_from_ref(&rec, &ref);
+
+ for (i = 0; i < N; i++) {
+ char name[100];
-+ uint8_t hash[SHA1_SIZE];
++ uint8_t hash[GIT_SHA1_RAWSZ];
+ snprintf(name, sizeof(name), "branch%02d", i);
+ memset(hash, i, sizeof(hash));
+
@@ reftable/block_test.c (new)
+
+ block_writer_release(&bw);
+
-+ block_reader_init(&br, &block, header_off, block_size, SHA1_SIZE);
++ block_reader_init(&br, &block, header_off, block_size, GIT_SHA1_RAWSZ);
+
+ block_reader_start(&br, &it);
+
@@ reftable/block_test.c (new)
+{
+ RUN_TEST(test_block_read_write);
+ return 0;
-+}
-
- ## reftable/zlib-compat.c (new) ##
-@@
-+/* taken from zlib's uncompr.c
-+
-+ commit cacf7f1d4e3d44d871b605da3b647f07d718623f
-+ Author: Mark Adler <[email protected]>
-+ Date: Sun Jan 15 09:18:46 2017 -0800
-+
-+ zlib 1.2.11
-+
-+*/
-+
-+/*
-+ * Copyright (C) 1995-2003, 2010, 2014, 2016 Jean-loup Gailly, Mark Adler
-+ * For conditions of distribution and use, see copyright notice in zlib.h
-+ */
-+
-+#include "system.h"
-+
-+/* clang-format off */
-+
-+/* ===========================================================================
-+ Decompresses the source buffer into the destination buffer. *sourceLen is
-+ the byte length of the source buffer. Upon entry, *destLen is the total size
-+ of the destination buffer, which must be large enough to hold the entire
-+ uncompressed data. (The size of the uncompressed data must have been saved
-+ previously by the compressor and transmitted to the decompressor by some
-+ mechanism outside the scope of this compression library.) Upon exit,
-+ *destLen is the size of the decompressed data and *sourceLen is the number
-+ of source bytes consumed. Upon return, source + *sourceLen points to the
-+ first unused input byte.
-+
-+ uncompress returns Z_OK if success, Z_MEM_ERROR if there was not enough
-+ memory, Z_BUF_ERROR if there was not enough room in the output buffer, or
-+ Z_DATA_ERROR if the input data was corrupted, including if the input data is
-+ an incomplete zlib stream.
-+*/
-+int ZEXPORT uncompress_return_consumed (
-+ Bytef *dest,
-+ uLongf *destLen,
-+ const Bytef *source,
-+ uLong *sourceLen) {
-+ z_stream stream;
-+ int err;
-+ const uInt max = (uInt)-1;
-+ uLong len, left;
-+ Byte buf[1]; /* for detection of incomplete stream when *destLen == 0 */
-+
-+ len = *sourceLen;
-+ if (*destLen) {
-+ left = *destLen;
-+ *destLen = 0;
-+ }
-+ else {
-+ left = 1;
-+ dest = buf;
-+ }
-+
-+ stream.next_in = (z_const Bytef *)source;
-+ stream.avail_in = 0;
-+ stream.zalloc = (alloc_func)0;
-+ stream.zfree = (free_func)0;
-+ stream.opaque = (voidpf)0;
-+
-+ err = inflateInit(&stream);
-+ if (err != Z_OK) return err;
-+
-+ stream.next_out = dest;
-+ stream.avail_out = 0;
-+
-+ do {
-+ if (stream.avail_out == 0) {
-+ stream.avail_out = left > (uLong)max ? max : (uInt)left;
-+ left -= stream.avail_out;
-+ }
-+ if (stream.avail_in == 0) {
-+ stream.avail_in = len > (uLong)max ? max : (uInt)len;
-+ len -= stream.avail_in;
-+ }
-+ err = inflate(&stream, Z_NO_FLUSH);
-+ } while (err == Z_OK);
-+
-+ *sourceLen -= len + stream.avail_in;
-+ if (dest != buf)
-+ *destLen = stream.total_out;
-+ else if (stream.total_out && err == Z_BUF_ERROR)
-+ left = 1;
-+
-+ inflateEnd(&stream);
-+ return err == Z_STREAM_END ? Z_OK :
-+ err == Z_NEED_DICT ? Z_DATA_ERROR :
-+ err == Z_BUF_ERROR && left + stream.avail_out ? Z_DATA_ERROR :
-+ err;
+}
## t/helper/test-reftable.c ##
8: ceddefadd48c ! 13: c03957db45c0 reftable: a generic binary tree implementation
@@ Makefile: REFTABLE_OBJS += reftable/block.o
REFTABLE_OBJS += reftable/publicbasics.o
REFTABLE_OBJS += reftable/record.o
+REFTABLE_OBJS += reftable/tree.o
- REFTABLE_OBJS += reftable/zlib-compat.o
+REFTABLE_TEST_OBJS += reftable/basics_test.o
REFTABLE_TEST_OBJS += reftable/block_test.o
@@ reftable/tree.c (new)
+void infix_walk(struct tree_node *t, void (*action)(void *arg, void *key),
+ void *arg)
+{
-+ if (t->left != NULL) {
++ if (t->left) {
+ infix_walk(t->left, action, arg);
+ }
+ action(arg, t->key);
-+ if (t->right != NULL) {
++ if (t->right) {
+ infix_walk(t->right, action, arg);
+ }
+}
@@ reftable/tree.c (new)
+ if (t == NULL) {
+ return;
+ }
-+ if (t->left != NULL) {
++ if (t->left) {
+ tree_free(t->left);
+ }
-+ if (t->right != NULL) {
++ if (t->right) {
+ tree_free(t->right);
+ }
+ reftable_free(t);
@@ reftable/tree_test.c (new)
+
+static void check_increasing(void *arg, void *key)
+{
-+ struct curry *c = (struct curry *)arg;
-+ if (c->last != NULL) {
++ struct curry *c = arg;
++ if (c->last) {
+ assert(test_compare(c->last, key) < 0);
+ }
+ c->last = key;
9: 1f635e6c86c5 ! 14: 922deb272fff reftable: write reftable files
@@ Makefile: REFTABLE_OBJS += reftable/blocksource.o
REFTABLE_OBJS += reftable/record.o
REFTABLE_OBJS += reftable/tree.o
+REFTABLE_OBJS += reftable/writer.o
- REFTABLE_OBJS += reftable/zlib-compat.o
REFTABLE_TEST_OBJS += reftable/basics_test.o
+ REFTABLE_TEST_OBJS += reftable/block_test.o
## reftable/reftable-writer.h (new) ##
@@
@@ reftable/reftable-writer.h (new)
+
+/* reftable_new_writer creates a new writer */
+struct reftable_writer *
-+reftable_new_writer(int (*writer_func)(void *, const void *, size_t),
++reftable_new_writer(ssize_t (*writer_func)(void *, const void *, size_t),
+ void *writer_arg, struct reftable_write_options *opts);
+
+/* Set the range of update indices for the records we will add. When writing a
@@ reftable/writer.c (new)
+ }
+
+ if (opts->hash_id == 0) {
-+ opts->hash_id = SHA1_ID;
++ opts->hash_id = GIT_SHA1_HASH_ID;
+ }
+ if (opts->block_size == 0) {
+ opts->block_size = DEFAULT_BLOCK_SIZE;
@@ reftable/writer.c (new)
+
+static int writer_version(struct reftable_writer *w)
+{
-+ return (w->opts.hash_id == 0 || w->opts.hash_id == SHA1_ID) ? 1 : 2;
++ return (w->opts.hash_id == 0 || w->opts.hash_id == GIT_SHA1_HASH_ID) ?
++ 1 :
++ 2;
+}
+
+static int writer_write_header(struct reftable_writer *w, uint8_t *dest)
@@ reftable/writer.c (new)
+static struct strbuf reftable_empty_strbuf = STRBUF_INIT;
+
+struct reftable_writer *
-+reftable_new_writer(int (*writer_func)(void *, const void *, size_t),
++reftable_new_writer(ssize_t (*writer_func)(void *, const void *, size_t),
+ void *writer_arg, struct reftable_write_options *opts)
+{
+ struct reftable_writer *wp =
@@ reftable/writer.c (new)
+static int writer_add_record(struct reftable_writer *w,
+ struct reftable_record *rec)
+{
-+ int result = -1;
+ struct strbuf key = STRBUF_INIT;
-+ int err = 0;
++ int err = -1;
+ reftable_record_key(rec, &key);
-+ if (strbuf_cmp(&w->last_key, &key) >= 0)
++ if (strbuf_cmp(&w->last_key, &key) >= 0) {
++ err = REFTABLE_API_ERROR;
+ goto done;
++ }
+
+ strbuf_reset(&w->last_key);
+ strbuf_addbuf(&w->last_key, &key);
@@ reftable/writer.c (new)
+ assert(block_writer_type(w->block_writer) == reftable_record_type(rec));
+
+ if (block_writer_add(w->block_writer, rec) == 0) {
-+ result = 0;
++ err = 0;
+ goto done;
+ }
+
+ err = writer_flush_block(w);
+ if (err < 0) {
-+ result = err;
+ goto done;
+ }
+
+ writer_reinit_block_writer(w, reftable_record_type(rec));
+ err = block_writer_add(w->block_writer, rec);
+ if (err < 0) {
-+ result = err;
+ goto done;
+ }
+
-+ result = 0;
++ err = 0;
+done:
+ strbuf_release(&key);
-+ return result;
++ return err;
+}
+
+int reftable_writer_add_ref(struct reftable_writer *w,
@@ reftable/writer.c (new)
+ if (err < 0)
+ return err;
+
-+ if (!w->opts.skip_index_objects &&
-+ reftable_ref_record_val1(ref) != NULL) {
++ if (!w->opts.skip_index_objects && reftable_ref_record_val1(ref)) {
+ struct strbuf h = STRBUF_INIT;
+ strbuf_add(&h, (char *)reftable_ref_record_val1(ref),
+ hash_size(w->opts.hash_id));
@@ reftable/writer.c (new)
+ strbuf_release(&h);
+ }
+
-+ if (!w->opts.skip_index_objects &&
-+ reftable_ref_record_val2(ref) != NULL) {
++ if (!w->opts.skip_index_objects && reftable_ref_record_val2(ref)) {
+ struct strbuf h = STRBUF_INIT;
+ strbuf_add(&h, reftable_ref_record_val2(ref),
+ hash_size(w->opts.hash_id));
@@ reftable/writer.c (new)
+ struct reftable_log_record *log)
+{
+ struct reftable_record rec = { NULL };
-+ if (w->block_writer != NULL &&
++ if (w->block_writer &&
+ block_writer_type(w->block_writer) == BLOCK_TYPE_REF) {
+ int err = writer_finish_public_section(w);
+ if (err < 0)
@@ reftable/writer.c (new)
+ return REFTABLE_API_ERROR;
+
+ input_log_message = log->update.message;
-+ if (!w->opts.exact_log_message && log->update.message != NULL) {
++ if (!w->opts.exact_log_message && log->update.message) {
+ strbuf_addstr(&cleaned_message, log->update.message);
+ while (cleaned_message.len &&
+ cleaned_message.buf[cleaned_message.len - 1] == '\n')
@@ reftable/writer.c (new)
+
+static void update_common(void *void_arg, void *key)
+{
-+ struct common_prefix_arg *arg = (struct common_prefix_arg *)void_arg;
-+ struct obj_index_tree_node *entry = (struct obj_index_tree_node *)key;
-+ if (arg->last != NULL) {
++ struct common_prefix_arg *arg = void_arg;
++ struct obj_index_tree_node *entry = key;
++ if (arg->last) {
+ int n = common_prefix_size(&entry->hash, arg->last);
+ if (n > arg->max) {
+ arg->max = n;
@@ reftable/writer.c (new)
+
+static void write_object_record(void *void_arg, void *key)
+{
-+ struct write_record_arg *arg = (struct write_record_arg *)void_arg;
-+ struct obj_index_tree_node *entry = (struct obj_index_tree_node *)key;
++ struct write_record_arg *arg = void_arg;
++ struct obj_index_tree_node *entry = key;
+ struct reftable_obj_record obj_rec = {
+ .hash_prefix = (uint8_t *)entry->hash.buf,
+ .hash_prefix_len = arg->w->stats.object_id_len,
@@ reftable/writer.c (new)
+
+static void object_record_free(void *void_arg, void *key)
+{
-+ struct obj_index_tree_node *entry = (struct obj_index_tree_node *)key;
++ struct obj_index_tree_node *entry = key;
+
+ FREE_AND_NULL(entry->offsets);
+ strbuf_release(&entry->hash);
@@ reftable/writer.c (new)
+{
+ struct write_record_arg closure = { .w = w };
+ struct common_prefix_arg common = { NULL };
-+ if (w->obj_index_tree != NULL) {
++ if (w->obj_index_tree) {
+ infix_walk(w->obj_index_tree, &update_common, &common);
+ }
+ w->stats.object_id_len = common.max + 1;
+
+ writer_reinit_block_writer(w, BLOCK_TYPE_OBJ);
+
-+ if (w->obj_index_tree != NULL) {
++ if (w->obj_index_tree) {
+ infix_walk(w->obj_index_tree, &write_object_record, &closure);
+ }
+
@@ reftable/writer.c (new)
+ return err;
+ }
+
-+ if (w->obj_index_tree != NULL) {
++ if (w->obj_index_tree) {
+ infix_walk(w->obj_index_tree, &object_record_free, NULL);
+ tree_free(w->obj_index_tree);
+ w->obj_index_tree = NULL;
@@ reftable/writer.h (new)
+#include "reftable-writer.h"
+
+struct reftable_writer {
-+ int (*write)(void *, const void *, size_t);
++ ssize_t (*write)(void *, const void *, size_t);
+ void *write_arg;
+ int pending_padding;
+ struct strbuf last_key;
10: 9c3fd77fbbac ! 15: afca8f8f6a29 reftable: generic interface to tables
@@ Makefile: REFTABLE_OBJS += reftable/block.o
REFTABLE_OBJS += reftable/blocksource.o
REFTABLE_OBJS += reftable/publicbasics.o
REFTABLE_OBJS += reftable/record.o
-+REFTABLE_OBJS += reftable/reftable.o
++REFTABLE_OBJS += reftable/refname.o
++REFTABLE_OBJS += reftable/generic.o
++REFTABLE_OBJS += reftable/stack.o
REFTABLE_OBJS += reftable/tree.o
REFTABLE_OBJS += reftable/writer.o
- REFTABLE_OBJS += reftable/zlib-compat.o
+
+
+ ## reftable/generic.c (new) ##
+@@
++/*
++Copyright 2020 Google LLC
++
++Use of this source code is governed by a BSD-style
++license that can be found in the LICENSE file or at
++https://developers.google.com/open-source/licenses/bsd
++*/
++
++#include "basics.h"
++#include "record.h"
++#include "generic.h"
++#include "reftable-iterator.h"
++#include "reftable-generic.h"
++
++int reftable_table_seek_ref(struct reftable_table *tab,
++ struct reftable_iterator *it, const char *name)
++{
++ struct reftable_ref_record ref = {
++ .refname = (char *)name,
++ };
++ struct reftable_record rec = { NULL };
++ reftable_record_from_ref(&rec, &ref);
++ return tab->ops->seek_record(tab->table_arg, it, &rec);
++}
++
++int reftable_table_seek_log(struct reftable_table *tab,
++ struct reftable_iterator *it, const char *name)
++{
++ struct reftable_log_record log = {
++ .refname = (char *)name,
++ .update_index = ~((uint64_t)0),
++ };
++ struct reftable_record rec = { NULL };
++ reftable_record_from_log(&rec, &log);
++ return tab->ops->seek_record(tab->table_arg, it, &rec);
++}
++
++int reftable_table_read_ref(struct reftable_table *tab, const char *name,
++ struct reftable_ref_record *ref)
++{
++ struct reftable_iterator it = { NULL };
++ int err = reftable_table_seek_ref(tab, &it, name);
++ if (err)
++ goto done;
++
++ err = reftable_iterator_next_ref(&it, ref);
++ if (err)
++ goto done;
++
++ if (strcmp(ref->refname, name) ||
++ reftable_ref_record_is_deletion(ref)) {
++ reftable_ref_record_release(ref);
++ err = 1;
++ goto done;
++ }
++
++done:
++ reftable_iterator_destroy(&it);
++ return err;
++}
++
++int reftable_table_print(struct reftable_table *tab) {
++ struct reftable_iterator it = { NULL };
++ struct reftable_ref_record ref = { NULL };
++ struct reftable_log_record log = { NULL };
++ uint32_t hash_id = reftable_table_hash_id(tab);
++ int err = reftable_table_seek_ref(tab, &it, "");
++ if (err < 0) {
++ return err;
++ }
++
++ while (1) {
++ err = reftable_iterator_next_ref(&it, &ref);
++ if (err > 0) {
++ break;
++ }
++ if (err < 0) {
++ return err;
++ }
++ reftable_ref_record_print(&ref, hash_id);
++ }
++ reftable_iterator_destroy(&it);
++ reftable_ref_record_release(&ref);
++
++ err = reftable_table_seek_log(tab, &it, "");
++ if (err < 0) {
++ return err;
++ }
++ while (1) {
++ err = reftable_iterator_next_log(&it, &log);
++ if (err > 0) {
++ break;
++ }
++ if (err < 0) {
++ return err;
++ }
++ reftable_log_record_print(&log, hash_id);
++ }
++ reftable_iterator_destroy(&it);
++ reftable_log_record_release(&log);
++ return 0;
++}
++
++uint64_t reftable_table_max_update_index(struct reftable_table *tab)
++{
++ return tab->ops->max_update_index(tab->table_arg);
++}
++
++uint64_t reftable_table_min_update_index(struct reftable_table *tab)
++{
++ return tab->ops->min_update_index(tab->table_arg);
++}
++
++uint32_t reftable_table_hash_id(struct reftable_table *tab)
++{
++ return tab->ops->hash_id(tab->table_arg);
++}
++
++void reftable_iterator_destroy(struct reftable_iterator *it)
++{
++ if (!it->ops) {
++ return;
++ }
++ it->ops->close(it->iter_arg);
++ it->ops = NULL;
++ FREE_AND_NULL(it->iter_arg);
++}
++
++int reftable_iterator_next_ref(struct reftable_iterator *it,
++ struct reftable_ref_record *ref)
++{
++ struct reftable_record rec = { NULL };
++ reftable_record_from_ref(&rec, ref);
++ return iterator_next(it, &rec);
++}
++
++int reftable_iterator_next_log(struct reftable_iterator *it,
++ struct reftable_log_record *log)
++{
++ struct reftable_record rec = { NULL };
++ reftable_record_from_log(&rec, log);
++ return iterator_next(it, &rec);
++}
++
++int iterator_next(struct reftable_iterator *it, struct reftable_record *rec)
++{
++ return it->ops->next(it->iter_arg, rec);
++}
++
++static int empty_iterator_next(void *arg, struct reftable_record *rec)
++{
++ return 1;
++}
++
++static void empty_iterator_close(void *arg)
++{
++}
++
++static struct reftable_iterator_vtable empty_vtable = {
++ .next = &empty_iterator_next,
++ .close = &empty_iterator_close,
++};
++
++void iterator_set_empty(struct reftable_iterator *it)
++{
++ assert(!it->ops);
++ it->iter_arg = NULL;
++ it->ops = &empty_vtable;
++}
## reftable/generic.h (new) ##
@@
@@ reftable/reftable-generic.h (new)
+ void *table_arg;
+};
+
++int reftable_table_seek_log(struct reftable_table *tab,
++ struct reftable_iterator *it, const char *name);
++
+int reftable_table_seek_ref(struct reftable_table *tab,
+ struct reftable_iterator *it, const char *name);
+
@@ reftable/reftable-generic.h (new)
+int reftable_table_read_ref(struct reftable_table *tab, const char *name,
+ struct reftable_ref_record *ref);
+
++/* dump table contents onto stdout for debugging */
++int reftable_table_print(struct reftable_table *tab);
++
+#endif
## reftable/reftable-iterator.h (new) ##
@@ reftable/reftable.c (new)
+
+void reftable_iterator_destroy(struct reftable_iterator *it)
+{
-+ if (it->ops == NULL) {
++ if (!it->ops) {
+ return;
+ }
+ it->ops->close(it->iter_arg);
@@ reftable/reftable.c (new)
+
+void iterator_set_empty(struct reftable_iterator *it)
+{
-+ assert(it->ops == NULL);
++ assert(!it->ops);
+ it->iter_arg = NULL;
+ it->ops = &empty_vtable;
+}
11: 8ba486f44a72 ! 16: 501cf0677ce4 reftable: read reftable files
@@ Makefile: REFTABLE_OBJS += reftable/basics.o
REFTABLE_OBJS += reftable/publicbasics.o
+REFTABLE_OBJS += reftable/reader.o
REFTABLE_OBJS += reftable/record.o
- REFTABLE_OBJS += reftable/reftable.o
- REFTABLE_OBJS += reftable/tree.o
+ REFTABLE_OBJS += reftable/refname.o
+ REFTABLE_OBJS += reftable/generic.o
## reftable/iter.c (new) ##
@@
@@ reftable/iter.c (new)
+
+int iterator_is_null(struct reftable_iterator *it)
+{
-+ return it->ops == NULL;
++ return !it->ops;
+}
+
+static void filtering_ref_iterator_close(void *iter_arg)
+{
-+ struct filtering_ref_iterator *fri =
-+ (struct filtering_ref_iterator *)iter_arg;
++ struct filtering_ref_iterator *fri = iter_arg;
+ strbuf_release(&fri->oid);
+ reftable_iterator_destroy(&fri->it);
+}
@@ reftable/iter.c (new)
+static int filtering_ref_iterator_next(void *iter_arg,
+ struct reftable_record *rec)
+{
-+ struct filtering_ref_iterator *fri =
-+ (struct filtering_ref_iterator *)iter_arg;
-+ struct reftable_ref_record *ref =
-+ (struct reftable_ref_record *)rec->data;
++ struct filtering_ref_iterator *fri = iter_arg;
++ struct reftable_ref_record *ref = rec->data;
+ int err = 0;
+ while (1) {
+ err = reftable_iterator_next_ref(&fri->it, ref);
@@ reftable/iter.c (new)
+void iterator_from_filtering_ref_iterator(struct reftable_iterator *it,
+ struct filtering_ref_iterator *fri)
+{
-+ assert(it->ops == NULL);
++ assert(!it->ops);
+ it->iter_arg = fri;
+ it->ops = &filtering_ref_iterator_vtable;
+}
+
+static void indexed_table_ref_iter_close(void *p)
+{
-+ struct indexed_table_ref_iter *it = (struct indexed_table_ref_iter *)p;
++ struct indexed_table_ref_iter *it = p;
+ block_iter_close(&it->cur);
+ reftable_block_done(&it->block_reader.block);
++ reftable_free(it->offsets);
+ strbuf_release(&it->oid);
+}
+
@@ reftable/iter.c (new)
+
+static int indexed_table_ref_iter_next(void *p, struct reftable_record *rec)
+{
-+ struct indexed_table_ref_iter *it = (struct indexed_table_ref_iter *)p;
-+ struct reftable_ref_record *ref =
-+ (struct reftable_ref_record *)rec->data;
++ struct indexed_table_ref_iter *it = p;
++ struct reftable_ref_record *ref = rec->data;
+
+ while (1) {
+ int err = block_iter_next(&it->cur, rec);
@@ reftable/iter.c (new)
+void iterator_from_indexed_table_ref_iter(struct reftable_iterator *it,
+ struct indexed_table_ref_iter *itr)
+{
-+ assert(it->ops == NULL);
++ assert(!it->ops);
+ it->iter_arg = itr;
+ it->ops = &indexed_table_ref_iter_vtable;
+}
@@ reftable/iter.h (new)
+
+void iterator_from_indexed_table_ref_iter(struct reftable_iterator *it,
+ struct indexed_table_ref_iter *itr);
++
++/* Takes ownership of `offsets` */
+int new_indexed_table_ref_iter(struct indexed_table_ref_iter **dest,
+ struct reftable_reader *r, uint8_t *oid,
+ int oid_len, uint64_t *offsets, int offset_len);
@@ reftable/reader.c (new)
+
+void block_source_close(struct reftable_block_source *source)
+{
-+ if (source->ops == NULL) {
++ if (!source->ops) {
+ return;
+ }
+
@@ reftable/reader.c (new)
+ f += 8;
+
+ if (r->version == 1) {
-+ r->hash_id = SHA1_ID;
++ r->hash_id = GIT_SHA1_HASH_ID;
+ } else {
+ r->hash_id = get_be32(f);
+ switch (r->hash_id) {
-+ case SHA1_ID:
++ case GIT_SHA1_HASH_ID:
+ break;
-+ case SHA256_ID:
++ case GIT_SHA256_HASH_ID:
+ break;
+ default:
+ err = REFTABLE_FORMAT_ERROR;
@@ reftable/reader.c (new)
+
+static void table_iter_block_done(struct table_iter *ti)
+{
-+ if (ti->bi.br == NULL) {
++ if (!ti->bi.br) {
+ return;
+ }
+ reftable_block_done(&ti->bi.br->block);
@@ reftable/reader.c (new)
+
+static int table_iter_next_void(void *ti, struct reftable_record *rec)
+{
-+ return table_iter_next((struct table_iter *)ti, rec);
++ return table_iter_next(ti, rec);
+}
+
+static void table_iter_close(void *p)
+{
-+ struct table_iter *ti = (struct table_iter *)p;
++ struct table_iter *ti = p;
+ table_iter_block_done(ti);
+ block_iter_close(&ti->bi);
+}
@@ reftable/reader.c (new)
+static void iterator_from_table_iter(struct reftable_iterator *it,
+ struct table_iter *ti)
+{
-+ assert(it->ops == NULL);
++ assert(!it->ops);
+ it->iter_arg = ti;
+ it->ops = &table_iter_vtable;
+}
@@ reftable/reader.c (new)
+static int reftable_reader_seek_void(void *tab, struct reftable_iterator *it,
+ struct reftable_record *rec)
+{
-+ return reader_seek((struct reftable_reader *)tab, it, rec);
++ return reader_seek(tab, it, rec);
+}
+
+static uint32_t reftable_reader_hash_id_void(void *tab)
+{
-+ return reftable_reader_hash_id((struct reftable_reader *)tab);
++ return reftable_reader_hash_id(tab);
+}
+
+static uint64_t reftable_reader_min_update_index_void(void *tab)
+{
-+ return reftable_reader_min_update_index((struct reftable_reader *)tab);
++ return reftable_reader_min_update_index(tab);
+}
+
+static uint64_t reftable_reader_max_update_index_void(void *tab)
+{
-+ return reftable_reader_max_update_index((struct reftable_reader *)tab);
++ return reftable_reader_max_update_index(tab);
+}
+
+static struct reftable_table_vtable reader_vtable = {
@@ reftable/reader.c (new)
+void reftable_table_from_reader(struct reftable_table *tab,
+ struct reftable_reader *reader)
+{
-+ assert(tab->ops == NULL);
++ assert(!tab->ops);
+ tab->ops = &reader_vtable;
+ tab->table_arg = reader;
++}
++
++
++int reftable_reader_print_file(const char *tablename)
++{
++ struct reftable_block_source src = { NULL };
++ int err = reftable_block_source_from_file(&src, tablename);
++ struct reftable_reader *r = NULL;
++ struct reftable_table tab = { NULL };
++ if (err < 0)
++ goto done;
++
++ err = reftable_new_reader(&r, &src, tablename);
++ if (err < 0)
++ goto done;
++
++ reftable_table_from_reader(&tab, r);
++ err = reftable_table_print(&tab);
++done:
++ reftable_reader_free(r);
++ return err;
+}
## reftable/reader.h (new) ##
@@ reftable/reftable-reader.h (new)
+void reftable_table_from_reader(struct reftable_table *tab,
+ struct reftable_reader *reader);
+
++/* print table onto stdout for debugging. */
++int reftable_reader_print_file(const char *tablename);
++
+#endif
12: 9ade9303f08f ! 17: dc04ec88cf53 reftable: reftable file level tests
@@ Commit message
Signed-off-by: Han-Wen Nienhuys <[email protected]>
## Makefile ##
-@@ Makefile: REFTABLE_OBJS += reftable/zlib-compat.o
+@@ Makefile: REFTABLE_OBJS += reftable/writer.o
REFTABLE_TEST_OBJS += reftable/basics_test.o
REFTABLE_TEST_OBJS += reftable/block_test.o
REFTABLE_TEST_OBJS += reftable/record_test.o
-+REFTABLE_TEST_OBJS += reftable/reftable_test.o
++REFTABLE_TEST_OBJS += reftable/readwrite_test.o
REFTABLE_TEST_OBJS += reftable/test_framework.o
REFTABLE_TEST_OBJS += reftable/tree_test.o
- ## reftable/reftable_test.c (new) ##
+ ## reftable/readwrite_test.c (new) ##
@@
+/*
+Copyright 2020 Google LLC
@@ reftable/reftable_test.c (new)
+ *names = reftable_calloc(sizeof(char *) * (N + 1));
+ reftable_writer_set_limits(w, update_index, update_index);
+ for (i = 0; i < N; i++) {
-+ uint8_t hash[SHA256_SIZE] = { 0 };
++ uint8_t hash[GIT_SHA256_RAWSZ] = { 0 };
+ char name[100];
+ int n;
+
@@ reftable/reftable_test.c (new)
+ }
+
+ for (i = 0; i < N; i++) {
-+ uint8_t hash[SHA256_SIZE] = { 0 };
++ uint8_t hash[GIT_SHA256_RAWSZ] = { 0 };
+ char name[100];
+ int n;
+
@@ reftable/reftable_test.c (new)
+ for (i = 0; i < stats->ref_stats.blocks; i++) {
+ int off = i * opts.block_size;
+ if (off == 0) {
-+ off = header_size((hash_id == SHA256_ID) ? 2 : 1);
++ off = header_size((hash_id == GIT_SHA256_HASH_ID) ? 2 :
++ 1);
+ }
+ EXPECT(buf->buf[off] == 'r');
+ }
@@ reftable/reftable_test.c (new)
+ /* This tests buffer extension for log compression. Must use a random
+ hash, to ensure that the compressed part is larger than the original.
+ */
-+ uint8_t hash1[SHA1_SIZE], hash2[SHA1_SIZE];
-+ for (int i = 0; i < SHA1_SIZE; i++) {
++ uint8_t hash1[GIT_SHA1_RAWSZ], hash2[GIT_SHA1_RAWSZ];
++ for (int i = 0; i < GIT_SHA1_RAWSZ; i++) {
+ hash1[i] = (uint8_t)(rand() % 256);
+ hash2[i] = (uint8_t)(rand() % 256);
+ }
@@ reftable/reftable_test.c (new)
+ EXPECT_ERR(err);
+ }
+ for (i = 0; i < N; i++) {
-+ uint8_t hash1[SHA1_SIZE], hash2[SHA1_SIZE];
++ uint8_t hash1[GIT_SHA1_RAWSZ], hash2[GIT_SHA1_RAWSZ];
+ struct reftable_log_record log = { NULL };
+ set_test_hash(hash1, i);
+ set_test_hash(hash2, i + 1);
@@ reftable/reftable_test.c (new)
+ int err = 0;
+ int j = 0;
+
-+ write_table(&names, &buf, N, 256, SHA1_ID);
++ write_table(&names, &buf, N, 256, GIT_SHA1_HASH_ID);
+
+ block_source_from_strbuf(&source, &buf);
+
@@ reftable/reftable_test.c (new)
+ char **names;
+ struct strbuf buf = STRBUF_INIT;
+ int N = 1;
-+ write_table(&names, &buf, N, 4096, SHA1_ID);
++ write_table(&names, &buf, N, 4096, GIT_SHA1_HASH_ID);
+ EXPECT(buf.len < 200);
+ strbuf_release(&buf);
+ free_names(names);
@@ reftable/reftable_test.c (new)
+ struct reftable_log_record log = { NULL };
+ struct reftable_iterator it = { NULL };
+
-+ write_table(&names, &buf, N, 256, SHA1_ID);
++ write_table(&names, &buf, N, 256, GIT_SHA1_HASH_ID);
+
+ block_source_from_strbuf(&source, &buf);
+
@@ reftable/reftable_test.c (new)
+
+static void test_table_read_write_seek_linear(void)
+{
-+ test_table_read_write_seek(0, SHA1_ID);
++ test_table_read_write_seek(0, GIT_SHA1_HASH_ID);
+}
+
+static void test_table_read_write_seek_linear_sha256(void)
+{
-+ test_table_read_write_seek(0, SHA256_ID);
++ test_table_read_write_seek(0, GIT_SHA256_HASH_ID);
+}
+
+static void test_table_read_write_seek_index(void)
+{
-+ test_table_read_write_seek(1, SHA1_ID);
++ test_table_read_write_seek(1, GIT_SHA1_HASH_ID);
+}
+
+static void test_table_refs_for(int indexed)
@@ reftable/reftable_test.c (new)
+ int N = 50;
+ char **want_names = reftable_calloc(sizeof(char *) * (N + 1));
+ int want_names_len = 0;
-+ uint8_t want_hash[SHA1_SIZE];
++ uint8_t want_hash[GIT_SHA1_RAWSZ];
+
+ struct reftable_write_options opts = {
+ .block_size = 256,
@@ reftable/reftable_test.c (new)
+ set_test_hash(want_hash, 4);
+
+ for (i = 0; i < N; i++) {
-+ uint8_t hash[SHA1_SIZE];
++ uint8_t hash[GIT_SHA1_RAWSZ];
+ char fill[51] = { 0 };
+ char name[100];
-+ uint8_t hash1[SHA1_SIZE];
-+ uint8_t hash2[SHA1_SIZE];
++ uint8_t hash1[GIT_SHA1_RAWSZ];
++ uint8_t hash2[GIT_SHA1_RAWSZ];
+ struct reftable_ref_record ref = { NULL };
+
+ memset(hash, i, sizeof(hash));
@@ reftable/reftable_test.c (new)
+ n = reftable_writer_add_ref(w, &ref);
+ EXPECT(n == 0);
+
-+ if (!memcmp(hash1, want_hash, SHA1_SIZE) ||
-+ !memcmp(hash2, want_hash, SHA1_SIZE)) {
++ if (!memcmp(hash1, want_hash, GIT_SHA1_RAWSZ) ||
++ !memcmp(hash2, want_hash, GIT_SHA1_RAWSZ)) {
+ want_names[want_names_len++] = xstrdup(name);
+ }
+ }
@@ reftable/reftable_test.c (new)
+ strbuf_release(&buf);
+}
+
-+int reftable_test_main(int argc, const char *argv[])
++static void test_write_key_order(void)
++{
++ struct reftable_write_options opts = { 0 };
++ struct strbuf buf = STRBUF_INIT;
++ struct reftable_writer *w =
++ reftable_new_writer(&strbuf_add_void, &buf, &opts);
++ struct reftable_ref_record refs[2] = {
++ {
++ .refname = "b",
++ .update_index = 1,
++ .value_type = REFTABLE_REF_SYMREF,
++ .value = {
++ .symref = "target",
++ },
++ }, {
++ .refname = "a",
++ .update_index = 1,
++ .value_type = REFTABLE_REF_SYMREF,
++ .value = {
++ .symref = "target",
++ },
++ }
++ };
++ int err;
++
++ reftable_writer_set_limits(w, 1, 1);
++ err = reftable_writer_add_ref(w, &refs[0]);
++ EXPECT_ERR(err);
++ err = reftable_writer_add_ref(w, &refs[1]);
++ printf("%d\n", err);
++ EXPECT(err == REFTABLE_API_ERROR);
++ reftable_writer_close(w);
++ reftable_writer_free(w);
++ strbuf_release(&buf);
++}
++
++int readwrite_test_main(int argc, const char *argv[])
+{
+ RUN_TEST(test_log_write_read);
++ RUN_TEST(test_write_key_order);
+ RUN_TEST(test_table_read_write_seek_linear_sha256);
+ RUN_TEST(test_log_buffer_size);
+ RUN_TEST(test_table_write_small_table);
@@ reftable/reftable_test.c (new)
+ return 0;
+}
+ ## reftable/reftable-tests.h ##
+@@ reftable/reftable-tests.h: int block_test_main(int argc, const char **argv);
+ int merged_test_main(int argc, const char **argv);
+ int record_test_main(int argc, const char **argv);
+ int refname_test_main(int argc, const char **argv);
+-int reftable_test_main(int argc, const char **argv);
++int readwrite_test_main(int argc, const char **argv);
+ int stack_test_main(int argc, const char **argv);
+ int tree_test_main(int argc, const char **argv);
+ int reftable_dump_main(int argc, char *const *argv);
+
## t/helper/test-reftable.c ##
@@ t/helper/test-reftable.c: int cmd__reftable(int argc, const char **argv)
basics_test_main(argc, argv);
block_test_main(argc, argv);
record_test_main(argc, argv);
-+ reftable_test_main(argc, argv);
++ readwrite_test_main(argc, argv);
tree_test_main(argc, argv);
return 0;
}
13: 0a6119d910a3 ! 18: 84578b666a40 reftable: add a heap-based priority queue for reftable records
@@ Makefile: REFTABLE_OBJS += reftable/block.o
+REFTABLE_OBJS += reftable/pq.o
REFTABLE_OBJS += reftable/reader.o
REFTABLE_OBJS += reftable/record.o
- REFTABLE_OBJS += reftable/reftable.o
-@@ Makefile: REFTABLE_OBJS += reftable/zlib-compat.o
+ REFTABLE_OBJS += reftable/refname.o
+@@ Makefile: REFTABLE_OBJS += reftable/writer.o
REFTABLE_TEST_OBJS += reftable/basics_test.o
REFTABLE_TEST_OBJS += reftable/block_test.o
+REFTABLE_TEST_OBJS += reftable/pq_test.o
REFTABLE_TEST_OBJS += reftable/record_test.o
- REFTABLE_TEST_OBJS += reftable/reftable_test.o
+ REFTABLE_TEST_OBJS += reftable/readwrite_test.o
REFTABLE_TEST_OBJS += reftable/test_framework.o
## reftable/pq.c (new) ##
@@ reftable/pq_test.c (new)
+
+ merged_iter_pqueue_check(pq);
+
-+ if (last != NULL) {
++ if (last) {
+ assert(strcmp(last, ref->refname) < 0);
+ }
+ last = ref->refname;
@@ reftable/reftable-tests.h: license that can be found in the LICENSE file or at
+int pq_test_main(int argc, const char **argv);
int record_test_main(int argc, const char **argv);
int refname_test_main(int argc, const char **argv);
- int reftable_test_main(int argc, const char **argv);
+ int readwrite_test_main(int argc, const char **argv);
## t/helper/test-reftable.c ##
@@ t/helper/test-reftable.c: int cmd__reftable(int argc, const char **argv)
@@ t/helper/test-reftable.c: int cmd__reftable(int argc, const char **argv)
block_test_main(argc, argv);
+ pq_test_main(argc, argv);
record_test_main(argc, argv);
- reftable_test_main(argc, argv);
+ readwrite_test_main(argc, argv);
tree_test_main(argc, argv);
14: 393fcecdae1e ! 19: e3a776f2076c reftable: add merged table view
@@ Makefile: REFTABLE_OBJS += reftable/block.o
REFTABLE_OBJS += reftable/pq.o
REFTABLE_OBJS += reftable/reader.o
REFTABLE_OBJS += reftable/record.o
-@@ Makefile: REFTABLE_OBJS += reftable/zlib-compat.o
+@@ Makefile: REFTABLE_OBJS += reftable/writer.o
REFTABLE_TEST_OBJS += reftable/basics_test.o
REFTABLE_TEST_OBJS += reftable/block_test.o
+REFTABLE_TEST_OBJS += reftable/merged_test.o
REFTABLE_TEST_OBJS += reftable/pq_test.o
REFTABLE_TEST_OBJS += reftable/record_test.o
- REFTABLE_TEST_OBJS += reftable/reftable_test.o
+ REFTABLE_TEST_OBJS += reftable/readwrite_test.o
## reftable/merged.c (new) ##
@@
@@ reftable/merged.c (new)
+
+static void merged_iter_close(void *p)
+{
-+ struct merged_iter *mi = (struct merged_iter *)p;
++ struct merged_iter *mi = p;
+ int i = 0;
+ merged_iter_pqueue_release(&mi->pq);
+ for (i = 0; i < mi->stack_len; i++) {
@@ reftable/merged.c (new)
+
+static int merged_iter_next_void(void *p, struct reftable_record *rec)
+{
-+ struct merged_iter *mi = (struct merged_iter *)p;
++ struct merged_iter *mi = p;
+ if (merged_iter_pqueue_is_empty(mi->pq))
+ return 1;
+
@@ reftable/merged.c (new)
+static void iterator_from_merged_iter(struct reftable_iterator *it,
+ struct merged_iter *mi)
+{
-+ assert(it->ops == NULL);
++ assert(!it->ops);
+ it->iter_arg = mi;
+ it->ops = &merged_iter_vtable;
+}
@@ reftable/merged.c (new)
+ }
+ }
+
-+ m = (struct reftable_merged_table *)reftable_calloc(
-+ sizeof(struct reftable_merged_table));
++ m = reftable_calloc(sizeof(struct reftable_merged_table));
+ m->stack = stack;
+ m->stack_len = n;
+ m->min = first_min;
@@ reftable/merged.c (new)
+
+void reftable_merged_table_free(struct reftable_merged_table *mt)
+{
-+ if (mt == NULL) {
++ if (!mt) {
+ return;
+ }
+ merged_table_release(mt);
@@ reftable/merged.c (new)
+ struct reftable_iterator *it,
+ struct reftable_record *rec)
+{
-+ return merged_table_seek_record((struct reftable_merged_table *)tab, it,
-+ rec);
++ return merged_table_seek_record(tab, it, rec);
+}
+
+static uint32_t reftable_merged_table_hash_id_void(void *tab)
+{
-+ return reftable_merged_table_hash_id(
-+ (struct reftable_merged_table *)tab);
++ return reftable_merged_table_hash_id(tab);
+}
+
+static uint64_t reftable_merged_table_min_update_index_void(void *tab)
+{
-+ return reftable_merged_table_min_update_index(
-+ (struct reftable_merged_table *)tab);
++ return reftable_merged_table_min_update_index(tab);
+}
+
+static uint64_t reftable_merged_table_max_update_index_void(void *tab)
+{
-+ return reftable_merged_table_max_update_index(
-+ (struct reftable_merged_table *)tab);
++ return reftable_merged_table_max_update_index(tab);
+}
+
+static struct reftable_table_vtable merged_table_vtable = {
@@ reftable/merged.c (new)
+void reftable_table_from_merged_table(struct reftable_table *tab,
+ struct reftable_merged_table *merged)
+{
-+ assert(tab->ops == NULL);
++ assert(!tab->ops);
+ tab->ops = &merged_table_vtable;
+ tab->table_arg = merged;
+}
@@ reftable/merged_test.c (new)
+ reftable_table_from_reader(&tabs[i], (*readers)[i]);
+ }
+
-+ err = reftable_new_merged_table(&mt, tabs, n, SHA1_ID);
++ err = reftable_new_merged_table(&mt, tabs, n, GIT_SHA1_HASH_ID);
+ EXPECT_ERR(err);
+ return mt;
+}
@@ reftable/merged_test.c (new)
+
+static void test_merged_between(void)
+{
-+ uint8_t hash1[SHA1_SIZE] = { 1, 2, 3, 0 };
++ uint8_t hash1[GIT_SHA1_RAWSZ] = { 1, 2, 3, 0 };
+
+ struct reftable_ref_record r1[] = { {
+ .refname = "b",
@@ reftable/merged_test.c (new)
+
+static void test_merged(void)
+{
-+ uint8_t hash1[SHA1_SIZE] = { 1 };
-+ uint8_t hash2[SHA1_SIZE] = { 2 };
++ uint8_t hash1[GIT_SHA1_RAWSZ] = { 1 };
++ uint8_t hash2[GIT_SHA1_RAWSZ] = { 2 };
+ struct reftable_ref_record r1[] = {
+ {
+ .refname = "a",
@@ reftable/merged_test.c (new)
+
+ assert(ARRAY_SIZE(want) == len);
+ for (i = 0; i < len; i++) {
-+ assert(reftable_ref_record_equal(&want[i], &out[i], SHA1_SIZE));
++ assert(reftable_ref_record_equal(&want[i], &out[i],
++ GIT_SHA1_RAWSZ));
+ }
+ for (i = 0; i < len; i++) {
+ reftable_ref_record_release(&out[i]);
@@ reftable/merged_test.c (new)
+ EXPECT_ERR(err);
+
+ hash_id = reftable_reader_hash_id(rd);
-+ assert(hash_id == SHA1_ID);
++ assert(hash_id == GIT_SHA1_HASH_ID);
+
+ reftable_table_from_reader(&tab[0], rd);
-+ err = reftable_new_merged_table(&merged, tab, 1, SHA1_ID);
++ err = reftable_new_merged_table(&merged, tab, 1, GIT_SHA1_HASH_ID);
+ EXPECT_ERR(err);
+
+ reftable_reader_free(rd);
@@ t/helper/test-reftable.c: int cmd__reftable(int argc, const char **argv)
+ merged_test_main(argc, argv);
pq_test_main(argc, argv);
record_test_main(argc, argv);
- reftable_test_main(argc, argv);
+ readwrite_test_main(argc, argv);
15: 3186b2b70f73 ! 20: 7913027063a2 reftable: implement refname validation
@@ Commit message
Signed-off-by: Han-Wen Nienhuys <[email protected]>
## Makefile ##
-@@ Makefile: REFTABLE_OBJS += reftable/merged.o
- REFTABLE_OBJS += reftable/pq.o
- REFTABLE_OBJS += reftable/reader.o
- REFTABLE_OBJS += reftable/record.o
-+REFTABLE_OBJS += reftable/refname.o
- REFTABLE_OBJS += reftable/reftable.o
- REFTABLE_OBJS += reftable/tree.o
- REFTABLE_OBJS += reftable/writer.o
-@@ Makefile: REFTABLE_TEST_OBJS += reftable/block_test.o
- REFTABLE_TEST_OBJS += reftable/merged_test.o
+@@ Makefile: REFTABLE_TEST_OBJS += reftable/merged_test.o
REFTABLE_TEST_OBJS += reftable/pq_test.o
REFTABLE_TEST_OBJS += reftable/record_test.o
+ REFTABLE_TEST_OBJS += reftable/readwrite_test.o
+REFTABLE_TEST_OBJS += reftable/refname_test.o
- REFTABLE_TEST_OBJS += reftable/reftable_test.o
REFTABLE_TEST_OBJS += reftable/test_framework.o
REFTABLE_TEST_OBJS += reftable/tree_test.o
+
## reftable/refname.c (new) ##
@@
@@ reftable/refname.c (new)
+
+static int find_name(size_t k, void *arg)
+{
-+ struct find_arg *f_arg = (struct find_arg *)arg;
++ struct find_arg *f_arg = arg;
+ return strcmp(f_arg->names[k], f_arg->want) >= 0;
+}
+
@@ reftable/refname_test.c (new)
+ .tab = tab,
+ };
+
-+ if (cases[i].add != NULL) {
++ if (cases[i].add) {
+ mod.add = &cases[i].add;
+ mod.add_len = 1;
+ }
-+ if (cases[i].del != NULL) {
++ if (cases[i].del) {
+ mod.del = &cases[i].del;
+ mod.del_len = 1;
+ }
@@ t/helper/test-reftable.c: int cmd__reftable(int argc, const char **argv)
pq_test_main(argc, argv);
record_test_main(argc, argv);
+ refname_test_main(argc, argv);
- reftable_test_main(argc, argv);
+ readwrite_test_main(argc, argv);
tree_test_main(argc, argv);
return 0;
16: b5492d5a13d7 ! 21: 3f1e5aedf4a8 reftable: implement stack, a mutable database of reftable files.
@@ Commit message
Signed-off-by: Han-Wen Nienhuys <[email protected]>
## Makefile ##
-@@ Makefile: REFTABLE_OBJS += reftable/reader.o
- REFTABLE_OBJS += reftable/record.o
- REFTABLE_OBJS += reftable/refname.o
- REFTABLE_OBJS += reftable/reftable.o
-+REFTABLE_OBJS += reftable/stack.o
- REFTABLE_OBJS += reftable/tree.o
- REFTABLE_OBJS += reftable/writer.o
- REFTABLE_OBJS += reftable/zlib-compat.o
@@ Makefile: REFTABLE_TEST_OBJS += reftable/pq_test.o
REFTABLE_TEST_OBJS += reftable/record_test.o
+ REFTABLE_TEST_OBJS += reftable/readwrite_test.o
REFTABLE_TEST_OBJS += reftable/refname_test.o
- REFTABLE_TEST_OBJS += reftable/reftable_test.o
+REFTABLE_TEST_OBJS += reftable/stack_test.o
REFTABLE_TEST_OBJS += reftable/test_framework.o
REFTABLE_TEST_OBJS += reftable/tree_test.o
@@ reftable/reftable-stack.h (new)
+ void *arg),
+ void *arg);
+
-+/* Commits the transaction, releasing the lock. */
++/* Commits the transaction, releasing the lock. After calling this,
++ * reftable_addition_destroy should still be called.
++ */
+int reftable_addition_commit(struct reftable_addition *add);
+
+/* Release all non-committed data from the transaction, and deallocate the
@@ reftable/reftable-stack.h (new)
+struct reftable_compaction_stats *
+reftable_stack_compaction_stats(struct reftable_stack *st);
+
++/* print the entire stack represented by the directory */
++int reftable_stack_print_directory(const char *stackdir);
++
+#endif
## reftable/stack.c (new) ##
@@ reftable/stack.c (new)
+ strbuf_addstr(dest, name);
+}
+
-+static int reftable_fd_write(void *arg, const void *data, size_t sz)
++static ssize_t reftable_fd_write(void *arg, const void *data, size_t sz)
+{
+ int *fdp = (int *)arg;
+ return write(*fdp, data, sz);
@@ reftable/stack.c (new)
+ int err = 0;
+
+ if (config.hash_id == 0) {
-+ config.hash_id = SHA1_ID;
++ config.hash_id = GIT_SHA1_HASH_ID;
+ }
+
+ *dest = NULL;
@@ reftable/stack.c (new)
+{
+ char **names = NULL;
+ int err = 0;
-+ if (st->merged != NULL) {
++ if (st->merged) {
+ reftable_merged_table_free(st->merged);
+ st->merged = NULL;
+ }
@@ reftable/stack.c (new)
+ FREE_AND_NULL(names);
+ }
+
-+ if (st->readers != NULL) {
++ if (st->readers) {
+ int i = 0;
+ struct strbuf filename = STRBUF_INIT;
+ for (i = 0; i < st->readers_len; i++) {
@@ reftable/stack.c (new)
+static int reftable_stack_reload_once(struct reftable_stack *st, char **names,
+ int reuse_open)
+{
-+ int cur_len = st->merged == NULL ? 0 : st->merged->stack_len;
++ int cur_len = !st->merged ? 0 : st->merged->stack_len;
+ struct reftable_reader **cur = stack_copy_readers(st, cur_len);
+ int err = 0;
+ int names_len = names_length(names);
@@ reftable/stack.c (new)
+ tables under control so this is not quadratic. */
+ int j = 0;
+ for (j = 0; reuse_open && j < cur_len; j++) {
-+ if (cur[j] != NULL && 0 == strcmp(cur[j]->name, name)) {
++ if (cur[j] && 0 == strcmp(cur[j]->name, name)) {
+ rd = cur[j];
+ cur[j] = NULL;
+ break;
+ }
+ }
+
-+ if (rd == NULL) {
++ if (!rd) {
+ struct reftable_block_source src = { NULL };
+ struct strbuf table_path = STRBUF_INIT;
+ stack_filename(&table_path, st, name);
@@ reftable/stack.c (new)
+
+ new_tables = NULL;
+ st->readers_len = new_readers_len;
-+ if (st->merged != NULL) {
++ if (st->merged) {
+ merged_table_release(st->merged);
+ reftable_merged_table_free(st->merged);
+ }
-+ if (st->readers != NULL) {
++ if (st->readers) {
+ reftable_free(st->readers);
+ }
+ st->readers = new_readers;
@@ reftable/stack.c (new)
+ new_merged->suppress_deletions = 1;
+ st->merged = new_merged;
+ for (i = 0; i < cur_len; i++) {
-+ if (cur[i] != NULL) {
++ if (cur[i]) {
+ const char *name = reader_name(cur[i]);
+ struct strbuf filename = STRBUF_INIT;
+ stack_filename(&filename, st, name);
@@ reftable/stack.c (new)
+ return err;
+
+ for (i = 0; i < st->readers_len; i++) {
-+ if (names[i] == NULL) {
++ if (!names[i]) {
+ err = 1;
+ goto done;
+ }
@@ reftable/stack.c (new)
+ }
+ }
+
-+ if (names[st->merged->stack_len] != NULL) {
++ if (names[st->merged->stack_len]) {
+ err = 1;
+ goto done;
+ }
@@ reftable/stack.c (new)
+
+void reftable_addition_destroy(struct reftable_addition *add)
+{
-+ if (add == NULL) {
++ if (!add) {
+ return;
+ }
+ reftable_addition_close(add);
@@ reftable/stack.c (new)
+ continue;
+ }
+
-+ if (config != NULL && config->min_update_index > 0 &&
++ if (config && config->min_update_index > 0 &&
+ log.update_index < config->min_update_index) {
+ continue;
+ }
+
-+ if (config != NULL && config->time > 0 &&
++ if (config && config->time > 0 &&
+ log.update.time < config->time) {
+ continue;
+ }
@@ reftable/stack.c (new)
+
+done:
+ reftable_iterator_destroy(&it);
-+ if (mt != NULL) {
++ if (mt) {
+ merged_table_release(mt);
+ reftable_merged_table_free(mt);
+ }
@@ reftable/stack.c (new)
+ int j = 0;
+ int is_empty_table = 0;
+
-+ if (first > last || (expiry == NULL && first == last)) {
++ if (first > last || (!expiry && first == last)) {
+ err = 0;
+ goto done;
+ }
@@ reftable/stack.c (new)
+{
+ uint64_t *sizes =
+ reftable_calloc(sizeof(uint64_t) * st->merged->stack_len);
-+ int version = (st->config.hash_id == SHA1_ID) ? 1 : 2;
++ int version = (st->config.hash_id == GIT_SHA1_HASH_ID) ? 1 : 2;
+ int overhead = header_size(version) - 1;
+ int i = 0;
+ for (i = 0; i < st->merged->stack_len; i++) {
@@ reftable/stack.c (new)
+static int is_table_name(const char *s)
+{
+ const char *dot = strrchr(s, '.');
-+ return dot != NULL && !strcmp(dot, ".ref");
++ return dot && !strcmp(dot, ".ref");
+}
+
+static void remove_maybe_stale_table(struct reftable_stack *st, uint64_t max,
@@ reftable/stack.c (new)
+ reftable_stack_merged_table(st));
+ DIR *dir = opendir(st->reftable_dir);
+ struct dirent *d = NULL;
-+ if (dir == NULL) {
++ if (!dir) {
+ return REFTABLE_IO_ERROR;
+ }
+
-+ while ((d = readdir(dir)) != NULL) {
++ while ((d = readdir(dir))) {
+ int i = 0;
+ int found = 0;
+ if (!is_table_name(d->d_name))
@@ reftable/stack.c (new)
+done:
+ reftable_addition_destroy(add);
+ return err;
++}
++
++int reftable_stack_print_directory(const char *stackdir)
++{
++ struct reftable_stack *stack = NULL;
++ struct reftable_write_options cfg = { 0 };
++ struct reftable_merged_table *merged = NULL;
++ struct reftable_table table = { NULL };
++
++ int err = reftable_new_stack(&stack, stackdir, cfg);
++ if (err < 0)
++ goto done;
++
++ merged = reftable_stack_merged_table(stack);
++ reftable_table_from_merged_table(&table, merged);
++ err = reftable_table_print(&table);
++done:
++ reftable_stack_destroy(stack);
++ return err;
+}
## reftable/stack.h (new) ##
@@ reftable/stack_test.c (new)
+ if (dir == NULL)
+ return 0;
+
-+ while ((d = readdir(dir)) != NULL) {
++ while ((d = readdir(dir))) {
+ if (!strcmp(d->d_name, "..") || !strcmp(d->d_name, "."))
+ continue;
+ len++;
@@ reftable/stack_test.c (new)
+ return template;
+}
+
++static char *get_tmp_dir(const char *prefix)
++{
++ char *dir = get_tmp_template(prefix);
++ EXPECT(mkdtemp(dir));
++ return dir;
++}
++
+static void test_read_file(void)
+{
+ char *fn = get_tmp_template(__FUNCTION__);
@@ reftable/stack_test.c (new)
+ err = read_lines(fn, &names);
+ EXPECT_ERR(err);
+
-+ for (i = 0; names[i] != NULL; i++) {
++ for (i = 0; names[i]; i++) {
+ EXPECT(0 == strcmp(want[i], names[i]));
+ }
+ free_names(names);
@@ reftable/stack_test.c (new)
+
+static void test_reftable_stack_add_one(void)
+{
-+ char *dir = get_tmp_template(__FUNCTION__);
++ char *dir = get_tmp_dir(__FUNCTION__);
++
+ struct reftable_write_options cfg = { 0 };
+ struct reftable_stack *st = NULL;
+ int err;
@@ reftable/stack_test.c (new)
+ };
+ struct reftable_ref_record dest = { NULL };
+
-+ EXPECT(mkdtemp(dir));
+
+ err = reftable_new_stack(&st, dir, cfg);
+ EXPECT_ERR(err);
@@ reftable/stack_test.c (new)
+ struct reftable_write_options cfg = { 0 };
+ struct reftable_stack *st1 = NULL;
+ struct reftable_stack *st2 = NULL;
-+ char *dir = get_tmp_template(__FUNCTION__);
++ char *dir = get_tmp_dir(__FUNCTION__);
++
+ int err;
+ struct reftable_ref_record ref1 = {
+ .refname = "HEAD",
@@ reftable/stack_test.c (new)
+ .value.symref = "master",
+ };
+
-+ EXPECT(mkdtemp(dir));
+
+ /* simulate multi-process access to the same stack
+ by creating two stacks for the same directory.
@@ reftable/stack_test.c (new)
+
+static void test_reftable_stack_transaction_api(void)
+{
-+ char *dir = get_tmp_template(__FUNCTION__);
++ char *dir = get_tmp_dir(__FUNCTION__);
++
+ struct reftable_write_options cfg = { 0 };
+ struct reftable_stack *st = NULL;
+ int err;
@@ reftable/stack_test.c (new)
+ };
+ struct reftable_ref_record dest = { NULL };
+
-+ EXPECT(mkdtemp(dir));
+
+ err = reftable_new_stack(&st, dir, cfg);
+ EXPECT_ERR(err);
@@ reftable/stack_test.c (new)
+ struct reftable_write_options cfg = { 0 };
+ struct reftable_stack *st = NULL;
+ int err;
-+ char *dir = get_tmp_template(__FUNCTION__);
++ char *dir = get_tmp_dir(__FUNCTION__);
++
+ int i;
+ struct reftable_ref_record ref = {
+ .refname = "a/b",
@@ reftable/stack_test.c (new)
+ };
+ char *additions[] = { "a", "a/b/c" };
+
-+ EXPECT(mkdtemp(dir));
+ err = reftable_new_stack(&st, dir, cfg);
+ EXPECT_ERR(err);
+
@@ reftable/stack_test.c (new)
+
+static void test_reftable_stack_update_index_check(void)
+{
-+ char *dir = get_tmp_template(__FUNCTION__);
++ char *dir = get_tmp_dir(__FUNCTION__);
++
+ struct reftable_write_options cfg = { 0 };
+ struct reftable_stack *st = NULL;
+ int err;
@@ reftable/stack_test.c (new)
+ .value_type = REFTABLE_REF_SYMREF,
+ .value.symref = "master",
+ };
-+ EXPECT(mkdtemp(dir));
+
+ err = reftable_new_stack(&st, dir, cfg);
+ EXPECT_ERR(err);
@@ reftable/stack_test.c (new)
+
+static void test_reftable_stack_lock_failure(void)
+{
-+ char *dir = get_tmp_template(__FUNCTION__);
++ char *dir = get_tmp_dir(__FUNCTION__);
++
+ struct reftable_write_options cfg = { 0 };
+ struct reftable_stack *st = NULL;
+ int err, i;
-+ EXPECT(mkdtemp(dir));
+
+ err = reftable_new_stack(&st, dir, cfg);
+ EXPECT_ERR(err);
@@ reftable/stack_test.c (new)
+ .exact_log_message = 1,
+ };
+ struct reftable_stack *st = NULL;
-+ char *dir = get_tmp_template(__FUNCTION__);
++ char *dir = get_tmp_dir(__FUNCTION__);
++
+ struct reftable_ref_record refs[2] = { { NULL } };
+ struct reftable_log_record logs[2] = { { NULL } };
+ int N = ARRAY_SIZE(refs);
+
-+ EXPECT(mkdtemp(dir));
+
+ err = reftable_new_stack(&st, dir, cfg);
+ EXPECT_ERR(err);
@@ reftable/stack_test.c (new)
+ refs[i].refname = xstrdup(buf);
+ refs[i].update_index = i + 1;
+ refs[i].value_type = REFTABLE_REF_VAL1;
-+ refs[i].value.val1 = reftable_malloc(SHA1_SIZE);
++ refs[i].value.val1 = reftable_malloc(GIT_SHA1_RAWSZ);
+ set_test_hash(refs[i].value.val1, i);
+
+ logs[i].refname = xstrdup(buf);
+ logs[i].update_index = N + i + 1;
+ logs[i].value_type = REFTABLE_LOG_UPDATE;
+
-+ logs[i].update.new_hash = reftable_malloc(SHA1_SIZE);
++ logs[i].update.new_hash = reftable_malloc(GIT_SHA1_RAWSZ);
+ logs[i].update.email = xstrdup("identity@invalid");
+ set_test_hash(logs[i].update.new_hash, i);
+ }
@@ reftable/stack_test.c (new)
+
+ int err = reftable_stack_read_ref(st, refs[i].refname, &dest);
+ EXPECT_ERR(err);
-+ EXPECT(reftable_ref_record_equal(&dest, refs + i, SHA1_SIZE));
++ EXPECT(reftable_ref_record_equal(&dest, refs + i,
++ GIT_SHA1_RAWSZ));
+ reftable_ref_record_release(&dest);
+ }
+
@@ reftable/stack_test.c (new)
+ struct reftable_log_record dest = { NULL };
+ int err = reftable_stack_read_log(st, refs[i].refname, &dest);
+ EXPECT_ERR(err);
-+ EXPECT(reftable_log_record_equal(&dest, logs + i, SHA1_SIZE));
++ EXPECT(reftable_log_record_equal(&dest, logs + i,
++ GIT_SHA1_RAWSZ));
+ reftable_log_record_release(&dest);
+ }
+
@@ reftable/stack_test.c (new)
+ 0,
+ };
+ struct reftable_stack *st = NULL;
-+ char *dir = get_tmp_template(__FUNCTION__);
++ char *dir = get_tmp_dir(__FUNCTION__);
++
+
-+ uint8_t h1[SHA1_SIZE] = { 0x01 }, h2[SHA1_SIZE] = { 0x02 };
++ uint8_t h1[GIT_SHA1_RAWSZ] = { 0x01 }, h2[GIT_SHA1_RAWSZ] = { 0x02 };
+
+ struct reftable_log_record input = { .refname = "branch",
+ .update_index = 1,
@@ reftable/stack_test.c (new)
+ .update_index = 1,
+ };
+
-+ EXPECT(mkdtemp(dir));
+ err = reftable_new_stack(&st, dir, cfg);
+ EXPECT_ERR(err);
+
@@ reftable/stack_test.c (new)
+static void test_reftable_stack_tombstone(void)
+{
+ int i = 0;
-+ char *dir = get_tmp_template(__FUNCTION__);
++ char *dir = get_tmp_dir(__FUNCTION__);
++
+ struct reftable_write_options cfg = { 0 };
+ struct reftable_stack *st = NULL;
+ int err;
@@ reftable/stack_test.c (new)
+ struct reftable_ref_record dest = { NULL };
+ struct reftable_log_record log_dest = { NULL };
+
-+ EXPECT(mkdtemp(dir));
+
+ err = reftable_new_stack(&st, dir, cfg);
+ EXPECT_ERR(err);
+
++ /* even entries add the refs, odd entries delete them. */
+ for (i = 0; i < N; i++) {
+ const char *buf = "branch";
+ refs[i].refname = xstrdup(buf);
+ refs[i].update_index = i + 1;
+ if (i % 2 == 0) {
+ refs[i].value_type = REFTABLE_REF_VAL1;
-+ refs[i].value.val1 = reftable_malloc(SHA1_SIZE);
++ refs[i].value.val1 = reftable_malloc(GIT_SHA1_RAWSZ);
+ set_test_hash(refs[i].value.val1, i);
+ }
+
@@ reftable/stack_test.c (new)
+ logs[i].update_index = 42;
+ if (i % 2 == 0) {
+ logs[i].value_type = REFTABLE_LOG_UPDATE;
-+ logs[i].update.new_hash = reftable_malloc(SHA1_SIZE);
++ logs[i].update.new_hash =
++ reftable_malloc(GIT_SHA1_RAWSZ);
+ set_test_hash(logs[i].update.new_hash, i);
+ logs[i].update.email = xstrdup("identity@invalid");
+ }
@@ reftable/stack_test.c (new)
+ int err = reftable_stack_add(st, &write_test_ref, &refs[i]);
+ EXPECT_ERR(err);
+ }
++
+ for (i = 0; i < N; i++) {
+ struct write_log_arg arg = {
+ .log = &logs[i],
@@ reftable/stack_test.c (new)
+
+static void test_reftable_stack_hash_id(void)
+{
-+ char *dir = get_tmp_template(__FUNCTION__);
++ char *dir = get_tmp_dir(__FUNCTION__);
++
+ struct reftable_write_options cfg = { 0 };
+ struct reftable_stack *st = NULL;
+ int err;
@@ reftable/stack_test.c (new)
+ .value.symref = "target",
+ .update_index = 1,
+ };
-+ struct reftable_write_options cfg32 = { .hash_id = SHA256_ID };
++ struct reftable_write_options cfg32 = { .hash_id = GIT_SHA256_HASH_ID };
+ struct reftable_stack *st32 = NULL;
+ struct reftable_write_options cfg_default = { 0 };
+ struct reftable_stack *st_default = NULL;
+ struct reftable_ref_record dest = { NULL };
+
-+ EXPECT(mkdtemp(dir));
+ err = reftable_new_stack(&st, dir, cfg);
+ EXPECT_ERR(err);
+
@@ reftable/stack_test.c (new)
+ err = reftable_stack_read_ref(st_default, "master", &dest);
+ EXPECT_ERR(err);
+
-+ EXPECT(reftable_ref_record_equal(&ref, &dest, SHA1_SIZE));
++ EXPECT(reftable_ref_record_equal(&ref, &dest, GIT_SHA1_RAWSZ));
+ reftable_ref_record_release(&dest);
+ reftable_stack_destroy(st);
+ reftable_stack_destroy(st_default);
@@ reftable/stack_test.c (new)
+
+static void test_reflog_expire(void)
+{
-+ char *dir = get_tmp_template(__FUNCTION__);
++ char *dir = get_tmp_dir(__FUNCTION__);
++
+ struct reftable_write_options cfg = { 0 };
+ struct reftable_stack *st = NULL;
+ struct reftable_log_record logs[20] = { { NULL } };
@@ reftable/stack_test.c (new)
+ };
+ struct reftable_log_record log = { NULL };
+
-+ EXPECT(mkdtemp(dir));
+
+ err = reftable_new_stack(&st, dir, cfg);
+ EXPECT_ERR(err);
@@ reftable/stack_test.c (new)
+ logs[i].update_index = i;
+ logs[i].value_type = REFTABLE_LOG_UPDATE;
+ logs[i].update.time = i;
-+ logs[i].update.new_hash = reftable_malloc(SHA1_SIZE);
++ logs[i].update.new_hash = reftable_malloc(GIT_SHA1_RAWSZ);
+ logs[i].update.email = xstrdup("identity@invalid");
+ set_test_hash(logs[i].update.new_hash, i);
+ }
@@ reftable/stack_test.c (new)
+ struct reftable_write_options cfg = { 0 };
+ struct reftable_stack *st = NULL;
+ int err;
-+ char *dir = get_tmp_template(__FUNCTION__);
++ char *dir = get_tmp_dir(__FUNCTION__);
++
+ struct reftable_stack *st2 = NULL;
+
-+ EXPECT(mkdtemp(dir));
+
+ err = reftable_new_stack(&st, dir, cfg);
+ EXPECT_ERR(err);
@@ reftable/stack_test.c (new)
+{
+ struct reftable_write_options cfg = { 0 };
+ struct reftable_stack *st = NULL;
-+ char *dir = get_tmp_template(__FUNCTION__);
++ char *dir = get_tmp_dir(__FUNCTION__);
++
+ int err, i;
+ int N = 100;
-+ EXPECT(mkdtemp(dir));
+
+ err = reftable_new_stack(&st, dir, cfg);
+ EXPECT_ERR(err);
@@ reftable/stack_test.c (new)
+{
+ struct reftable_write_options cfg = { 0 };
+ struct reftable_stack *st1 = NULL, *st2 = NULL;
-+ char *dir = get_tmp_template(__FUNCTION__);
++ char *dir = get_tmp_dir(__FUNCTION__);
++
+ int err, i;
+ int N = 3;
-+ EXPECT(mkdtemp(dir));
+
+ err = reftable_new_stack(&st1, dir, cfg);
+ EXPECT_ERR(err);
@@ reftable/stack_test.c (new)
+{
+ struct reftable_write_options cfg = { 0 };
+ struct reftable_stack *st1 = NULL, *st2 = NULL, *st3 = NULL;
-+ char *dir = get_tmp_template(__FUNCTION__);
++ char *dir = get_tmp_dir(__FUNCTION__);
++
+ int err, i;
+ int N = 3;
-+ EXPECT(mkdtemp(dir));
+
+ err = reftable_new_stack(&st1, dir, cfg);
+ EXPECT_ERR(err);
@@ reftable/stack_test.c (new)
+
+int stack_test_main(int argc, const char *argv[])
+{
-+ RUN_TEST(test_reftable_stack_compaction_concurrent_clean);
++ RUN_TEST(test_empty_add);
++ RUN_TEST(test_log2);
++ RUN_TEST(test_names_equal);
++ RUN_TEST(test_parse_names);
++ RUN_TEST(test_read_file);
++ RUN_TEST(test_reflog_expire);
++ RUN_TEST(test_reftable_stack_add);
++ RUN_TEST(test_reftable_stack_add_one);
++ RUN_TEST(test_reftable_stack_auto_compaction);
+ RUN_TEST(test_reftable_stack_compaction_concurrent);
-+ RUN_TEST(test_reftable_stack_uptodate);
-+ RUN_TEST(test_reftable_stack_transaction_api);
++ RUN_TEST(test_reftable_stack_compaction_concurrent_clean);
+ RUN_TEST(test_reftable_stack_hash_id);
-+ RUN_TEST(test_sizes_to_segments_all_equal);
-+ RUN_TEST(test_reftable_stack_auto_compaction);
-+ RUN_TEST(test_reftable_stack_validate_refname);
-+ RUN_TEST(test_reftable_stack_update_index_check);
+ RUN_TEST(test_reftable_stack_lock_failure);
+ RUN_TEST(test_reftable_stack_log_normalize);
+ RUN_TEST(test_reftable_stack_tombstone);
-+ RUN_TEST(test_reftable_stack_add_one);
-+ RUN_TEST(test_empty_add);
-+ RUN_TEST(test_reflog_expire);
-+ RUN_TEST(test_suggest_compaction_segment);
-+ RUN_TEST(test_suggest_compaction_segment_nothing);
++ RUN_TEST(test_reftable_stack_transaction_api);
++ RUN_TEST(test_reftable_stack_update_index_check);
++ RUN_TEST(test_reftable_stack_uptodate);
++ RUN_TEST(test_reftable_stack_validate_refname);
+ RUN_TEST(test_sizes_to_segments);
++ RUN_TEST(test_sizes_to_segments_all_equal);
+ RUN_TEST(test_sizes_to_segments_empty);
-+ RUN_TEST(test_log2);
-+ RUN_TEST(test_parse_names);
-+ RUN_TEST(test_read_file);
-+ RUN_TEST(test_names_equal);
-+ RUN_TEST(test_reftable_stack_add);
++ RUN_TEST(test_suggest_compaction_segment);
++ RUN_TEST(test_suggest_compaction_segment_nothing);
+ return 0;
+}
@@ t/helper/test-reftable.c
@@ t/helper/test-reftable.c: int cmd__reftable(int argc, const char **argv)
record_test_main(argc, argv);
refname_test_main(argc, argv);
- reftable_test_main(argc, argv);
+ readwrite_test_main(argc, argv);
+ stack_test_main(argc, argv);
tree_test_main(argc, argv);
return 0;
17: 5db7c8ab7f23 ! 22: b2ac95a25af8 reftable: add dump utility
@@ reftable/dump.c (new)
+#include <unistd.h>
+#include <string.h>
+
-+#include "reftable.h"
++#include "reftable-blocksource.h"
++#include "reftable-error.h"
++#include "reftable-merged.h"
++#include "reftable-record.h"
+#include "reftable-tests.h"
-+
-+static uint32_t hash_id;
-+
-+static int dump_table(const char *tablename)
-+{
-+ struct reftable_block_source src = { 0 };
-+ int err = reftable_block_source_from_file(&src, tablename);
-+ struct reftable_iterator it = { 0 };
-+ struct reftable_ref_record ref = { 0 };
-+ struct reftable_log_record log = { 0 };
-+ struct reftable_reader *r = NULL;
-+
-+ if (err < 0)
-+ return err;
-+
-+ err = reftable_new_reader(&r, &src, tablename);
-+ if (err < 0)
-+ return err;
-+
-+ err = reftable_reader_seek_ref(r, &it, "");
-+ if (err < 0) {
-+ return err;
-+ }
-+
-+ while (1) {
-+ err = reftable_iterator_next_ref(&it, &ref);
-+ if (err > 0) {
-+ break;
-+ }
-+ if (err < 0) {
-+ return err;
-+ }
-+ reftable_ref_record_print(&ref, hash_id);
-+ }
-+ reftable_iterator_destroy(&it);
-+ reftable_ref_record_clear(&ref);
-+
-+ err = reftable_reader_seek_log(r, &it, "");
-+ if (err < 0) {
-+ return err;
-+ }
-+ while (1) {
-+ err = reftable_iterator_next_log(&it, &log);
-+ if (err > 0) {
-+ break;
-+ }
-+ if (err < 0) {
-+ return err;
-+ }
-+ reftable_log_record_print(&log, hash_id);
-+ }
-+ reftable_iterator_destroy(&it);
-+ reftable_log_record_clear(&log);
-+
-+ reftable_reader_free(r);
-+ return 0;
-+}
++#include "reftable-writer.h"
++#include "reftable-iterator.h"
++#include "reftable-reader.h"
++#include "reftable-stack.h"
++#include "reftable-generic.h"
+
+static int compact_stack(const char *stackdir)
+{
+ struct reftable_stack *stack = NULL;
-+ struct reftable_write_options cfg = {};
++ struct reftable_write_options cfg = { 0 };
+
+ int err = reftable_new_stack(&stack, stackdir, cfg);
+ if (err < 0)
@@ reftable/dump.c (new)
+ if (err < 0)
+ goto done;
+done:
-+ if (stack != NULL) {
++ if (stack) {
+ reftable_stack_destroy(stack);
+ }
+ return err;
+}
+
-+static int dump_stack(const char *stackdir)
-+{
-+ struct reftable_stack *stack = NULL;
-+ struct reftable_write_options cfg = {};
-+ struct reftable_iterator it = { 0 };
-+ struct reftable_ref_record ref = { 0 };
-+ struct reftable_log_record log = { 0 };
-+ struct reftable_merged_table *merged = NULL;
-+
-+ int err = reftable_new_stack(&stack, stackdir, cfg);
-+ if (err < 0)
-+ return err;
-+
-+ merged = reftable_stack_merged_table(stack);
-+
-+ err = reftable_merged_table_seek_ref(merged, &it, "");
-+ if (err < 0) {
-+ return err;
-+ }
-+
-+ while (1) {
-+ err = reftable_iterator_next_ref(&it, &ref);
-+ if (err > 0) {
-+ break;
-+ }
-+ if (err < 0) {
-+ return err;
-+ }
-+ reftable_ref_record_print(&ref, hash_id);
-+ }
-+ reftable_iterator_destroy(&it);
-+ reftable_ref_record_clear(&ref);
-+
-+ err = reftable_merged_table_seek_log(merged, &it, "");
-+ if (err < 0) {
-+ return err;
-+ }
-+ while (1) {
-+ err = reftable_iterator_next_log(&it, &log);
-+ if (err > 0) {
-+ break;
-+ }
-+ if (err < 0) {
-+ return err;
-+ }
-+ reftable_log_record_print(&log, hash_id);
-+ }
-+ reftable_iterator_destroy(&it);
-+ reftable_log_record_clear(&log);
-+
-+ reftable_stack_destroy(stack);
-+ return 0;
-+}
-+
+static void print_help(void)
+{
+ printf("usage: dump [-cst] arg\n\n"
@@ reftable/dump.c (new)
+ " -t dump table\n"
+ " -s dump stack\n"
+ " -h this help\n"
-+ " -2 use SHA256\n"
+ "\n");
+}
+
+int reftable_dump_main(int argc, char *const *argv)
+{
+ int err = 0;
-+ int opt;
+ int opt_dump_table = 0;
+ int opt_dump_stack = 0;
+ int opt_compact = 0;
-+ const char *arg = NULL;
-+ while ((opt = getopt(argc, argv, "2chts")) != -1) {
-+ switch (opt) {
-+ case '2':
-+ hash_id = 0x73323536;
++ const char *arg = NULL, *argv0 = argv[0];
++
++ for (; argc > 1; argv++, argc--)
++ if (*argv[1] != '-')
+ break;
-+ case 't':
++ else if (!strcmp("-t", argv[1]))
+ opt_dump_table = 1;
-+ break;
-+ case 's':
++ else if (!strcmp("-s", argv[1]))
+ opt_dump_stack = 1;
-+ break;
-+ case 'c':
++ else if (!strcmp("-c", argv[1]))
+ opt_compact = 1;
-+ break;
-+ case '?':
-+ case 'h':
++ else if (!strcmp("-?", argv[1]) || !strcmp("-h", argv[1])) {
+ print_help();
+ return 2;
-+ break;
+ }
-+ }
+
-+ if (argv[optind] == NULL) {
++ if (argc != 2) {
+ fprintf(stderr, "need argument\n");
+ print_help();
+ return 2;
+ }
+
-+ arg = argv[optind];
++ arg = argv[1];
+
+ if (opt_dump_table) {
-+ err = dump_table(arg);
++ err = reftable_reader_print_file(arg);
+ } else if (opt_dump_stack) {
-+ err = dump_stack(arg);
++ err = reftable_stack_print_directory(arg);
+ } else if (opt_compact) {
+ err = compact_stack(arg);
+ }
+
+ if (err < 0) {
-+ fprintf(stderr, "%s: %s: %s\n", argv[0], arg,
++ fprintf(stderr, "%s: %s: %s\n", argv0, arg,
+ reftable_error_str(err));
+ return 1;
+ }
18: 2dc73bf2ec96 ! 23: 2fd7cb8c0983 Reftable support for git-core
@@ Commit message
Signed-off-by: Johannes Schindelin <[email protected]>
Helped-by: Johannes Schindelin <[email protected]>
Helped-by: Junio Hamano <[email protected]>
+ Helped-by: Patrick Steinhardt <[email protected]>
Co-authored-by: Jeff King <[email protected]>
## Documentation/config/extensions.txt ##
@@ builtin/init-db.c: int cmd_init_db(int argc, const char **argv, const char *pref
+ initial_branch, ref_storage_format, flags);
}
+ ## builtin/stash.c ##
+@@ builtin/stash.c: static int get_stash_info(struct stash_info *info, int argc, const char **argv)
+ static int do_clear_stash(void)
+ {
+ struct object_id obj;
++ int result;
+ if (get_oid(ref_stash, &obj))
+ return 0;
+
+- return delete_ref(NULL, ref_stash, &obj, 0);
++ result = delete_ref(NULL, ref_stash, &obj, 0);
++
++ /* Ignore error; this is necessary for reftable, which keeps reflogs
++ * even when refs are deleted. */
++ delete_reflog(ref_stash);
++ return result;
+ }
+
+ static int clear_stash(int argc, const char **argv, const char *prefix)
+
## builtin/worktree.c ##
@@
#include "utf8.h"
@@ refs.c
+const char *default_ref_storage(void)
+{
-+ const char *test = getenv("GIT_TEST_REFTABLE");
-+ return test ? "reftable" : "files";
++ return git_env_bool("GIT_TEST_REFTABLE", 0) ? "reftable" : "files";
+}
+
/*
@@ refs/reftable-backend.c (new)
+static struct reftable_stack *stack_for(struct git_reftable_ref_store *store,
+ const char *refname)
+{
-+ if (store->worktree_stack == NULL)
++ if (store->worktree_stack == NULL || refname == NULL)
+ return store->main_stack;
+
+ switch (ref_type(refname)) {
@@ refs/reftable-backend.c (new)
+ ri->base.flags = 0;
+ switch (ri->ref.value_type) {
+ case REFTABLE_REF_VAL1:
-+ hashcpy(ri->oid.hash, ri->ref.value.val1);
++ oidread(&ri->oid, ri->ref.value.val1);
+ break;
+ case REFTABLE_REF_VAL2:
-+ hashcpy(ri->oid.hash, ri->ref.value.val2.value);
++ oidread(&ri->oid, ri->ref.value.val2.value);
+ break;
+ case REFTABLE_REF_SYMREF: {
+ int out_flags = 0;
@@ refs/reftable-backend.c (new)
+ struct git_reftable_iterator *ri =
+ (struct git_reftable_iterator *)ref_iterator;
+ if (ri->ref.value_type == REFTABLE_REF_VAL2) {
-+ hashcpy(peeled->hash, ri->ref.value.val2.target_value);
++ oidread(peeled, ri->ref.value.val2.target_value);
+ return 0;
+ }
+
-+ return -1;
++ return 1;
+}
+
+static int reftable_ref_iterator_abort(struct ref_iterator *ref_iterator)
@@ refs/reftable-backend.c (new)
+ (struct git_reftable_ref_store *)ref_store;
+ struct reftable_addition *add = NULL;
+ struct reftable_stack *stack =
-+ transaction->nr ?
-+ stack_for(refs, transaction->updates[0]->refname) :
-+ refs->main_stack;
++ stack_for(refs,
++ transaction->nr ? transaction->updates[0]->refname : NULL);
++
+ int err = refs->err;
+ if (err < 0) {
+ goto done;
@@ refs/reftable-backend.c (new)
+ calloc(transaction->nr, sizeof(*logs));
+ struct ref_update **sorted =
+ malloc(transaction->nr * sizeof(struct ref_update *));
++ struct reftable_merged_table *mt = reftable_stack_merged_table(stack);
++ struct reftable_table tab = {NULL};
++ struct reftable_ref_record ref = {NULL};
++ reftable_table_from_merged_table(&tab, mt);
+ COPY_ARRAY(sorted, transaction->updates, transaction->nr);
+ QSORT(sorted, transaction->nr, ref_update_cmp);
+ reftable_writer_set_limits(writer, ts, ts);
@@ refs/reftable-backend.c (new)
+ for (i = 0; i < transaction->nr; i++) {
+ struct ref_update *u = sorted[i];
+ struct reftable_log_record *log = &logs[i];
++ struct object_id old_id;
+ fill_reftable_log_record(log);
+ log->update_index = ts;
+ log->value_type = REFTABLE_LOG_UPDATE;
+ log->refname = (char *)u->refname;
-+ log->update.old_hash = u->old_oid.hash;
+ log->update.new_hash = u->new_oid.hash;
+ log->update.message = u->msg;
+
++ err = reftable_table_read_ref(&tab, u->refname, &ref);
++ if (err < 0)
++ goto done;
++ else if (err > 0) {
++ old_id = null_oid;
++ } else {
++ oidread(&old_id, ref.value.val1);
++ }
++
++ /* XXX fold together with the old_id check below? */
++
++ log->update.old_hash = old_id.hash;
+ if (u->flags & REF_LOG_ONLY) {
+ continue;
+ }
@@ refs/reftable-backend.c (new)
+
+done:
+ assert(err != REFTABLE_API_ERROR);
++ reftable_ref_record_release(&ref);
+ free(logs);
+ free(sorted);
+ return err;
@@ refs/reftable-backend.c (new)
+ struct git_reftable_ref_store *refs =
+ (struct git_reftable_ref_store *)ref_store;
+ struct reftable_stack *stack =
-+ stack_for(refs, refnames->items[0].string);
++ stack_for(refs, refnames->nr ? refnames->items[0].string : NULL);
+ struct write_delete_refs_arg arg = {
+ .stack = stack,
+ .refnames = refnames,
@@ refs/reftable-backend.c (new)
+{
+ struct write_rename_arg *arg = (struct write_rename_arg *)argv;
+ uint64_t ts = reftable_stack_next_update_index(arg->stack);
-+ struct reftable_ref_record ref = { NULL };
-+ int err = reftable_stack_read_ref(arg->stack, arg->oldname, &ref);
++ struct reftable_ref_record old_ref = { NULL };
++ struct reftable_ref_record new_ref = { NULL };
++ int err = reftable_stack_read_ref(arg->stack, arg->oldname, &old_ref);
+
+ if (err) {
+ goto done;
+ }
+
-+ /* XXX do ref renames overwrite the target? */
-+ if (reftable_stack_read_ref(arg->stack, arg->newname, &ref) == 0) {
++ /* git-branch supports a --force, but the check is not atomic. */
++ if (reftable_stack_read_ref(arg->stack, arg->newname, &new_ref) == 0) {
+ goto done;
+ }
+
@@ refs/reftable-backend.c (new)
+ .update_index = ts,
+ .value_type = REFTABLE_REF_DELETION,
+ },
-+ ref,
++ old_ref,
+ };
+ todo[1].update_index = ts;
+ free(todo[1].refname);
@@ refs/reftable-backend.c (new)
+ }
+ }
+
-+ if (reftable_ref_record_val1(&ref)) {
-+ uint8_t *val1 = reftable_ref_record_val1(&ref);
++ if (reftable_ref_record_val1(&old_ref)) {
++ uint8_t *val1 = reftable_ref_record_val1(&old_ref);
+ struct reftable_log_record todo[2] = { { NULL } };
+ fill_reftable_log_record(&todo[0]);
+ fill_reftable_log_record(&todo[1]);
@@ refs/reftable-backend.c (new)
+
+done:
+ assert(err != REFTABLE_API_ERROR);
-+ reftable_ref_record_release(&ref);
++ reftable_ref_record_release(&new_ref);
++ reftable_ref_record_release(&old_ref);
++ return err;
++}
++
++static int write_copy_table(struct reftable_writer *writer, void *argv)
++{
++ struct write_rename_arg *arg = (struct write_rename_arg *)argv;
++ uint64_t ts = reftable_stack_next_update_index(arg->stack);
++ struct reftable_ref_record old_ref = { NULL };
++ struct reftable_ref_record new_ref = { NULL };
++ struct reftable_log_record log = { NULL };
++ struct reftable_iterator it = { NULL };
++ int err = reftable_stack_read_ref(arg->stack, arg->oldname, &old_ref);
++ if (err) {
++ goto done;
++ }
++
++ /* git-branch supports a --force, but the check is not atomic. */
++ if (reftable_stack_read_ref(arg->stack, arg->newname, &new_ref) == 0) {
++ goto done;
++ }
++
++ reftable_writer_set_limits(writer, ts, ts);
++
++ FREE_AND_NULL(old_ref.refname);
++ old_ref.refname = xstrdup(arg->newname);
++ old_ref.update_index = ts;
++ err = reftable_writer_add_ref(writer, &old_ref);
++ if (err < 0) {
++ goto done;
++ }
++
++ /* this copies the entire reflog history. Is this the right semantics? */
++ /* XXX should clear out existing reflog entries for oldname? */
++ err = reftable_merged_table_seek_log(reftable_stack_merged_table(arg->stack), &it, arg->oldname);
++ if (err < 0) {
++ goto done;
++ }
++ while (1) {
++ int err = reftable_iterator_next_log(&it, &log);
++ if (err < 0) {
++ goto done;
++ }
++
++ if (err > 0 || strcmp(log.refname, arg->oldname)) {
++ break;
++ }
++ FREE_AND_NULL(log.refname);
++ log.refname = xstrdup(arg->newname);
++ reftable_writer_add_log(writer, &log);
++ reftable_log_record_release(&log);
++ }
++
++done:
++ assert(err != REFTABLE_API_ERROR);
++ reftable_ref_record_release(&new_ref);
++ reftable_ref_record_release(&old_ref);
++ reftable_log_record_release(&log);
++ reftable_iterator_destroy(&it);
+ return err;
+}
+
@@ refs/reftable-backend.c (new)
+ const char *oldrefname, const char *newrefname,
+ const char *logmsg)
+{
-+ BUG("reftable reference store does not support copying references");
++ struct git_reftable_ref_store *refs =
++ (struct git_reftable_ref_store *)ref_store;
++ struct reftable_stack *stack = stack_for(refs, newrefname);
++ struct write_rename_arg arg = {
++ .stack = stack,
++ .oldname = oldrefname,
++ .newname = newrefname,
++ .logmsg = logmsg,
++ };
++ int err = refs->err;
++ if (err < 0) {
++ goto done;
++ }
++ err = reftable_stack_reload(stack);
++ if (err) {
++ goto done;
++ }
++
++ err = reftable_stack_add(stack, &write_copy_table, &arg);
++done:
++ assert(err != REFTABLE_API_ERROR);
++ return err;
+}
+
+struct git_reftable_reflog_ref_iterator {
@@ refs/reftable-backend.c (new)
+
+ free(ri->last_name);
+ ri->last_name = xstrdup(ri->log.refname);
-+ hashcpy(ri->oid.hash, ri->log.update.new_hash);
++ oidread(&ri->oid, ri->log.update.new_hash);
+ return ITER_OK;
+ }
+}
@@ refs/reftable-backend.c (new)
+ break;
+ }
+
-+ hashcpy(old_oid.hash, log.update.old_hash);
-+ hashcpy(new_oid.hash, log.update.new_hash);
++ oidread(&old_oid, log.update.old_hash);
++ oidread(&new_oid, log.update.new_hash);
+
+ full_committer = fmt_ident(log.update.name, log.update.email,
+ WANT_COMMITTER_IDENT,
@@ refs/reftable-backend.c (new)
+ struct object_id new_oid;
+ const char *full_committer = "";
+
-+ hashcpy(old_oid.hash, log->update.old_hash);
-+ hashcpy(new_oid.hash, log->update.new_hash);
++ oidread(&old_oid, log->update.old_hash);
++ oidread(&new_oid, log->update.new_hash);
+
+ full_committer = fmt_ident(log->update.name, log->update.email,
+ WANT_COMMITTER_IDENT, NULL,
@@ refs/reftable-backend.c (new)
+ return 0;
+}
+
-+static int git_reftable_delete_reflog(struct ref_store *ref_store,
-+ const char *refname)
-+{
-+ return 0;
-+}
-+
-+struct reflog_expiry_arg {
-+ struct git_reftable_ref_store *refs;
++struct write_reflog_delete_arg {
+ struct reftable_stack *stack;
-+ struct reftable_log_record *tombstones;
-+ int len;
-+ int cap;
++ const char *refname;
+};
+
-+static void clear_log_tombstones(struct reflog_expiry_arg *arg)
++static int write_reflog_delete_table(struct reftable_writer *writer, void *argv)
+{
-+ int i = 0;
-+ for (; i < arg->len; i++) {
-+ reftable_log_record_release(&arg->tombstones[i]);
++ struct write_reflog_delete_arg *arg = argv;
++ struct reftable_merged_table *mt = reftable_stack_merged_table(arg->stack);
++ struct reftable_log_record log = { NULL };
++ struct reftable_iterator it = { NULL };
++ uint64_t ts = reftable_stack_next_update_index(arg->stack);
++ int err = reftable_merged_table_seek_log(mt, &it, arg->refname);
++
++ reftable_writer_set_limits(writer, ts, ts);
++ while (err == 0) {
++ struct reftable_log_record tombstone = {
++ .refname = (char*)arg->refname,
++ .update_index = REFTABLE_LOG_DELETION,
++ };
++ err = reftable_iterator_next_log(&it, &log);
++ if (err > 0) {
++ err = 0;
++ break;
++ }
++
++ if (err < 0 || strcmp(log.refname, arg->refname)) {
++ break;
++ }
++ tombstone.update_index = log.update_index;
++ err = reftable_writer_add_log(writer, &tombstone);
+ }
+
-+ FREE_AND_NULL(arg->tombstones);
++ return err;
+}
+
-+static void add_log_tombstone(struct reflog_expiry_arg *arg,
-+ const char *refname, uint64_t ts)
++
++static int git_reftable_delete_reflog(struct ref_store *ref_store,
++ const char *refname)
+{
-+ struct reftable_log_record tombstone = {
-+ .refname = xstrdup(refname),
-+ .update_index = ts,
++ struct git_reftable_ref_store *refs =
++ (struct git_reftable_ref_store *)ref_store;
++ struct reftable_stack *stack = stack_for(refs, refname);
++ struct write_reflog_delete_arg arg = {
++ .stack = stack,
++ .refname = refname,
+ };
-+ if (arg->len == arg->cap) {
-+ arg->cap = 2 * arg->cap + 1;
-+ arg->tombstones =
-+ realloc(arg->tombstones, arg->cap * sizeof(tombstone));
-+ }
-+ arg->tombstones[arg->len++] = tombstone;
++ int err = reftable_stack_add(stack, &write_reflog_delete_table, &arg);
++ assert(err != REFTABLE_API_ERROR);
++ return err;
+}
+
++struct reflog_expiry_arg {
++ struct reftable_stack *stack;
++ struct reftable_log_record *records;
++ int len;
++};
++
+static int write_reflog_expiry_table(struct reftable_writer *writer, void *argv)
+{
+ struct reflog_expiry_arg *arg = (struct reflog_expiry_arg *)argv;
@@ refs/reftable-backend.c (new)
+ int i = 0;
+ reftable_writer_set_limits(writer, ts, ts);
+ for (i = 0; i < arg->len; i++) {
-+ int err = reftable_writer_add_log(writer, &arg->tombstones[i]);
++ int err = reftable_writer_add_log(writer, &arg->records[i]);
+ if (err) {
+ return err;
+ }
@@ refs/reftable-backend.c (new)
+ struct reftable_merged_table *mt = NULL;
+ struct reflog_expiry_arg arg = {
+ .stack = stack,
-+ .refs = refs,
+ };
-+ struct reftable_log_record log = { NULL };
++ struct reftable_log_record *logs = NULL;
++ struct reftable_log_record *rewritten = NULL;
++ int logs_len = 0;
++ int logs_cap = 0;
++ int i = 0;
++ uint8_t *last_hash = NULL;
+ struct reftable_iterator it = { NULL };
++ struct reftable_addition *add = NULL;
+ int err = 0;
+ if (refs->err < 0) {
+ return refs->err;
@@ refs/reftable-backend.c (new)
+ goto done;
+ }
+
++ err = reftable_stack_new_addition(&add, stack);
++ if (err) {
++ goto done;
++ }
++ prepare_fn(refname, oid, policy_cb_data);
+ while (1) {
-+ struct object_id ooid;
-+ struct object_id noid;
-+
++ struct reftable_log_record log = {NULL};
+ int err = reftable_iterator_next_log(&it, &log);
+ if (err < 0) {
+ goto done;
@@ refs/reftable-backend.c (new)
+ if (err > 0 || strcmp(log.refname, refname)) {
+ break;
+ }
-+ hashcpy(ooid.hash, log.update.old_hash);
-+ hashcpy(noid.hash, log.update.new_hash);
+
-+ if (should_prune_fn(&ooid, &noid, log.update.email,
-+ (timestamp_t)log.update.time,
-+ log.update.tz_offset, log.update.message,
++ if (logs_len >= logs_cap) {
++ int new_cap = logs_cap * 2 + 1;
++ logs = realloc(logs, new_cap * sizeof(*logs));
++ logs_cap = new_cap;
++ }
++ logs[logs_len++] = log;
++ }
++
++ rewritten = calloc(logs_len, sizeof(*rewritten));
++ for (i = logs_len-1; i >= 0; i--) {
++ struct object_id ooid;
++ struct object_id noid;
++ struct reftable_log_record *dest = &rewritten[i];
++
++ *dest = logs[i];
++ oidread(&ooid, logs[i].update.old_hash);
++ oidread(&noid, logs[i].update.new_hash);
++
++ if (should_prune_fn(&ooid, &noid, logs[i].update.email,
++ (timestamp_t)logs[i].update.time,
++ logs[i].update.tz_offset, logs[i].update.message,
+ policy_cb_data)) {
-+ add_log_tombstone(&arg, refname, log.update_index);
++ dest->value_type = REFTABLE_LOG_DELETION;
++ } else {
++ if ((flags & EXPIRE_REFLOGS_REWRITE) && last_hash != NULL) {
++ dest->update.old_hash = last_hash;
++ }
++ last_hash = logs[i].update.new_hash;
+ }
+ }
-+ err = reftable_stack_add(stack, &write_reflog_expiry_table, &arg);
++
++ arg.records = rewritten;
++ arg.len = logs_len;
++ err = reftable_addition_add(add, &write_reflog_expiry_table, &arg);
++ if (err < 0) {
++ goto done;
++ }
++
++ if (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {
++ /* XXX - skip writing records that were not changed. */
++ err = reftable_addition_commit(add);
++ } else {
++ /* XXX - print something */
++ }
+
+done:
++ if (add) {
++ cleanup_fn(policy_cb_data);
++ }
+ assert(err != REFTABLE_API_ERROR);
-+ reftable_log_record_release(&log);
++ reftable_addition_destroy(add);
++ for (i = 0; i < logs_len; i++)
++ reftable_log_record_release(&logs[i]);
++ free(logs);
++ free(rewritten);
+ reftable_iterator_destroy(&it);
-+ clear_log_tombstones(&arg);
+ return err;
+}
+
@@ refs/reftable-backend.c (new)
+ strbuf_addstr(referent, ref.value.symref);
+ *type |= REF_ISSYMREF;
+ } else if (reftable_ref_record_val1(&ref) != NULL) {
-+ hashcpy(oid->hash, reftable_ref_record_val1(&ref));
++ oidread(oid, reftable_ref_record_val1(&ref));
+ } else {
+ *type |= REF_ISBROKEN;
+ errno = EINVAL;
@@ setup.c: const char *setup_git_directory_gently(int *nongit_ok)
+ xstrdup_or_null(repo_fmt.ref_storage);
+ }
}
-
- strbuf_release(&dir);
+ /*
+ * Since precompose_string_if_needed() needs to look at
## t/t0031-reftable.sh (new) ##
@@
@@ t/t0031-reftable.sh (new)
+ mv .git/hooks .git/hooks-disabled
+}
+
++write_script fake_editor <<\EOF
++echo "$MSG" >"$1"
++echo "$MSG" >&2
++EOF
++GIT_EDITOR=./fake_editor
++export GIT_EDITOR
++
++test_expect_success 'read existing old OID if REF_HAVE_OLD is not set' '
++ initialize &&
++ test_commit 1st &&
++ test_commit 2nd &&
++ MSG=b4 git notes add &&
++ MSG=b3 git notes edit &&
++ echo b4 >expect &&
++ git notes --ref commits@{1} show >actual &&
++ test_cmp expect actual
++'
++
++test_expect_success 'git reflog delete' '
++ initialize &&
++ test_commit file &&
++ test_commit file2 &&
++ test_commit file3 &&
++ test_commit file4 &&
++ git reflog delete HEAD@{1} &&
++ git reflog > output &&
++ ! grep file3 output
++'
++
++test_expect_success 'branch -D delete nonexistent branch' '
++ initialize &&
++ test_commit file &&
++ test_must_fail git branch -D ../../my-private-file
++'
++
++test_expect_success 'branch copy' '
++ initialize &&
++ test_commit file1 &&
++ test_commit file2 &&
++ git branch src &&
++ git reflog src > expect &&
++ git branch -c src dst &&
++ git reflog dst | sed "s/dst/src/g" > actual &&
++ test_cmp expect actual
++'
++
++test_expect_success 'git stash' '
++ initialize &&
++ test_commit file &&
++ touch actual expected &&
++ git -c status.showStash=true status >expected &&
++ echo hoi >> file.t &&
++ git stash push -m stashed &&
++ git stash clear &&
++ git -c status.showStash=true status >actual &&
++ test_cmp expected actual
++'
++
++test_expect_success 'rename branch' '
++ initialize &&
++ git symbolic-ref HEAD refs/heads/before &&
++ test_commit file &&
++ git show-ref | sed s/before/after/g > expected &&
++ git branch -M after &&
++ git show-ref > actual &&
++ test_cmp expected actual
++'
++
+test_expect_success 'SHA256 support, env' '
+ rm -rf .git &&
+ GIT_DEFAULT_HASH=sha256 && export GIT_DEFAULT_HASH &&
19: b2fa6ea62c16 = 24: 742d716e4510 git-prompt: prepare for reftable refs backend
20: 39e644b9436b ! 25: aca82763e72c Add "test-tool dump-reftable" command.
@@ Commit message
Signed-off-by: Han-Wen Nienhuys <[email protected]>
## Makefile ##
-@@ Makefile: REFTABLE_OBJS += reftable/zlib-compat.o
+@@ Makefile: REFTABLE_OBJS += reftable/writer.o
REFTABLE_TEST_OBJS += reftable/basics_test.o
REFTABLE_TEST_OBJS += reftable/block_test.o
@@ Makefile: REFTABLE_OBJS += reftable/zlib-compat.o
REFTABLE_TEST_OBJS += reftable/pq_test.o
REFTABLE_TEST_OBJS += reftable/record_test.o
- ## reftable/dump.c ##
-@@ reftable/dump.c: license that can be found in the LICENSE file or at
- #include <unistd.h>
- #include <string.h>
-
--#include "reftable.h"
-+#include "reftable-blocksource.h"
-+#include "reftable-error.h"
-+#include "reftable-merged.h"
-+#include "reftable-record.h"
- #include "reftable-tests.h"
-+#include "reftable-writer.h"
-+#include "reftable-iterator.h"
-+#include "reftable-reader.h"
-+#include "reftable-stack.h"
-
- static uint32_t hash_id;
-
- static int dump_table(const char *tablename)
- {
-- struct reftable_block_source src = { 0 };
-+ struct reftable_block_source src = { NULL };
- int err = reftable_block_source_from_file(&src, tablename);
-- struct reftable_iterator it = { 0 };
-- struct reftable_ref_record ref = { 0 };
-- struct reftable_log_record log = { 0 };
-+ struct reftable_iterator it = { NULL };
-+ struct reftable_ref_record ref = { NULL };
-+ struct reftable_log_record log = { NULL };
- struct reftable_reader *r = NULL;
-
- if (err < 0)
-@@ reftable/dump.c: static int dump_table(const char *tablename)
- reftable_ref_record_print(&ref, hash_id);
- }
- reftable_iterator_destroy(&it);
-- reftable_ref_record_clear(&ref);
-+ reftable_ref_record_release(&ref);
-
- err = reftable_reader_seek_log(r, &it, "");
- if (err < 0) {
-@@ reftable/dump.c: static int dump_table(const char *tablename)
- reftable_log_record_print(&log, hash_id);
- }
- reftable_iterator_destroy(&it);
-- reftable_log_record_clear(&log);
-+ reftable_log_record_release(&log);
-
- reftable_reader_free(r);
- return 0;
-@@ reftable/dump.c: static int dump_table(const char *tablename)
- static int compact_stack(const char *stackdir)
- {
- struct reftable_stack *stack = NULL;
-- struct reftable_write_options cfg = {};
-+ struct reftable_write_options cfg = { 0 };
-
- int err = reftable_new_stack(&stack, stackdir, cfg);
- if (err < 0)
-@@ reftable/dump.c: static int compact_stack(const char *stackdir)
- static int dump_stack(const char *stackdir)
- {
- struct reftable_stack *stack = NULL;
-- struct reftable_write_options cfg = {};
-- struct reftable_iterator it = { 0 };
-- struct reftable_ref_record ref = { 0 };
-- struct reftable_log_record log = { 0 };
-+ struct reftable_write_options cfg = { 0 };
-+ struct reftable_iterator it = { NULL };
-+ struct reftable_ref_record ref = { NULL };
-+ struct reftable_log_record log = { NULL };
- struct reftable_merged_table *merged = NULL;
-
- int err = reftable_new_stack(&stack, stackdir, cfg);
-@@ reftable/dump.c: static int dump_stack(const char *stackdir)
- reftable_ref_record_print(&ref, hash_id);
- }
- reftable_iterator_destroy(&it);
-- reftable_ref_record_clear(&ref);
-+ reftable_ref_record_release(&ref);
-
- err = reftable_merged_table_seek_log(merged, &it, "");
- if (err < 0) {
-@@ reftable/dump.c: static int dump_stack(const char *stackdir)
- reftable_log_record_print(&log, hash_id);
- }
- reftable_iterator_destroy(&it);
-- reftable_log_record_clear(&log);
-+ reftable_log_record_release(&log);
-
- reftable_stack_destroy(stack);
- return 0;
-@@ reftable/dump.c: static void print_help(void)
- int reftable_dump_main(int argc, char *const *argv)
- {
- int err = 0;
-- int opt;
- int opt_dump_table = 0;
- int opt_dump_stack = 0;
- int opt_compact = 0;
-- const char *arg = NULL;
-- while ((opt = getopt(argc, argv, "2chts")) != -1) {
-- switch (opt) {
-- case '2':
-- hash_id = 0x73323536;
-+ const char *arg = NULL, *argv0 = argv[0];
-+
-+ for (; argc > 1; argv++, argc--)
-+ if (*argv[1] != '-')
- break;
-- case 't':
-+ else if (!strcmp("-2", argv[1]))
-+ hash_id = 0x73323536;
-+ else if (!strcmp("-t", argv[1]))
- opt_dump_table = 1;
-- break;
-- case 's':
-+ else if (!strcmp("-s", argv[1]))
- opt_dump_stack = 1;
-- break;
-- case 'c':
-+ else if (!strcmp("-c", argv[1]))
- opt_compact = 1;
-- break;
-- case '?':
-- case 'h':
-+ else if (!strcmp("-?", argv[1]) || !strcmp("-h", argv[1])) {
- print_help();
- return 2;
-- break;
- }
-- }
-
-- if (argv[optind] == NULL) {
-+ if (argc != 2) {
- fprintf(stderr, "need argument\n");
- print_help();
- return 2;
- }
-
-- arg = argv[optind];
-+ arg = argv[1];
-
- if (opt_dump_table) {
- err = dump_table(arg);
-@@ reftable/dump.c: int reftable_dump_main(int argc, char *const *argv)
- }
-
- if (err < 0) {
-- fprintf(stderr, "%s: %s: %s\n", argv[0], arg,
-+ fprintf(stderr, "%s: %s: %s\n", argv0, arg,
- reftable_error_str(err));
- return 1;
- }
-
## t/helper/test-reftable.c ##
@@ t/helper/test-reftable.c: int cmd__reftable(int argc, const char **argv)
tree_test_main(argc, argv);
-: ------------ > 26: ae88f41de9a9 t1301: document what needs to be done for REFTABLE
-: ------------ > 27: 0c1eb69900a7 t1401,t2011: parameterize HEAD.lock for REFTABLE
-: ------------ > 28: dd8ffbf53157 t1404: annotate test cases with REFFILESSignature · 1 line
gitgitgadget
PATCH v7 1/28 refs: ref_iterator_peel returns boolean, rather than peel_status
From: Han-Wen Nienhuys <[email protected]>
Before, the cached ref_iterator would return peel_object() output directly. This led to spurious differences in the GIT_TRACE_REFS output, depending on the ref storage backend active.
--- refs.c | 2 +- refs/ref-cache.c | 2 +- refs/refs-internal.h | 3 +++ 3 files changed, 5 insertions(+), 2 deletions(-)
refs.c +1 -1
@@ -2010,7 +2010,7 @@ int peel_iterated_oid(const struct object_id *base, struct object_id *peeled) oideq(current_ref_iter->oid, base)))return ref_iterator_peel (current_ref_iter, peeled); -return peel_object (base, peeled); +return !!peel_object (base, peeled); } int refs_create_symref(struct ref_store *refs,
refs/ref-cache.c +1 -1
@@ -491,7 +491,7 @@ static int cache_ref_iterator_advance(struct ref_iterator *ref_iterator)static int cache_ref_iterator_peel (struct ref_iterator *ref_iterator,struct object_id *peeled) { -return peel_object (ref_iterator->oid, peeled); +return !!peel_object (ref_iterator->oid, peeled); }static int cache_ref_iterator_abort (struct ref_iterator *ref_iterator)
refs/refs-internal.h +3 -0
@@ -453,6 +453,9 @@ void base_ref_iterator_free(struct ref_iterator *iter);*/ typedef int ref_iterator_advance_fn (struct ref_iterator *ref_iterator); +/* +* Peels the current ref, returning 0 for success. +*/ typedef int ref_iterator_peel_fn (struct ref_iterator *ref_iterator,struct object_id *peeled);
Signature · 1 line
gitgitgadget
Re: [PATCH v7 01/28] refs: ref_iterator_peel returns boolean, rather than peel_status
Quoted 6 lines · Han-Wen Nienhuys via GitGitGadget
From: Han-Wen Nienhuys <[email protected]>
Before, the cached
ref_iteratorwould returnpeel_object()output directly. This led to spurious differences in theGIT_TRACE_REFSoutput, depending on the ref storage backend active.
Quoted 6 lines
Signed-off-by Han-Wen Nienhuys <[email protected]>--- refs.c | 2 +- refs/ref-cache.c | 2 +- refs/refs-internal.h | 3 +++ 3 files changed, 5 insertions(+), 2 deletions(-)
A few observations.
* The ref_iterator_peel() is defined to "Return 0 on success." in refs/refs-internal.h and implication of the missing mention of what is returned on failure is that it can give any non-zero value back, and in this project a failure usually is signaled by returning a negative value.
* If the trace output cares that all non-success trace entries to look identical, I wonder if that layer should be the one that normalizes "0 is success, any other value is failure" into "0 is success, 1 is failure" (even though I find a positive 1 used as a failure a bit odd).
* refs.c::peel_object() is defined to return "enum peel_status" which is not even a simple yes/no.
PEEL_PEELED = 0
PEEL_INVALID = -1
PEEL_NON_TAG = -2
PEEL_IS_SYMREF = -3
PEEL_BROKEN = -4
The comment in refs/refs-internal.h about "Reference interators" suggests me that ref_iterator_peel() is supposed to be a moral equivalent of (recently gone) peel_ref() which was a thin wrapper to still surviving peel_object(), so I expect ref_iterator_peel() to allow the caller to differentiate various failure modes. And as a way to debug into a running system, I am not sure if losing the distinction in the trace output is even desirable.
packed_ref_iterator_peel() does use !!peel_object(), but I have a feeling that that is what should be fixed and not the other way around.
I haven't seen a good justification given to help convince me that this is a good change (and I presume it is, as I trust you or any other contributores enough not to knowingly make the system worse),
Quoted 3 lines
+/*
+ * Peels the current ref, returning 0 for success.
+ */
And if it does make sense to squash the peel status down to "bool", then the comment should mention the single acceptable value for failure, not just "0 for success" which implies "different negative values depending on the nature of failure".
Quoted 2 lines
typedef int ref_iterator_peel_fn(struct
ref_iterator*ref_iterator,
structobject_id*peeled);
Thanks.
Re: [PATCH v7 01/28] refs: ref_iterator_peel returns boolean, rather than peel_status
Quoted 24 lines · Junio C Hamano
Quoted 6 lines · Han-Wen Nienhuys via GitGitGadget
From: Han-Wen Nienhuys <[email protected]>
Before, the cached
ref_iteratorwould returnpeel_object()output directly. This led to spurious differences in theGIT_TRACE_REFSoutput, depending on the ref storage backend active.Quoted 6 lines
Signed-off-by Han-Wen Nienhuys <[email protected]>--- refs.c | 2 +- refs/ref-cache.c | 2 +- refs/refs-internal.h | 3 +++ 3 files changed, 5 insertions(+), 2 deletions(-)A few observations.
* The
ref_iterator_peel()is defined to "Return 0 on success." inrefs/refs-internal.hand implication of the missing mention of what is returned on failure is that it can give any non-zero value back, and in this project a failure usually is signaled by returning a negative value.
Quoted 5 lines
* If the trace output cares that all non-success trace entries to look identical, I wonder if that layer should be the one that normalizes "0 is success, any other value is failure" into "0 is success, 1 is failure" (even though I find a positive 1 used as a failure a bit odd).
No, it should not. The whole point of the trace output is to see where and how the files backend and the reftable backend return different results. By having the trace layer paper over differences, I will miss regressions of the reftable backend.
Quoted 17 lines
*
refs.c::peel_object()is defined to return "enumpeel_status" which is not even a simple yes/no.
PEEL_PEELED= 0
PEEL_INVALID= -1
PEEL_NON_TAG= -2
PEEL_IS_SYMREF= -3
PEEL_BROKEN= -4The comment in
refs/refs-internal.habout "Reference interators" suggests me thatref_iterator_peel()is supposed to be a moral equivalent of (recently gone)peel_ref()which was a thin wrapper to still survivingpeel_object(), so I expectref_iterator_peel()to allow the caller to differentiate various failure modes. And as a way to debug into a running system, I am not sure if losing the distinction in the trace output is even desirable.
Quoted 3 lines
packed_ref_iterator_peel()does use !!peel_object(), but I have a feeling that that is what should be fixed and not the other way around.
All these functions go into ref_iterator_peel, which is only called from peel_iterated_oid().
That function calls !!peel_object(base, peeled) if there is no iterator. That leads me to conclude
All callers of peel_iterated_oid use it as a boolean exclusively, i.e.
if (!peel_iterated_oid( .. )) { .. }
Aside from these considerations, it is also odd to return peel_status. For example, PEEL_IS_SYMREF is unnecessary, because the symref status is already returned from iterator_next().
Quoted 4 lines
I haven't seen a good justification given to help convince me that this is a good change (and I presume it is, as I trust you or any other contributores enough not to knowingly make the system worse),
Quoted 8 lines
Quoted 3 lines
+/*
+ * Peels the current ref, returning 0 for success.
+ */And if it does make sense to squash the peel status down to "bool", then the comment should mention the single acceptable value for failure, not just "0 for success" which implies "different negative values depending on the nature of failure".
I can make it return -1 instead.
Signature · 7 lines
Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Re: [PATCH v7 01/28] refs: ref_iterator_peel returns boolean, rather than peel_status
Quoted 11 lines · Han-Wen Nienhuys
All callers of
peel_iterated_oiduse it as a boolean exclusively, i.e.if (!peel_iterated_oid( .. )) { .. }
Aside from these considerations, it is also odd to return
peel_status. For example,PEEL_IS_SYMREFis unnecessary, because the symref status is already returned fromiterator_next().Quoted 3 lines
I haven't seen a good justification given to help convince me that this is a good change (and I presume it is, as I trust you or any other contributores enough not to knowingly make the system worse),
I am not sure I agree 100% with
if (!do_this()) {
...
}i.e. "try doing it and do one thing upon success" is a sign that all error conditions will ever be treated equally and justifies to squash all the error codes the current code tries to return, but whether I agree with it or not, I'd want to see it recorded in the proposed log message. It would help future developers and allow them to take into account your motivation behind this change, when they need to update the implementation.
Quoted 10 lines
Quoted 8 lines
Quoted 3 lines
+/*
+ * Peels the current ref, returning 0 for success.
+ */And if it does make sense to squash the peel status down to "bool", then the comment should mention the single acceptable value for failure, not just "0 for success" which implies "different negative values depending on the nature of failure".
I can make it return -1 instead.
I do care more about _documenting_ it here, not just ending the sentence with ", returning 0 for success", it should also say "returns X upon failure", whether the value of X is 1 or -1 (and obviously our codebase prefers -1 for an error, but that is secondary).
Thanks.
PATCH v7 2/28 refs: document reflog_expire_fn's flag argument
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
--- refs/refs-internal.h | 4 ++++ 1 file changed, 4 insertions(+)
refs/refs-internal.h +4 -0
@@ -592,6 +592,10 @@ typedef int reflog_exists_fn(struct ref_store *ref_store, const char *refname);typedef int create_reflog_fn (struct ref_store *ref_store,const char *refname,int force_create,struct strbuf *err);typedef int delete_reflog_fn (struct ref_store *ref_store,const char *refname); + +/* +* `flags` accepts a bitmask of `expire_reflog_flags`. +*/ typedef int reflog_expire_fn(struct ref_store *ref_store, const char *refname, const struct object_id *oid, unsigned int flags,
Signature · 1 line
gitgitgadget
Re: [PATCH v7 02/28] refs: document reflog_expire_fn's flag argument
Quoted 12 lines · Han-Wen Nienhuys via GitGitGadget
From: Han-Wen Nienhuys <[email protected]>
Signed-off-by Han-Wen Nienhuys <[email protected]>--- refs/refs-internal.h | 4 ++++ 1 file changed, 4 insertions(+)refs/refs-internal.h +0
-0
refs/refs-internal.h typedef int reflog_exists_fn(struct ref_store *ref_store, const char *refname); · line 598
Quoted 7 lines
refs/refs-internal.h +4
-0@@ -592,6 +592,10 @@ typedef int reflog_exists_fn(struct ref_store *ref_store, const char *refname);typedef int create_reflog_fn (struct ref_store *ref_store,const char *refname,int force_create,struct strbuf *err);typedef int delete_reflog_fn (struct ref_store *ref_store,const char *refname); + +/* +* `flags` accepts a bitmask of `expire_reflog_flags`. +*/
OK. It would have been better to say what expire_reflog_flags is (i.e. enum expire_reflog_flags), though.
Quoted 3 lines
refs/refs-internal.h +0
-0typedef int reflog_expire_fn(struct ref_store *ref_store, const char *refname, const struct object_id *oid, unsigned int flags,
Re: [PATCH v7 02/28] refs: document reflog_expire_fn's flag argument
On Tue, Apr 20, 2021 at 9:34 PM Junio C Hamano <[email protected]> wrote:> > +/*
Quoted 5 lines
Quoted 2 lines
+ *
flagsaccepts a bitmask ofexpire_reflog_flags.
+ */OK. It would have been better to say what
expire_reflog_flagsis (i.e.enum expire_reflog_flags), though.
Done, in the last version.
Signature · 11 lines
Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
PATCH v7 3/28 refs/debug: trace into reflog expiry too
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
--- refs/debug.c | 47 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-)
refs/debug.c +44 -3
@@ -353,6 +353,40 @@ static int debug_delete_reflog(struct ref_store *ref_store, const char *refname)return res; } +struct debug_reflog_expiry_should_prune { +reflog_expiry_prepare_fn *prepare; +reflog_expiry_should_prune_fn *should_prune; +reflog_expiry_cleanup_fn *cleanup; +void *cb_data; +}; + +static void debug_reflog_expiry_prepare (const char *refname, +const struct object_id *oid, +void *cb_data) +{ +struct debug_reflog_expiry_should_prune *prune = cb_data; +trace_printf_key (&trace_refs,"reflog_expire_prepare: %s\n" , refname); + prune->prepare (refname, oid, prune->cb_data); +} + +static int debug_reflog_expiry_should_prune_fn (struct object_id *ooid, +struct object_id *noid, +const char *email, +timestamp_t timestamp,int tz, +const char *message,void *cb_data) { +struct debug_reflog_expiry_should_prune *prune = cb_data; + +int result = prune->should_prune (ooid, noid, email, timestamp, tz, message, prune->cb_data); +trace_printf_key (&trace_refs,"reflog_expire_should_prune: %s %ld: %d\n" , message, (long int ) timestamp, result); +return result; +} + +static void debug_reflog_expiry_cleanup (void *cb_data) +{ +struct debug_reflog_expiry_should_prune *prune = cb_data; + prune->cleanup (prune->cb_data); +} + static int debug_reflog_expire(struct ref_store *ref_store, const char *refname, const struct object_id *oid, unsigned int flags, reflog_expiry_prepare_fn prepare_fn,
@@ -361,10 +395,17 @@ static int debug_reflog_expire(struct ref_store *ref_store, const char *refname, void *policy_cb_data) {struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store; +struct debug_reflog_expiry_should_prune prune = { + .prepare = prepare_fn, + .cleanup = cleanup_fn, + .should_prune = should_prune_fn, + .cb_data = policy_cb_data, + };int res = drefs->refs->be->reflog_expire (drefs->refs, refname, oid, - flags, prepare_fn, - should_prune_fn, cleanup_fn, - policy_cb_data); + flags, &debug_reflog_expiry_prepare, + &debug_reflog_expiry_should_prune_fn, + &debug_reflog_expiry_cleanup, + &prune);trace_printf_key (&trace_refs,"reflog_expire: %s: %d\n" , refname, res);return res; }
Signature · 1 line
gitgitgadget
Re: [PATCH v7 03/28] refs/debug: trace into reflog expiry too
Quoted 6 lines · Han-Wen Nienhuys via GitGitGadget
From: Han-Wen Nienhuys <[email protected]>
Signed-off-by Han-Wen Nienhuys <[email protected]>--- refs/debug.c | 47 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-)
Nicely done.
I as a reader of this patch do have to wonder, with the above very limited log message material, how useful did "debug_reflog_expire()" machinery used to be without any tracing.
It just reported the fact that expire method was called and what the backend did as a whole, instead of reporting what the machinery decided for each reflog entry to be pruned (or not pruned)?
Not a problem with this patch at all, and certainly it does not have to be part of this series, but it feels very backwards, at least to me, to have the method should_prune in ref backends. As a function to make a policy decision (e.g. "this has a timestamp older than X, so needs to be pruned", "the author of this change I do not like, so let's prune it ;-)"), it is more natural to have it as independent as possible from the individual backends, no?
Re: [PATCH v7 03/28] refs/debug: trace into reflog expiry too
Quoted 5 lines · Junio C Hamano
Nicely done.
I as a reader of this patch do have to wonder, with the above very limited log message material, how useful did "
debug_reflog_expire()" machinery used to be without any tracing.
It wasn't; that's why I'm changing it :-)
I noticed a test failure with reftable, and adding this function showed me I was expiring the entries in the reverse order, so that
git reflog delete branch@{1}
would remove one but oldest entry in the reflog.
Quoted 7 lines
Not a problem with this patch at all, and certainly it does not have to be part of this series, but it feels very backwards, at least to me, to have the method
should_prunein ref backends. As a function to make a policy decision (e.g. "this has a timestamp older than X, so needs to be pruned", "the author of this change I do not like, so let's prune it ;-)"), it is more natural to have it as independent as possible from the individual backends, no?
the should_prune function is passed in into the ref backend as an argument of type reflog_expiry_should_prune_fn to the refs_reflog_expire() function.
Signature · 7 lines
Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
PATCH v7 4/28 hash.h: provide constants for the hash IDs
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
This will simplify referencing them from code that is not deeply integrated with Git, in particular, the reftable library.
--- hash.h | 6 ++++++ object-file.c | 6 ++---- 2 files changed, 8 insertions(+), 4 deletions(-)
hash.h +6 -0
@@ -161,12 +161,18 @@ static inline int hash_algo_by_ptr(const struct git_hash_algo *p)return p - hash_algos; } +/* "sha1", big-endian */ +#define GIT_SHA1_HASH_ID 0x73686131 +/* The length in bytes and in hex digits of an object name (SHA-1 value). */ #define GIT_SHA1_RAWSZ 20#define GIT_SHA1_HEXSZ (2 * GIT_SHA1_RAWSZ)/* The block size of SHA-1. */ #define GIT_SHA1_BLKSZ 64 +/* "s256", big-endian */ +#define GIT_SHA256_HASH_ID 0x73323536 +/* The length in bytes and in hex digits of an object name (SHA-256 value). */ #define GIT_SHA256_RAWSZ 32#define GIT_SHA256_HEXSZ (2 * GIT_SHA256_RAWSZ)
object-file.c +2 -4
@@ -146,8 +146,7 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = { }, {"sha1" , -/* "sha1", big-endian */ -0x73686131 , +GIT_SHA1_HASH_ID ,GIT_SHA1_RAWSZ ,GIT_SHA1_HEXSZ , GIT_SHA1_BLKSZ,
@@ -160,8 +159,7 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = { }, {"sha256" , -/* "s256", big-endian */ -0x73323536 , +GIT_SHA256_HASH_ID ,GIT_SHA256_RAWSZ ,GIT_SHA256_HEXSZ , GIT_SHA256_BLKSZ,
Signature · 1 line
gitgitgadget
Re: [PATCH v7 04/28] hash.h: provide constants for the hash IDs
Quoted 20 lines · Han-Wen Nienhuys via GitGitGadget
From: Han-Wen Nienhuys <[email protected]>
This will simplify referencing them from code that is not deeply integrated with Git, in particular, the reftable library.
Signed-off-by Han-Wen Nienhuys <[email protected]>--- hash.h | 6 ++++++ object-file.c | 6 ++---- 2 files changed, 8 insertions(+), 4 deletions(-)hash.h +1
-0@@ -161,12 +161,18 @@ static inline int hash_algo_by_ptr(const struct git_hash_algo *p)return p - hash_algos; } +/* "sha1", big-endian */
hash.h static inline int hash_algo_by_ptr(const struct git_hash_algo *p) · line 174
Quoted 10 lines
hash.h +4
-0+#define GIT_SHA1_HASH_ID 0x73686131 +/* The length in bytes and in hex digits of an object name (SHA-1 value). */ #define GIT_SHA1_RAWSZ 20#define GIT_SHA1_HEXSZ (2 * GIT_SHA1_RAWSZ)/* The block size of SHA-1. */ #define GIT_SHA1_BLKSZ 64 +/* "s256", big-endian */ +#define GIT_SHA256_HASH_ID 0x73323536
I agree that it makes sense to have symbolic constants defined in this file. These are values that fit in the ".format_id" member of "struct git_hash_algo", and it is preferrable to make sure that the names align (if I were designing in void, I would have called the member "algo_id" and made the constants GIT_(SHA1|SHA256)_ALGO_ID).
Brian? What's your preference ("I am fine to store HASH_ID in the '.format_id' member" is perfectly an acceptable answer).
Thanks.
Re: [PATCH v7 04/28] hash.h: provide constants for the hash IDs
Quoted 39 lines · Junio C Hamano
Quoted 20 lines · Han-Wen Nienhuys via GitGitGadget
From: Han-Wen Nienhuys <[email protected]>
This will simplify referencing them from code that is not deeply integrated with Git, in particular, the reftable library.
Signed-off-by Han-Wen Nienhuys <[email protected]>--- hash.h | 6 ++++++ object-file.c | 6 ++---- 2 files changed, 8 insertions(+), 4 deletions(-)hash.h +1
-0@@ -161,12 +161,18 @@ static inline int hash_algo_by_ptr(const struct git_hash_algo *p)return p - hash_algos; } +/* "sha1", big-endian */ hash.h static inline int hash_algo_by_ptr(const struct git_hash_algo *p) · line 174
Quoted 10 lines
hash.h +4
-0+#define GIT_SHA1_HASH_ID 0x73686131 +/* The length in bytes and in hex digits of an object name (SHA-1 value). */ #define GIT_SHA1_RAWSZ 20#define GIT_SHA1_HEXSZ (2 * GIT_SHA1_RAWSZ)/* The block size of SHA-1. */ #define GIT_SHA1_BLKSZ 64 +/* "s256", big-endian */ +#define GIT_SHA256_HASH_ID 0x73323536I agree that it makes sense to have symbolic constants defined in this file. These are values that fit in the ".format_id" member of "struct
git_hash_algo", and it is preferrable to make sure that the names align (if I were designing in void, I would have called the member "algo_id" and made the constants GIT_(SHA1|SHA256)_ALGO_ID).
Quoted 2 lines
Brian? What's your preference ("I am fine to store
HASH_IDin the '.format_id' member" is perfectly an acceptable answer).
I slightly prefer FORMAT_ID because it's consistent (and for that reason only), but if HASH_ID is more convenient, that's fine; I don't have a strong opinion at all. Definitely don't reroll the series because of my slight preference. Either way, I think these are fine things to have as constants, and I appreciate you hoisting the comments here.
Signature · 2 lines
brian m. carlson (he/him or they/them) Houston, Texas, US
Re: [PATCH v7 04/28] hash.h: provide constants for the hash IDs
Quoted 6 lines · brian m. carlson
Quoted 6 lines
I agree that it makes sense to have symbolic constants defined in this file. These are values that fit in the ".format_id" member of "struct
git_hash_algo", and it is preferrable to make sure that the names align (if I were designing in void, I would have called the member "algo_id" and made the constants GIT_(SHA1|SHA256)_ALGO_ID).
Quoted 8 lines
Quoted 2 lines
Brian? What's your preference ("I am fine to store
HASH_IDin the '.format_id' member" is perfectly an acceptable answer).I slightly prefer
FORMAT_IDbecause it's consistent (and for that reason only), but ifHASH_IDis more convenient, that's fine; I don't have a strong opinion at all. Definitely don't reroll the series because of my slight preference. Either way, I think these are fine things to have as constants, and I appreciate you hoisting the comments here.
Either way is fine for me. I can paint the bikeshed in your favorite color :)
Signature · 11 lines
Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Re: [PATCH v7 04/28] hash.h: provide constants for the hash IDs
Quoted 12 lines · Han-Wen Nienhuys
Quoted 9 lines · brian m. carlson
Quoted 8 lines
I agree that it makes sense to have symbolic constants defined in this file. These are values that fit in the ".format_id" member of "struct
git_hash_algo", and it is preferrable to make sure that the names align (if I were designing in void, I would have called the member "algo_id" and made the constants GIT_(SHA1|SHA256)_ALGO_ID).Brian? What's your preference ("I am fine to store
HASH_IDin the '.format_id' member" is perfectly an acceptable answer).
Quoted 7 lines
Quoted 5 lines
I slightly prefer
FORMAT_IDbecause it's consistent (and for that reason only), but ifHASH_IDis more convenient, that's fine; I don't have a strong opinion at all. Definitely don't reroll the series because of my slight preference. Either way, I think these are fine things to have as constants, and I appreciate you hoisting the comments here.Either way is fine for me. I can paint the bikeshed in your favorite color :)
This is now the first commit of the reftable series. I think it could graduate separately.
Signature · 7 lines
Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
PATCH v7 5/28 init-db: set the_repository->hash_algo early on
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
The reftable backend needs to know the hash algorithm for writing the initialization hash table.
The initial reftable contains a symref HEAD => "main" (or "master"), which is agnostic to the size of hash value, but this is an exceptional circumstance, and the reftable library does not cater to this exception. It insists that all tables in the stack have a consistent format ID for the hash algorithm.
Call set_repo_hash_algo directly after calling validate_hash_algorithm() (which reads $GIT_DEFAULT_HASH).
--- builtin/init-db.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
builtin/init-db.c +21 -0
@@ -424,6 +424,27 @@ int init_db(const char *git_dir, const char *real_git_dir,validate_hash_algorithm (&repo_fmt, hash); +/* +* At this point, the_repository we have in-core does not look +* anything like one that we would see initialized in an already +* working repository after calling setup_git_directory(). +* +* Calling repository.c::initialize_the_repository() may have +* prepared the .index .objects and .parsed_objects members, but +* other members like .gitdir, .commondir, etc. have not been +* initialized. +* +* Many API functions assume they are working with the_repository +* that has sensibly been initialized, but because we haven't +* really read from an existing repository, we need to hand-craft +* the necessary members of the structure to get out of this +* chicken-and-egg situation. +* +* For now, we update the hash algorithm member to what the +* validate_hash_algorithm() call decided for us. +*/ +repo_set_hash_algo (the_repository, repo_fmt.hash_algo); + reinit =create_default_files (template_dir, original_git_dir, initial_branch, &repo_fmt, flags &INIT_DB_QUIET );
Signature · 1 line
gitgitgadget
PATCH v7 6/28 reftable: add LICENSE
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
TODO: relicense?
--- reftable/LICENSE | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 reftable/LICENSE
reftable/LICENSE new file +31 -0
@@ -0,0 +1,31 @@ +BSD License + +Copyright (c) 2020, Google LLC +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + +* Redistributions of source code must retain the above copyright notice, +this list of conditions and the following disclaimer. + +* Redistributions in binary form must reproduce the above copyright +notice, this list of conditions and the following disclaimer in the +documentation and/or other materials provided with the distribution. + +* Neither the name of Google LLC nor the names of its contributors may +be used to endorse or promote products derived from this software +without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
Signature · 1 line
gitgitgadget
Re: [PATCH v7 06/28] reftable: add LICENSE
Quoted 3 lines · Han-Wen Nienhuys via GitGitGadget
From: Han-Wen Nienhuys <[email protected]>
TODO: relicense?
So, still no resolution to the questions in v6 in [1]?
Re: [PATCH v7 06/28] reftable: add LICENSE
Quoted 9 lines · Ævar Arnfjörð Bjarmason
Quoted 3 lines · Han-Wen Nienhuys via GitGitGadget
From: Han-Wen Nienhuys <[email protected]>
TODO: relicense?
So, still no resolution to the questions in v6 in [1]?
I reworded the commit message, but that update was lost in one of the copious rebase operations. I recovered it from reflog, and changed it. Hopefully it survives this again.
Signature · 11 lines
Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
PATCH v7 7/28 reftable: add error related functionality
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
The reftable/ directory is structured as a library, so it cannot crash on misuse. Instead, it returns an error codes.
In addition, the error code can be used to signal conditions from lower levels of the library to be handled by higher levels of the library. For example, a transaction might legitimately write an empty reftable file, but in that case, we'd want to shortcut the transaction overhead.
--- reftable/error.c | 41 ++++++++++++++++++++++++++ reftable/reftable-error.h | 62 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 reftable/error.c create mode 100644 reftable/reftable-error.h
reftable/error.c new file +41 -0
@@ -0,0 +1,41 @@ +/* +Copyright 2020 Google LLC + +Use of this source code is governed by a BSD-style +license that can be found in the LICENSE file or at +https://developers.google.com/open-source/licenses/bsd +*/ + +#include "reftable-error.h" + +#include <stdio.h> + +const char *reftable_error_str (int err) +{ +static char buf[250 ]; +switch (err) { +case REFTABLE_IO_ERROR : +return "I/O error" ; +case REFTABLE_FORMAT_ERROR : +return "corrupt reftable file" ; +case REFTABLE_NOT_EXIST_ERROR : +return "file does not exist" ; +case REFTABLE_LOCK_ERROR : +return "data is outdated" ; +case REFTABLE_API_ERROR : +return "misuse of the reftable API" ; +case REFTABLE_ZLIB_ERROR : +return "zlib failure" ; +case REFTABLE_NAME_CONFLICT : +return "file/directory conflict" ; +case REFTABLE_EMPTY_TABLE_ERROR : +return "wrote empty table" ; +case REFTABLE_REFNAME_ERROR : +return "invalid refname" ; +case -1 : +return "general error" ; +default : +snprintf (buf,sizeof (buf),"unknown error code %d" , err); +return buf; + } +}
reftable/reftable-error.h new file +62 -0
62 lines not on this page · raw message
Signature · 1 line
gitgitgadget
PATCH v7 8/28 reftable: utility functions
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
This commit provides basic utility classes for the reftable library.
--- Makefile | 25 +++++- contrib/buildsystems/CMakeLists.txt | 14 ++- reftable/basics.c | 128 ++++++++++++++++++++++++++++ reftable/basics.h | 60 +++++++++++++ reftable/basics_test.c | 98 +++++++++++++++++++++ reftable/publicbasics.c | 58 +++++++++++++ reftable/reftable-malloc.h | 18 ++++ reftable/reftable-tests.h | 22 +++++ reftable/system.h | 24 ++++++ reftable/test_framework.c | 23 +++++ reftable/test_framework.h | 53 ++++++++++++ t/helper/test-reftable.c | 9 ++ t/helper/test-tool.c | 3 +- t/helper/test-tool.h | 1 + t/t0032-reftable-unittest.sh | 15 ++++ 15 files changed, 545 insertions(+), 6 deletions(-) create mode 100644 reftable/basics.c create mode 100644 reftable/basics.h create mode 100644 reftable/basics_test.c create mode 100644 reftable/publicbasics.c create mode 100644 reftable/reftable-malloc.h create mode 100644 reftable/reftable-tests.h create mode 100644 reftable/system.h create mode 100644 reftable/test_framework.c create mode 100644 reftable/test_framework.h create mode 100644 t/helper/test-reftable.c create mode 100755 t/t0032-reftable-unittest.sh
Makefile +22 -3
@@ -736,6 +736,7 @@ TEST_BUILTINS_OBJS += test-read-cache.o TEST_BUILTINS_OBJS += test-read-graph.o TEST_BUILTINS_OBJS += test-read-midx.o TEST_BUILTINS_OBJS += test-ref-store.o +TEST_BUILTINS_OBJS += test-reftable.o TEST_BUILTINS_OBJS += test-regex.o TEST_BUILTINS_OBJS += test-repository.o TEST_BUILTINS_OBJS += test-revision-walking.o
@@ -813,6 +814,8 @@ TEST_SHELL_PATH = $(SHELL_PATH) LIB_FILE = libgit.a XDIFF_LIB = xdiff/lib.a +REFTABLE_LIB = reftable/libreftable.a +REFTABLE_TEST_LIB = reftable/libreftable_test.a GENERATED_H += command-list.h GENERATED_H += config-list.h
@@ -1181,7 +1184,7 @@ THIRD_PARTY_SOURCES += compat/regex/% THIRD_PARTY_SOURCES += sha1collisiondetection/% THIRD_PARTY_SOURCES += sha1dc/% -GITLIBS = common-main.o $(LIB_FILE) $(XDIFF_LIB) +GITLIBS = common-main.o $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB) EXTLIBS = GIT_USER_AGENT = git/$(GIT_VERSION)
@@ -2398,7 +2401,15 @@ XDIFF_OBJS += xdiff/xutils.o .PHONY: xdiff-objs xdiff-objs: $(XDIFF_OBJS) +REFTABLE_OBJS += reftable/basics.o +REFTABLE_OBJS += reftable/error.o +REFTABLE_OBJS += reftable/publicbasics.o + +REFTABLE_TEST_OBJS += reftable/test_framework.o +REFTABLE_TEST_OBJS += reftable/basics_test.o + TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) + .PHONY: test-objs test-objs: $(TEST_OBJS)
@@ -2414,6 +2425,8 @@ OBJECTS += $(PROGRAM_OBJS) OBJECTS += $(TEST_OBJS) OBJECTS += $(XDIFF_OBJS) OBJECTS += $(FUZZ_OBJS) +OBJECTS += $(REFTABLE_OBJS) $(REFTABLE_TEST_OBJS) + ifndef NO_CURL OBJECTS += http.o http-walker.o remote-curl.o endif
@@ -2565,6 +2578,12 @@ $(LIB_FILE): $(LIB_OBJS) $(XDIFF_LIB): $(XDIFF_OBJS) $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^ +$(REFTABLE_LIB): $(REFTABLE_OBJS) + $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^ + +$(REFTABLE_TEST_LIB): $(REFTABLE_TEST_OBJS) + $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^ + export DEFAULT_EDITOR DEFAULT_PAGER Documentation/GIT-EXCLUDED-PROGRAMS: FORCE
@@ -2845,7 +2864,7 @@ perf: all t/helper/test-tool$X: $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS)) -t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS) +t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS) $(REFTABLE_TEST_LIB) $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS) check-sha1:: t/helper/test-tool$X
@@ -3175,7 +3194,7 @@ cocciclean: clean: profile-clean coverage-clean cocciclean $(RM) *.res $(RM) $(OBJECTS) - $(RM) $(LIB_FILE) $(XDIFF_LIB) + $(RM) $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB) $(REFTABLE_TEST_LIB) $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X $(RM) $(TEST_PROGRAMS) $(RM) $(FUZZ_PROGRAMS)
contrib/buildsystems/CMakeLists.txt +12 -2
@@ -611,6 +611,12 @@ parse_makefile_for_sources(libxdiff_SOURCES "XDIFF_OBJS") list(TRANSFORM libxdiff_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/") add_library(xdiff STATIC ${libxdiff_SOURCES}) +#reftable +parse_makefile_for_sources(reftable_SOURCES "REFTABLE_OBJS") + +list(TRANSFORM reftable_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/") +add_library(reftable STATIC ${reftable_SOURCES}) + if(WIN32) if(NOT MSVC)#use windres when compiling with gcc and clang add_custom_command(OUTPUT ${CMAKE_BINARY_DIR}/git.res
@@ -633,7 +639,7 @@ endif() #link all required libraries to common-main add_library(common-main OBJECT ${CMAKE_SOURCE_DIR}/common-main.c) -target_link_libraries(common-main libgit xdiff ${ZLIB_LIBRARIES}) +target_link_libraries(common-main libgit xdiff reftable ${ZLIB_LIBRARIES}) if(Intl_FOUND) target_link_libraries(common-main ${Intl_LIBRARIES}) endif()
@@ -873,11 +879,15 @@ if(BUILD_TESTING) add_executable(test-fake-ssh ${CMAKE_SOURCE_DIR}/t/helper/test-fake-ssh.c) target_link_libraries(test-fake-ssh common-main) +#reftable-tests +parse_makefile_for_sources(test-reftable_SOURCES "REFTABLE_TEST_OBJS") +list(TRANSFORM test-reftable_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/") + #test-tool parse_makefile_for_sources(test-tool_SOURCES "TEST_BUILTINS_OBJS") list(TRANSFORM test-tool_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/t/helper/") -add_executable(test-tool ${CMAKE_SOURCE_DIR}/t/helper/test-tool.c ${test-tool_SOURCES}) +add_executable(test-tool ${CMAKE_SOURCE_DIR}/t/helper/test-tool.c ${test-tool_SOURCES} ${test-reftable_SOURCES}) target_link_libraries(test-tool common-main) set_target_properties(test-fake-ssh test-tool
reftable/basics.c new file +128 -0
128 lines not on this page · raw message
reftable/basics.h new file +60 -0
60 lines not on this page · raw message
reftable/basics_test.c new file +98 -0
98 lines not on this page · raw message
reftable/publicbasics.c new file +58 -0
58 lines not on this page · raw message
reftable/reftable-malloc.h new file +18 -0
@@ -0,0 +1,18 @@ +/* +Copyright 2020 Google LLC + +Use of this source code is governed by a BSD-style +license that can be found in the LICENSE file or at +https://developers.google.com/open-source/licenses/bsd +*/ + +#ifndef REFTABLE_H +#define REFTABLE_H + +#include <stddef.h> + +/* Overrides the functions to use for memory management. */ +void reftable_set_alloc (void *(*malloc)(size_t ), +void *(*realloc)(void *,size_t ),void (*free)(void *)); + +#endif
reftable/reftable-tests.h new file +22 -0
@@ -0,0 +1,22 @@ +/* +Copyright 2020 Google LLC + +Use of this source code is governed by a BSD-style +license that can be found in the LICENSE file or at +https://developers.google.com/open-source/licenses/bsd +*/ + +#ifndef REFTABLE_TESTS_H +#define REFTABLE_TESTS_H + +int basics_test_main (int argc,const char **argv); +int block_test_main (int argc,const char **argv); +int merged_test_main (int argc,const char **argv); +int record_test_main (int argc,const char **argv); +int refname_test_main (int argc,const char **argv); +int reftable_test_main (int argc,const char **argv); +int stack_test_main (int argc,const char **argv); +int tree_test_main (int argc,const char **argv); +int reftable_dump_main (int argc,char *const *argv); + +#endif
reftable/system.h new file +24 -0
@@ -0,0 +1,24 @@ +/* +Copyright 2020 Google LLC + +Use of this source code is governed by a BSD-style +license that can be found in the LICENSE file or at +https://developers.google.com/open-source/licenses/bsd +*/ + +#ifndef SYSTEM_H +#define SYSTEM_H + +// This header glues the reftable library to the rest of Git + +#include "git-compat-util.h" +#include "strbuf.h" +#include "hash.h" /* hash ID, sizes.*/ +#include "dir.h" /* remove_dir_recursively, for tests.*/ + +#include <zlib.h> + +struct strbuf ; +int hash_size (uint32_t id); + +#endif
reftable/test_framework.c new file +23 -0
@@ -0,0 +1,23 @@ +/* +Copyright 2020 Google LLC + +Use of this source code is governed by a BSD-style +license that can be found in the LICENSE file or at +https://developers.google.com/open-source/licenses/bsd +*/ + +#include "system.h" +#include "test_framework.h" + +#include "basics.h" + +void set_test_hash (uint8_t *p,int i) +{ +memset (p, (uint8_t )i,hash_size (GIT_SHA1_HASH_ID )); +} + +ssize_t strbuf_add_void (void *b,const void *data,size_t sz) +{ +strbuf_add (b, data, sz); +return sz; +}
reftable/test_framework.h new file +53 -0
53 lines not on this page · raw message
t/helper/test-reftable.c new file +9 -0
@@ -0,0 +1,9 @@ +#include "reftable/reftable-tests.h" +#include "test-tool.h" + +int cmd__reftable (int argc,const char **argv) +{ +basics_test_main (argc, argv); + +return 0 ; +}
t/helper/test-tool.c +2 -1
@@ -50,13 +50,14 @@ static struct test_cmd cmds[] = { { "pcre2-config", cmd__pcre2_config }, { "pkt-line", cmd__pkt_line }, { "prio-queue", cmd__prio_queue }, - { "proc-receive", cmd__proc_receive}, + { "proc-receive", cmd__proc_receive }, { "progress", cmd__progress }, { "reach", cmd__reach }, { "read-cache", cmd__read_cache }, { "read-graph", cmd__read_graph }, { "read-midx", cmd__read_midx }, { "ref-store", cmd__ref_store }, + { "reftable", cmd__reftable }, { "regex", cmd__regex }, { "repository", cmd__repository }, { "revision-walking", cmd__revision_walking },
t/helper/test-tool.h +1 -0
@@ -46,6 +46,7 @@ int cmd__read_cache(int argc, const char **argv);int cmd__read_graph (int argc,const char **argv);int cmd__read_midx (int argc,const char **argv);int cmd__ref_store (int argc,const char **argv); +int cmd__reftable (int argc,const char **argv);int cmd__regex (int argc,const char **argv);int cmd__repository (int argc,const char **argv);int cmd__revision_walking (int argc,const char **argv);
t/t0032-reftable-unittest.sh new file +15 -0
@@ -0,0 +1,15 @@ +#!/bin/sh +# +# Copyright (c) 2020 Google LLC +# + +test_description='reftable unittests' + +. ./test-lib.sh + +test_expect_success 'unittests' ' + TMPDIR=$(pwd) && export TMPDIR && + test-tool reftable +' + +test_done
Signature · 1 line
gitgitgadget
PATCH v7 9/28 reftable: add blocksource, an abstraction for random access reads
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
The reftable format is usually used with files for storage. However, we abstract away this using the blocksource data structure. This has two advantages:
* log blocks are zlib compressed, and handling them is simplified if we can discard byte segments from within the block layer.
* for unittests, it is useful to read and write in-memory. The blocksource allows us to abstract the data away from on-disk files.
--- Makefile | 1 + reftable/blocksource.c | 148 ++++++++++++++++++++++++++++++++ reftable/blocksource.h | 22 +++++ reftable/reftable-blocksource.h | 49 +++++++++++ 4 files changed, 220 insertions(+) create mode 100644 reftable/blocksource.c create mode 100644 reftable/blocksource.h create mode 100644 reftable/reftable-blocksource.h
Makefile +1 -0
@@ -2403,6 +2403,7 @@ xdiff-objs: $(XDIFF_OBJS) REFTABLE_OBJS += reftable/basics.o REFTABLE_OBJS += reftable/error.o +REFTABLE_OBJS += reftable/blocksource.o REFTABLE_OBJS += reftable/publicbasics.o REFTABLE_TEST_OBJS += reftable/test_framework.o
reftable/blocksource.c new file +148 -0
148 lines not on this page · raw message
reftable/blocksource.h new file +22 -0
@@ -0,0 +1,22 @@ +/* +Copyright 2020 Google LLC + +Use of this source code is governed by a BSD-style +license that can be found in the LICENSE file or at +https://developers.google.com/open-source/licenses/bsd +*/ + +#ifndef BLOCKSOURCE_H +#define BLOCKSOURCE_H + +#include "system.h" + +struct reftable_block_source ; + +/* Create an in-memory block source for reading reftables */ +void block_source_from_strbuf (struct reftable_block_source *bs, +struct strbuf *buf); + +struct reftable_block_source malloc_block_source (void ); + +#endif
reftable/reftable-blocksource.h new file +49 -0
49 lines not on this page · raw message
Signature · 1 line
gitgitgadget
PATCH v7 10/28 reftable: (de)serialization for the polymorphic record type.
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
The reftable format is structured as a sequence of blocks, and each block contains a sequence of prefix-compressed key-value records. There are 4 types of records, and they have similarities in how they must be handled. This is achieved by introducing a polymorphic 'record' type that encapsulates ref, log, index and object records.
--- Makefile | 2 + reftable/constants.h | 21 + reftable/record.c | 1202 ++++++++++++++++++++++++++++++++++++ reftable/record.h | 139 +++++ reftable/record_test.c | 407 ++++++++++++ reftable/reftable-record.h | 114 ++++ t/helper/test-reftable.c | 2 +- 7 files changed, 1886 insertions(+), 1 deletion(-) create mode 100644 reftable/constants.h create mode 100644 reftable/record.c create mode 100644 reftable/record.h create mode 100644 reftable/record_test.c create mode 100644 reftable/reftable-record.h
Makefile +2 -0
@@ -2405,7 +2405,9 @@ REFTABLE_OBJS += reftable/basics.o REFTABLE_OBJS += reftable/error.o REFTABLE_OBJS += reftable/blocksource.o REFTABLE_OBJS += reftable/publicbasics.o +REFTABLE_OBJS += reftable/record.o +REFTABLE_TEST_OBJS += reftable/record_test.o REFTABLE_TEST_OBJS += reftable/test_framework.o REFTABLE_TEST_OBJS += reftable/basics_test.o
reftable/constants.h new file +21 -0
@@ -0,0 +1,21 @@ +/* +Copyright 2020 Google LLC + +Use of this source code is governed by a BSD-style +license that can be found in the LICENSE file or at +https://developers.google.com/open-source/licenses/bsd +*/ + +#ifndef CONSTANTS_H +#define CONSTANTS_H + +#define BLOCK_TYPE_LOG 'g' +#define BLOCK_TYPE_INDEX 'i' +#define BLOCK_TYPE_REF 'r' +#define BLOCK_TYPE_OBJ 'o' +#define BLOCK_TYPE_ANY 0 + +#define MAX_RESTARTS ((1 << 16) - 1) +#define DEFAULT_BLOCK_SIZE 4096 + +#endif
reftable/record.c new file +1202 -0
1,202 lines not on this page · raw message
reftable/record.h new file +139 -0
139 lines not on this page · raw message
reftable/record_test.c new file +407 -0
407 lines not on this page · raw message
reftable/reftable-record.h new file +114 -0
114 lines not on this page · raw message
t/helper/test-reftable.c +1 -1
@@ -4,6 +4,6 @@int cmd__reftable (int argc,const char **argv) {basics_test_main (argc, argv); - +record_test_main (argc, argv);return 0 ; }
Signature · 1 line
gitgitgadget
Re: [PATCH v7 10/28] reftable: (de)serialization for the polymorphic record type.
Quoted 1 line · Han-Wen Nienhuys via GitGitGadget
From: Han-Wen Nienhuys <[email protected]>
[...snip...]
reftable/record.c[...snip...> +static void reftable_obj_record_copy_from(void *rec, const
Quoted 5 lines
reftable/record.c[...snip...> +static void reftable_obj_record_copy_from(void *rec, const new file +0
-0
void *src_rec,
Quoted 6 lines
reftable/record.c[...snip...> +static void reftable_obj_record_copy_from(void *rec, const new file +6
-0+ int hash_size) +{ + struct reftable_obj_record *obj = rec; + const struct reftable_obj_record *src = + (const struct reftable_obj_record *)src_rec; + int olen;
reftable/record.c[...snip...> +static void reftable_obj_record_copy_from(void *rec, const
Quoted 10 lines
reftable/record.c[...snip...> +static void reftable_obj_record_copy_from(void *rec, const new file +10
-0+ + reftable_obj_record_release(obj); + *obj = *src; + obj->hash_prefix = reftable_malloc(obj->hash_prefix_len); + memcpy(obj->hash_prefix, src->hash_prefix, obj->hash_prefix_len); + + olen = obj->offset_len * sizeof(uint64_t); + obj->offsets = reftable_malloc(olen); + memcpy(obj->offsets, src->offsets, olen); +}
If src->offset_len is 0 then offsets will be NULL - and passing NULL into memcpy() results in undefined behaviour. I think we should either add an "if (src->offset_len)" check around the memcpy, or perhaps switch to COPY_ARRAY() which performs that check for us. (We can probably also skip the malloc and hence also olen calculation in this scenario though, because obj->offsets should already be NULL if src->offsets was NULL?)
Specifically, if I run t0032 as such:
make CC=clang-11 UBSAN_OPTIONS=print_stacktrace=1 SANITIZE=undefined
COPTS="-Og -g" GIT_TEST_OPTS=-v T=t0032-reftable-unittest.sh test
UBSAN reports the following error:
reftable/record.c:475:23: runtime error: null pointer passed as argument 2, which is declared to never be null /usr/include/string.h:43:28: note: nonnull attribute specified here
#0 0x698e52 in reftable_obj_record_copy_from record.c:475:2
#1 0x6ac806 in test_copy record_test.c:21:2
#2 0x6abb73 in test_reftable_obj_record_roundtrip record_test.c:334:3
#3 0x6abb73 in record_test_main record_test.c:403:2
#4 0x437f08 in cmd__reftable test-reftable.c:10:2
#5 0x427e74 in cmd_main test-tool.c:126:11
#6 0x428043 in main common-main.c:52:11
#7 0x7f3cd1044349 in __libc_start_main (/lib64/libc.so.6+0x24349)
#8 0x407209 in _start start.S:120SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior reftable/record.c:475:23 in
[...snip...]
Quoted 17 lines
reftable/record.c[...snip...> +static void reftable_obj_record_copy_from(void *rec, const new file +17
-0+static int reftable_obj_record_decode(void *rec, struct strbuf key, + uint8_t val_type, struct string_view in, + int hash_size) +{ + struct string_view start = in; + struct reftable_obj_record *r = rec; + uint64_t count = val_type; + int n = 0; + uint64_t last; + int j; + r->hash_prefix = reftable_malloc(key.len); + memcpy(r->hash_prefix, key.buf, key.len); + r->hash_prefix_len = key.len; + + if (val_type == 0) { + n = get_var_int(&count, &in); + if (n < 0) {
reftable/record.c[...snip...> +static void reftable_obj_record_copy_from(void *rec, const
Quoted 10 lines
reftable/record.c[...snip...> +static void reftable_obj_record_copy_from(void *rec, const new file +10
-0+ return n; + } + + string_view_consume(&in, n); + } + + r->offsets = NULL; + r->offset_len = 0; + if (count == 0) + return start.len - in.len;
And it looks like creating a reftable_obj_record with offsets = NULL and offset_len = 0 is considered valid based on the code above.
[... snipped the rest ...]
Re: [PATCH v7 10/28] reftable: (de)serialization for the polymorphic record type.
Quoted 6 lines · Andrzej Hunt
If
src->offset_lenis 0 then offsets will be NULL - and passing NULL intomemcpy()results in undefined behaviour. I think we should either add an "if (src->offset_len)" check around the memcpy, or perhaps switch toCOPY_ARRAY()which performs that check for us. (We can probably also skip the malloc and hence also olen calculation in this scenario though, becauseobj->offsetsshould already be NULL ifsrc->offsetswas NULL?)
Thanks for the detailed report. I've fixed this for the next round.
Signature · 11 lines
Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
PATCH v7 11/28 Provide zlib's uncompress2 from compat/zlib-compat.c
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
This will be needed for reading reflog blocks in reftable.
--- Makefile | 7 +++ compat/.gitattributes | 1 + compat/zlib-uncompress2.c | 92 +++++++++++++++++++++++++++++++++++++++ configure.ac | 13 ++++++ git-compat-util.h | 4 ++ 5 files changed, 117 insertions(+) create mode 100644 compat/.gitattributes create mode 100644 compat/zlib-uncompress2.c
Makefile +7 -0
@@ -256,6 +256,8 @@ all:: # # Define NO_DEFLATE_BOUND if your zlib does not have deflateBound. # +# Define NO_UNCOMPRESS2 if your zlib does not have uncompress2. +# # Define NO_NORETURN if using buggy versions of gcc 4.6+ and profile feedback, # as the compiler can crash (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49299) #
@@ -1706,6 +1708,11 @@ ifdef NO_DEFLATE_BOUND BASIC_CFLAGS += -DNO_DEFLATE_BOUND endif +ifdef NO_UNCOMPRESS2 + BASIC_CFLAGS += -DNO_UNCOMPRESS2 + LIB_OBJS += compat/zlib-uncompress2.o +endif + ifdef NO_POSIX_GOODIES BASIC_CFLAGS += -DNO_POSIX_GOODIES endif
compat/.gitattributes new file +1 -0
@@ -0,0 +1 @@ +/zlib-uncompress2.c whitespace=-indent-with-non-tab,-trailing-space
compat/zlib-uncompress2.c new file +92 -0
92 lines not on this page · raw message
configure.ac +13 -0
@@ -672,9 +672,22 @@ AC_LINK_IFELSE([ZLIBTEST_SRC], NO_DEFLATE_BOUND=yes]) LIBS="$old_LIBS" +AC_DEFUN([ZLIBTEST_UNCOMPRESS2_SRC], [ +AC_LANG_PROGRAM([#include <zlib.h>], + [uncompress2(NULL,NULL,NULL,NULL);])]) +AC_MSG_CHECKING([for uncompress2 in -lz]) +old_LIBS="$LIBS" +LIBS="$LIBS -lz" +AC_LINK_IFELSE([ZLIBTEST_UNCOMPRESS2_SRC], + [AC_MSG_RESULT([yes])], + [AC_MSG_RESULT([no]) + NO_UNCOMPRESS2=yes]) +LIBS="$old_LIBS" + GIT_UNSTASH_FLAGS($ZLIB_PATH) GIT_CONF_SUBST([NO_DEFLATE_BOUND]) +GIT_CONF_SUBST([NO_UNCOMPRESS2]) # # Define NEEDS_SOCKET if linking with libc is not enough (SunOS,
git-compat-util.h +4 -0
@@ -1382,4 +1382,8 @@ static inline void *container_of_or_null_offset(void *ptr, size_t offset)void sleep_millisec (int millisec); +#ifdef NO_UNCOMPRESS2 + +#endif +#endif
Signature · 1 line
gitgitgadget
PATCH v7 12/28 reftable: reading/writing blocks
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
The reftable format is structured as a sequence of block. Within a block, records are prefix compressed, with an index of offsets for fully expand keys to enable binary search within blocks.
This commit provides the logic to read and write these blocks.
--- Makefile | 2 + reftable/block.c | 446 +++++++++++++++++++++++++++++++++++++++ reftable/block.h | 127 +++++++++++ reftable/block_test.c | 121 +++++++++++ t/helper/test-reftable.c | 1 + 5 files changed, 697 insertions(+) create mode 100644 reftable/block.c create mode 100644 reftable/block.h create mode 100644 reftable/block_test.c
Makefile +2 -0
@@ -2410,10 +2410,12 @@ xdiff-objs: $(XDIFF_OBJS) REFTABLE_OBJS += reftable/basics.o REFTABLE_OBJS += reftable/error.o +REFTABLE_OBJS += reftable/block.o REFTABLE_OBJS += reftable/blocksource.o REFTABLE_OBJS += reftable/publicbasics.o REFTABLE_OBJS += reftable/record.o +REFTABLE_TEST_OBJS += reftable/block_test.o REFTABLE_TEST_OBJS += reftable/record_test.o REFTABLE_TEST_OBJS += reftable/test_framework.o REFTABLE_TEST_OBJS += reftable/basics_test.o
reftable/block.c new file +446 -0
446 lines not on this page · raw message
reftable/block.h new file +127 -0
127 lines not on this page · raw message
reftable/block_test.c new file +121 -0
121 lines not on this page · raw message
t/helper/test-reftable.c +1 -0
@@ -4,6 +4,7 @@int cmd__reftable (int argc,const char **argv) {basics_test_main (argc, argv); +block_test_main (argc, argv);record_test_main (argc, argv);return 0 ; }
Signature · 1 line
gitgitgadget
PATCH v7 13/28 reftable: a generic binary tree implementation
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
The reftable format includes support for an (OID => ref) map. This map can speed up visibility and reachability checks. In particular, various operations along the fetch/push path within Gerrit have ben sped up by using this structure.
The map is constructed with help of a binary tree. Object IDs are hashes, so they are uniformly distributed. Hence, the tree does not attempt forced rebalancing.
--- Makefile | 4 ++- reftable/tree.c | 63 ++++++++++++++++++++++++++++++++++++++++ reftable/tree.h | 34 ++++++++++++++++++++++ reftable/tree_test.c | 61 ++++++++++++++++++++++++++++++++++++++ t/helper/test-reftable.c | 1 + 5 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 reftable/tree.c create mode 100644 reftable/tree.h create mode 100644 reftable/tree_test.c
Makefile +3 -1
@@ -2414,11 +2414,13 @@ REFTABLE_OBJS += reftable/block.o REFTABLE_OBJS += reftable/blocksource.o REFTABLE_OBJS += reftable/publicbasics.o REFTABLE_OBJS += reftable/record.o +REFTABLE_OBJS += reftable/tree.o +REFTABLE_TEST_OBJS += reftable/basics_test.o REFTABLE_TEST_OBJS += reftable/block_test.o REFTABLE_TEST_OBJS += reftable/record_test.o REFTABLE_TEST_OBJS += reftable/test_framework.o -REFTABLE_TEST_OBJS += reftable/basics_test.o +REFTABLE_TEST_OBJS += reftable/tree_test.o TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))
reftable/tree.c new file +63 -0
63 lines not on this page · raw message
reftable/tree.h new file +34 -0
@@ -0,0 +1,34 @@ +/* +Copyright 2020 Google LLC + +Use of this source code is governed by a BSD-style +license that can be found in the LICENSE file or at +https://developers.google.com/open-source/licenses/bsd +*/ + +#ifndef TREE_H +#define TREE_H + +/* tree_node is a generic binary search tree. */ +struct tree_node { +void *key; +struct tree_node *left, *right; +}; + +/* looks for `key` in `rootp` using `compare` as comparison function. If insert +* is set, insert the key if it's not found. Else, return NULL. +*/ +struct tree_node *tree_search (void *key,struct tree_node **rootp, +int (*compare)(const void *,const void *), +int insert); + +/* performs an infix walk of the tree. */ +void infix_walk (struct tree_node *t,void (*action)(void *arg,void *key), +void *arg); + +/* +* deallocates the tree nodes recursively. Keys should be deallocated separately +* by walking over the tree. */ +void tree_free (struct tree_node *t); + +#endif
reftable/tree_test.c new file +61 -0
61 lines not on this page · raw message
t/helper/test-reftable.c +1 -0
@@ -6,5 +6,6 @@ int cmd__reftable(int argc, const char **argv)basics_test_main (argc, argv);block_test_main (argc, argv);record_test_main (argc, argv); +tree_test_main (argc, argv);return 0 ; }
Signature · 1 line
gitgitgadget
PATCH v7 14/28 reftable: write reftable files
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
--- Makefile | 1 + reftable/reftable-writer.h | 147 ++++++++ reftable/writer.c | 690 +++++++++++++++++++++++++++++++++++++ reftable/writer.h | 50 +++ 4 files changed, 888 insertions(+) create mode 100644 reftable/reftable-writer.h create mode 100644 reftable/writer.c create mode 100644 reftable/writer.h
Makefile +1 -0
@@ -2415,6 +2415,7 @@ REFTABLE_OBJS += reftable/blocksource.o REFTABLE_OBJS += reftable/publicbasics.o REFTABLE_OBJS += reftable/record.o REFTABLE_OBJS += reftable/tree.o +REFTABLE_OBJS += reftable/writer.o REFTABLE_TEST_OBJS += reftable/basics_test.o REFTABLE_TEST_OBJS += reftable/block_test.o
reftable/reftable-writer.h new file +147 -0
147 lines not on this page · raw message
reftable/writer.c new file +690 -0
690 lines not on this page · raw message
reftable/writer.h new file +50 -0
50 lines not on this page · raw message
Signature · 1 line
gitgitgadget
PATCH v7 15/28 reftable: generic interface to tables
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
--- Makefile | 3 + reftable/generic.c | 169 +++++++++++++++++++++++++++++++++++ reftable/generic.h | 32 +++++++ reftable/reftable-generic.h | 47 ++++++++++ reftable/reftable-iterator.h | 39 ++++++++ reftable/reftable.c | 115 ++++++++++++++++++++++++ 6 files changed, 405 insertions(+) create mode 100644 reftable/generic.c create mode 100644 reftable/generic.h create mode 100644 reftable/reftable-generic.h create mode 100644 reftable/reftable-iterator.h create mode 100644 reftable/reftable.c
Makefile +3 -0
@@ -2414,6 +2414,9 @@ REFTABLE_OBJS += reftable/block.o REFTABLE_OBJS += reftable/blocksource.o REFTABLE_OBJS += reftable/publicbasics.o REFTABLE_OBJS += reftable/record.o +REFTABLE_OBJS += reftable/refname.o +REFTABLE_OBJS += reftable/generic.o +REFTABLE_OBJS += reftable/stack.o REFTABLE_OBJS += reftable/tree.o REFTABLE_OBJS += reftable/writer.o
reftable/generic.c new file +169 -0
169 lines not on this page · raw message
reftable/generic.h new file +32 -0
@@ -0,0 +1,32 @@ +/* +Copyright 2020 Google LLC + +Use of this source code is governed by a BSD-style +license that can be found in the LICENSE file or at +https://developers.google.com/open-source/licenses/bsd +*/ + +#ifndef GENERIC_H +#define GENERIC_H + +#include "record.h" +#include "reftable-generic.h" + +/* generic interface to reftables */ +struct reftable_table_vtable { +int (*seek_record)(void *tab,struct reftable_iterator *it, +struct reftable_record *); +uint32_t (*hash_id)(void *tab); +uint64_t (*min_update_index)(void *tab); +uint64_t (*max_update_index)(void *tab); +}; + +struct reftable_iterator_vtable { +int (*next)(void *iter_arg,struct reftable_record *rec); +void (*close)(void *iter_arg); +}; + +void iterator_set_empty (struct reftable_iterator *it); +int iterator_next (struct reftable_iterator *it,struct reftable_record *rec); + +#endif
reftable/reftable-generic.h new file +47 -0
47 lines not on this page · raw message
reftable/reftable-iterator.h new file +39 -0
@@ -0,0 +1,39 @@ +/* +Copyright 2020 Google LLC + +Use of this source code is governed by a BSD-style +license that can be found in the LICENSE file or at +https://developers.google.com/open-source/licenses/bsd +*/ + +#ifndef REFTABLE_ITERATOR_H +#define REFTABLE_ITERATOR_H + +#include "reftable-record.h" + +struct reftable_iterator_vtable ; + +/* iterator is the generic interface for walking over data stored in a +* reftable. +*/ +struct reftable_iterator { +struct reftable_iterator_vtable *ops; +void *iter_arg; +}; + +/* reads the next reftable_ref_record. Returns < 0 for error, 0 for OK and > 0: +* end of iteration. +*/ +int reftable_iterator_next_ref (struct reftable_iterator *it, +struct reftable_ref_record *ref); + +/* reads the next reftable_log_record. Returns < 0 for error, 0 for OK and > 0: +* end of iteration. +*/ +int reftable_iterator_next_log (struct reftable_iterator *it, +struct reftable_log_record *log); + +/* releases resources associated with an iterator. */ +void reftable_iterator_destroy (struct reftable_iterator *it); + +#endif
reftable/reftable.c new file +115 -0
115 lines not on this page · raw message
Signature · 1 line
gitgitgadget
PATCH v7 16/28 reftable: read reftable files
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
This supports reading a single reftable file.
The commit introduces an abstract iterator type, which captures the usecases both of reading individual refs, and iterating over a segment of the ref namespace.
--- Makefile | 2 + reftable/iter.c | 194 +++++++++ reftable/iter.h | 69 ++++ reftable/reader.c | 794 +++++++++++++++++++++++++++++++++++++ reftable/reader.h | 66 +++ reftable/reftable-reader.h | 101 +++++ 6 files changed, 1226 insertions(+) create mode 100644 reftable/iter.c create mode 100644 reftable/iter.h create mode 100644 reftable/reader.c create mode 100644 reftable/reader.h create mode 100644 reftable/reftable-reader.h
Makefile +2 -0
@@ -2412,7 +2412,9 @@ REFTABLE_OBJS += reftable/basics.o REFTABLE_OBJS += reftable/error.o REFTABLE_OBJS += reftable/block.o REFTABLE_OBJS += reftable/blocksource.o +REFTABLE_OBJS += reftable/iter.o REFTABLE_OBJS += reftable/publicbasics.o +REFTABLE_OBJS += reftable/reader.o REFTABLE_OBJS += reftable/record.o REFTABLE_OBJS += reftable/refname.o REFTABLE_OBJS += reftable/generic.o
reftable/iter.c new file +194 -0
194 lines not on this page · raw message
reftable/iter.h new file +69 -0
69 lines not on this page · raw message
reftable/reader.c new file +794 -0
794 lines not on this page · raw message
reftable/reader.h new file +66 -0
66 lines not on this page · raw message
reftable/reftable-reader.h new file +101 -0
101 lines not on this page · raw message
Signature · 1 line
gitgitgadget
PATCH v7 17/28 reftable: reftable file level tests
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
With support for reading and writing files in place, we can construct files (in memory) and attempt to read them back.
Because some sections of the format are optional (eg. indices, log entries), we have to exercise this code using multiple sizes of input data
--- Makefile | 1 + reftable/readwrite_test.c | 621 ++++++++++++++++++++++++++++++++++++++ reftable/reftable-tests.h | 2 +- t/helper/test-reftable.c | 1 + 4 files changed, 624 insertions(+), 1 deletion(-) create mode 100644 reftable/readwrite_test.c
Makefile +1 -0
@@ -2425,6 +2425,7 @@ REFTABLE_OBJS += reftable/writer.o REFTABLE_TEST_OBJS += reftable/basics_test.o REFTABLE_TEST_OBJS += reftable/block_test.o REFTABLE_TEST_OBJS += reftable/record_test.o +REFTABLE_TEST_OBJS += reftable/readwrite_test.o REFTABLE_TEST_OBJS += reftable/test_framework.o REFTABLE_TEST_OBJS += reftable/tree_test.o
reftable/readwrite_test.c new file +621 -0
621 lines not on this page · raw message
reftable/reftable-tests.h +1 -1
@@ -14,7 +14,7 @@ int block_test_main(int argc, const char **argv);int merged_test_main (int argc,const char **argv);int record_test_main (int argc,const char **argv);int refname_test_main (int argc,const char **argv); -int reftable_test_main (int argc,const char **argv); +int readwrite_test_main (int argc,const char **argv);int stack_test_main (int argc,const char **argv);int tree_test_main (int argc,const char **argv);int reftable_dump_main (int argc,char *const *argv);
t/helper/test-reftable.c +1 -0
@@ -6,6 +6,7 @@ int cmd__reftable(int argc, const char **argv)basics_test_main (argc, argv);block_test_main (argc, argv);record_test_main (argc, argv); +readwrite_test_main (argc, argv);tree_test_main (argc, argv);return 0 ; }
Signature · 1 line
gitgitgadget
PATCH v7 18/28 reftable: add a heap-based priority queue for reftable records
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
This is needed to create a merged view multiple reftables
--- Makefile | 2 + reftable/pq.c | 115 ++++++++++++++++++++++++++++++++++++++ reftable/pq.h | 32 +++++++++++ reftable/pq_test.c | 72 ++++++++++++++++++++++++ reftable/reftable-tests.h | 1 + t/helper/test-reftable.c | 1 + 6 files changed, 223 insertions(+) create mode 100644 reftable/pq.c create mode 100644 reftable/pq.h create mode 100644 reftable/pq_test.c
Makefile +2 -0
@@ -2414,6 +2414,7 @@ REFTABLE_OBJS += reftable/block.o REFTABLE_OBJS += reftable/blocksource.o REFTABLE_OBJS += reftable/iter.o REFTABLE_OBJS += reftable/publicbasics.o +REFTABLE_OBJS += reftable/pq.o REFTABLE_OBJS += reftable/reader.o REFTABLE_OBJS += reftable/record.o REFTABLE_OBJS += reftable/refname.o
@@ -2424,6 +2425,7 @@ REFTABLE_OBJS += reftable/writer.o REFTABLE_TEST_OBJS += reftable/basics_test.o REFTABLE_TEST_OBJS += reftable/block_test.o +REFTABLE_TEST_OBJS += reftable/pq_test.o REFTABLE_TEST_OBJS += reftable/record_test.o REFTABLE_TEST_OBJS += reftable/readwrite_test.o REFTABLE_TEST_OBJS += reftable/test_framework.o
reftable/pq.c new file +115 -0
115 lines not on this page · raw message
reftable/pq.h new file +32 -0
@@ -0,0 +1,32 @@ +/* +Copyright 2020 Google LLC + +Use of this source code is governed by a BSD-style +license that can be found in the LICENSE file or at +https://developers.google.com/open-source/licenses/bsd +*/ + +#ifndef PQ_H +#define PQ_H + +#include "record.h" + +struct pq_entry { +int index; +struct reftable_record rec; +}; + +struct merged_iter_pqueue { +struct pq_entry *heap; +size_t len; +size_t cap; +}; + +struct pq_entry merged_iter_pqueue_top (struct merged_iter_pqueue pq); +int merged_iter_pqueue_is_empty (struct merged_iter_pqueue pq); +void merged_iter_pqueue_check (struct merged_iter_pqueue pq); +struct pq_entry merged_iter_pqueue_remove (struct merged_iter_pqueue *pq); +void merged_iter_pqueue_add (struct merged_iter_pqueue *pq,struct pq_entry e); +void merged_iter_pqueue_release (struct merged_iter_pqueue *pq); + +#endif
reftable/pq_test.c new file +72 -0
72 lines not on this page · raw message
reftable/reftable-tests.h +1 -0
@@ -12,6 +12,7 @@ license that can be found in the LICENSE file or atint basics_test_main (int argc,const char **argv);int block_test_main (int argc,const char **argv);int merged_test_main (int argc,const char **argv); +int pq_test_main (int argc,const char **argv);int record_test_main (int argc,const char **argv);int refname_test_main (int argc,const char **argv);int readwrite_test_main (int argc,const char **argv);
t/helper/test-reftable.c +1 -0
@@ -5,6 +5,7 @@ int cmd__reftable(int argc, const char **argv) {basics_test_main (argc, argv);block_test_main (argc, argv); +pq_test_main (argc, argv);record_test_main (argc, argv);readwrite_test_main (argc, argv);tree_test_main (argc, argv);
Signature · 1 line
gitgitgadget
PATCH v7 19/28 reftable: add merged table view
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
This adds an abstract, read-only interface to the ref database.
This primitive is used to construct the read view of the ref database (the read view is constructed by merging several *.ref files). It also provides the mechanism to provide a unified view of the refs in the main repository and the per-worktree refs.
--- Makefile | 2 + reftable/merged.c | 362 +++++++++++++++++++++++++++++++++++++ reftable/merged.h | 35 ++++ reftable/merged_test.c | 292 ++++++++++++++++++++++++++++++ reftable/reftable-merged.h | 72 ++++++++ t/helper/test-reftable.c | 1 + 6 files changed, 764 insertions(+) create mode 100644 reftable/merged.c create mode 100644 reftable/merged.h create mode 100644 reftable/merged_test.c create mode 100644 reftable/reftable-merged.h
Makefile +2 -0
@@ -2414,6 +2414,7 @@ REFTABLE_OBJS += reftable/block.o REFTABLE_OBJS += reftable/blocksource.o REFTABLE_OBJS += reftable/iter.o REFTABLE_OBJS += reftable/publicbasics.o +REFTABLE_OBJS += reftable/merged.o REFTABLE_OBJS += reftable/pq.o REFTABLE_OBJS += reftable/reader.o REFTABLE_OBJS += reftable/record.o
@@ -2425,6 +2426,7 @@ REFTABLE_OBJS += reftable/writer.o REFTABLE_TEST_OBJS += reftable/basics_test.o REFTABLE_TEST_OBJS += reftable/block_test.o +REFTABLE_TEST_OBJS += reftable/merged_test.o REFTABLE_TEST_OBJS += reftable/pq_test.o REFTABLE_TEST_OBJS += reftable/record_test.o REFTABLE_TEST_OBJS += reftable/readwrite_test.o
reftable/merged.c new file +362 -0
362 lines not on this page · raw message
reftable/merged.h new file +35 -0
@@ -0,0 +1,35 @@ +/* +Copyright 2020 Google LLC + +Use of this source code is governed by a BSD-style +license that can be found in the LICENSE file or at +https://developers.google.com/open-source/licenses/bsd +*/ + +#ifndef MERGED_H +#define MERGED_H + +#include "pq.h" + +struct reftable_merged_table { +struct reftable_table *stack; +size_t stack_len; +uint32_t hash_id; +int suppress_deletions; + +uint64_t min; +uint64_t max; +}; + +struct merged_iter { +struct reftable_iterator *stack; +uint32_t hash_id; +size_t stack_len; +uint8_t typ; +int suppress_deletions; +struct merged_iter_pqueue pq; +}; + +void merged_table_release (struct reftable_merged_table *mt); + +#endif
reftable/merged_test.c new file +292 -0
292 lines not on this page · raw message
reftable/reftable-merged.h new file +72 -0
72 lines not on this page · raw message
t/helper/test-reftable.c +1 -0
@@ -5,6 +5,7 @@ int cmd__reftable(int argc, const char **argv) {basics_test_main (argc, argv);block_test_main (argc, argv); +merged_test_main (argc, argv);pq_test_main (argc, argv);record_test_main (argc, argv);readwrite_test_main (argc, argv);
Signature · 1 line
gitgitgadget
PATCH v7 20/28 reftable: implement refname validation
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
The packed/loose format has restrictions on refnames: a and a/b cannot coexist. This limitation does not apply to reftable per se, but must be maintained for interoperability. This code adds validation routines to abort transactions that are trying to add invalid names.
--- Makefile | 1 + reftable/refname.c | 209 +++++++++++++++++++++++++++++++++++++++ reftable/refname.h | 29 ++++++ reftable/refname_test.c | 102 +++++++++++++++++++ t/helper/test-reftable.c | 1 + 5 files changed, 342 insertions(+) create mode 100644 reftable/refname.c create mode 100644 reftable/refname.h create mode 100644 reftable/refname_test.c
Makefile +1 -0
@@ -2430,6 +2430,7 @@ REFTABLE_TEST_OBJS += reftable/merged_test.o REFTABLE_TEST_OBJS += reftable/pq_test.o REFTABLE_TEST_OBJS += reftable/record_test.o REFTABLE_TEST_OBJS += reftable/readwrite_test.o +REFTABLE_TEST_OBJS += reftable/refname_test.o REFTABLE_TEST_OBJS += reftable/test_framework.o REFTABLE_TEST_OBJS += reftable/tree_test.o
reftable/refname.c new file +209 -0
209 lines not on this page · raw message
reftable/refname.h new file +29 -0
@@ -0,0 +1,29 @@ +/* +Copyright 2020 Google LLC + +Use of this source code is governed by a BSD-style +license that can be found in the LICENSE file or at +https://developers.google.com/open-source/licenses/bsd +*/ +#ifndef REFNAME_H +#define REFNAME_H + +#include "reftable-record.h" +#include "reftable-generic.h" + +struct modification { +struct reftable_table tab; + +char **add; +size_t add_len; + +char **del; +size_t del_len; +}; + +int validate_ref_record_addition (struct reftable_table tab, +struct reftable_ref_record *recs,size_t sz); + +int modification_validate (struct modification *mod); + +#endif
reftable/refname_test.c new file +102 -0
102 lines not on this page · raw message
t/helper/test-reftable.c +1 -0
@@ -8,6 +8,7 @@ int cmd__reftable(int argc, const char **argv)merged_test_main (argc, argv);pq_test_main (argc, argv);record_test_main (argc, argv); +refname_test_main (argc, argv);readwrite_test_main (argc, argv);tree_test_main (argc, argv);return 0 ;
Signature · 1 line
gitgitgadget
PATCH v7 21/28 reftable: implement stack, a mutable database of reftable files.
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
--- Makefile | 1 + reftable/reftable-stack.h | 128 ++++ reftable/stack.c | 1395 +++++++++++++++++++++++++++++++++++++ reftable/stack.h | 41 ++ reftable/stack_test.c | 940 +++++++++++++++++++++++++ t/helper/test-reftable.c | 1 + 6 files changed, 2506 insertions(+) create mode 100644 reftable/reftable-stack.h create mode 100644 reftable/stack.c create mode 100644 reftable/stack.h create mode 100644 reftable/stack_test.c
Makefile +1 -0
@@ -2431,6 +2431,7 @@ REFTABLE_TEST_OBJS += reftable/pq_test.o REFTABLE_TEST_OBJS += reftable/record_test.o REFTABLE_TEST_OBJS += reftable/readwrite_test.o REFTABLE_TEST_OBJS += reftable/refname_test.o +REFTABLE_TEST_OBJS += reftable/stack_test.o REFTABLE_TEST_OBJS += reftable/test_framework.o REFTABLE_TEST_OBJS += reftable/tree_test.o
reftable/reftable-stack.h new file +128 -0
128 lines not on this page · raw message
reftable/stack.c new file +1395 -0
1,395 lines not on this page · raw message
reftable/stack.h new file +41 -0
41 lines not on this page · raw message
reftable/stack_test.c new file +940 -0
940 lines not on this page · raw message
t/helper/test-reftable.c +1 -0
@@ -10,6 +10,7 @@ int cmd__reftable(int argc, const char **argv)record_test_main (argc, argv);refname_test_main (argc, argv);readwrite_test_main (argc, argv); +stack_test_main (argc, argv);tree_test_main (argc, argv);return 0 ; }
Signature · 1 line
gitgitgadget
PATCH v7 22/28 reftable: add dump utility
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
provide a command-line utility for inspecting individual tables, and inspecting a complete ref database
--- reftable/dump.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 reftable/dump.c
reftable/dump.c new file +100 -0
@@ -0,0 +1,100 @@ +/* +Copyright 2020 Google LLC + +Use of this source code is governed by a BSD-style +license that can be found in the LICENSE file or at +https://developers.google.com/open-source/licenses/bsd +*/ + +#include <stddef.h> +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <string.h> + +#include "reftable-blocksource.h" +#include "reftable-error.h" +#include "reftable-merged.h" +#include "reftable-record.h" +#include "reftable-tests.h" +#include "reftable-writer.h" +#include "reftable-iterator.h" +#include "reftable-reader.h" +#include "reftable-stack.h" +#include "reftable-generic.h" + +static int compact_stack (const char *stackdir) +{ +struct reftable_stack *stack =NULL ; +struct reftable_write_options cfg = {0 }; + +int err =reftable_new_stack (&stack, stackdir, cfg); +if (err <0 ) + goto done; + + err =reftable_stack_compact_all (stack,NULL ); +if (err <0 ) + goto done; +done: +if (stack) { +reftable_stack_destroy (stack); + } +return err; +} + +static void print_help (void ) +{ +printf ("usage: dump [-cst] arg\n\n" +"options: \n" +" -c compact\n" +" -t dump table\n" +" -s dump stack\n" +" -h this help\n" +"\n" ); +} + +int reftable_dump_main (int argc,char *const *argv) +{ +int err =0 ; +int opt_dump_table =0 ; +int opt_dump_stack =0 ; +int opt_compact =0 ; +const char *arg =NULL , *argv0 = argv[0 ]; + +for (; argc >1 ; argv++, argc--) +if (*argv[1 ] !='-' ) +break ; +else if (!strcmp ("-t" , argv[1 ])) + opt_dump_table =1 ; +else if (!strcmp ("-s" , argv[1 ])) + opt_dump_stack =1 ; +else if (!strcmp ("-c" , argv[1 ])) + opt_compact =1 ; +else if (!strcmp ("-?" , argv[1 ]) || !strcmp ("-h" , argv[1 ])) { +print_help (); +return 2 ; + } + +if (argc !=2 ) { +fprintf (stderr,"need argument\n" ); +print_help (); +return 2 ; + } + + arg = argv[1 ]; + +if (opt_dump_table) { + err =reftable_reader_print_file (arg); + }else if (opt_dump_stack) { + err =reftable_stack_print_directory (arg); + }else if (opt_compact) { + err =compact_stack (arg); + } + +if (err <0 ) { +fprintf (stderr,"%s: %s: %s\n" , argv0, arg, +reftable_error_str (err)); +return 1 ; + } +return 0 ; +}
Signature · 1 line
gitgitgadget
PATCH v7 23/28 Reftable support for git-core
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
For background, see the previous commit introducing the library.
This introduces the file refs/reftable-backend.c containing a reftable-powered ref storage backend.
It can be activated by passing --ref-storage=reftable to "git init", or setting GIT_TEST_REFTABLE in the environment.
Example use: see t/t0031-reftable.sh
--- Documentation/config/extensions.txt | 9 + .../technical/repository-version.txt | 7 + Makefile | 1 + builtin/clone.c | 5 +- builtin/init-db.c | 55 +- builtin/stash.c | 8 +- builtin/worktree.c | 27 +- cache.h | 8 +- config.mak.uname | 2 +- contrib/buildsystems/Generators/Vcxproj.pm | 11 +- refs.c | 26 +- refs.h | 3 + refs/refs-internal.h | 1 + refs/reftable-backend.c | 1616 +++++++++++++++++ repository.c | 2 + repository.h | 3 + setup.c | 9 +- t/t0031-reftable.sh | 271 +++ t/t1409-avoid-packing-refs.sh | 6 + t/t1450-fsck.sh | 6 + t/t3210-pack-refs.sh | 6 + t/test-lib.sh | 5 + 22 files changed, 2053 insertions(+), 34 deletions(-) create mode 100644 refs/reftable-backend.c create mode 100755 t/t0031-reftable.sh
Documentation/config/extensions.txt +9 -0
@@ -6,3 +6,12 @@ extensions.objectFormat:: Note that this setting should only be set by linkgit:git-init[1] or linkgit:git-clone[1]. Trying to change it after initialization will not work and will produce hard-to-diagnose issues. ++ +extensions.refStorage:: + Specify the ref storage mechanism to use. The acceptable values are `files` and + `reftable`. If not specified, `files` is assumed. It is an error to specify + this key unless `core.repositoryFormatVersion` is 1. ++ +Note that this setting should only be set by linkgit:git-init[1] or +linkgit:git-clone[1]. Trying to change it after initialization will not +work and will produce hard-to-diagnose issues.
Documentation/technical/repository-version.txt +7 -0
@@ -100,3 +100,10 @@ If set, by default "git config" reads from both "config" and multiple working directory mode, "config" file is shared while "config.worktree" is per-working directory (i.e., it's in GIT_COMMON_DIR/worktrees/<id>/config.worktree) + +==== `refStorage` + +Specifies the file format for the ref database. Values are `files` +(for the traditional packed + loose ref format) and `reftable` for the +binary reftable format. See https://github.com/google/reftable for +more information.
Makefile +1 -0
@@ -978,6 +978,7 @@ LIB_OBJS += reflog-walk.o LIB_OBJS += refs.o LIB_OBJS += refs/debug.o LIB_OBJS += refs/files-backend.o +LIB_OBJS += refs/reftable-backend.o LIB_OBJS += refs/iterator.o LIB_OBJS += refs/packed-backend.o LIB_OBJS += refs/ref-cache.o
builtin/clone.c +3 -2
@@ -1148,7 +1148,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) }init_db (git_dir, real_git_dir, option_template,GIT_HASH_UNKNOWN ,NULL , -INIT_DB_QUIET ); +default_ref_storage (),INIT_DB_QUIET );if (real_git_dir) git_dir = real_git_dir;
@@ -1299,7 +1299,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)* Now that we know what algorithm the remote side is using, * let's set ours to the same thing. */ -initialize_repository_version (hash_algo,1 ); +initialize_repository_version (hash_algo,1 , +default_ref_storage ());repo_set_hash_algo (the_repository, hash_algo); mapped_refs =wanted_peer_refs (refs, &remote->fetch);
builtin/init-db.c +41 -14
131 lines not on this page · raw message
builtin/stash.c +7 -1
@@ -207,10 +207,16 @@ static int get_stash_info(struct stash_info *info, int argc, const char **argv)static int do_clear_stash (void ) {struct object_id obj; +int result;if (get_oid (ref_stash, &obj))return 0 ; -return delete_ref (NULL , ref_stash, &obj,0 ); + result =delete_ref (NULL , ref_stash, &obj,0 ); + +/* Ignore error; this is necessary for reftable, which keeps reflogs +* even when refs are deleted. */ +delete_reflog (ref_stash); +return result; }static int clear_stash (int argc,const char **argv,const char *prefix)
builtin/worktree.c +24 -3
@@ -13,6 +13,7 @@#include "utf8.h" #include "worktree.h" #include "quote.h" +#include "../refs/refs-internal.h" static const char *const worktree_usage[] = {N_ ("git worktree add [<options>] <path> [<commit-ish>]" ),
@@ -330,9 +331,29 @@ static int add_worktree(const char *path, const char *refname,* worktree. */ strbuf_reset (&sb); -strbuf_addf (&sb,"%s/HEAD" , sb_repo.buf); -write_file (sb.buf,"%s" ,oid_to_hex (&null_oid)); -strbuf_reset (&sb); +if (get_main_ref_store (the_repository)->be == &refs_be_reftable) { +/* XXX this is cut & paste from reftable_init_db. */ +strbuf_addf (&sb,"%s/HEAD" , sb_repo.buf); +write_file (sb.buf,"%s" ,"ref: refs/heads/.invalid\n" ); +strbuf_reset (&sb); + +strbuf_addf (&sb,"%s/refs" , sb_repo.buf); +safe_create_dir (sb.buf,1 ); +strbuf_reset (&sb); + +strbuf_addf (&sb,"%s/refs/heads" , sb_repo.buf); +write_file (sb.buf,"this repository uses the reftable format" ); +strbuf_reset (&sb); + +strbuf_addf (&sb,"%s/reftable" , sb_repo.buf); +safe_create_dir (sb.buf,1 ); +strbuf_reset (&sb); + }else { +strbuf_addf (&sb,"%s/HEAD" , sb_repo.buf); +write_file (sb.buf,"%s" ,oid_to_hex (&null_oid)); +strbuf_reset (&sb); + } +strbuf_addf (&sb,"%s/commondir" , sb_repo.buf);write_file (sb.buf,"../.." );
cache.h +5 -3
@@ -629,9 +629,10 @@ int path_inside_repo(const char *prefix, const char *path);#define INIT_DB_EXIST_OK 0x0002int init_db (const char *git_dir,const char *real_git_dir, -const char *template_dir,int hash_algo, -const char *initial_branch,unsigned int flags); -void initialize_repository_version (int hash_algo,int reinit); +const char *template_dir,int hash_algo,const char *initial_branch, +const char *ref_storage_format,unsigned int flags); +void initialize_repository_version (int hash_algo,int reinit, +const char *ref_storage_format);void sanitize_stdfds (void );int daemonize (void );
@@ -1045,6 +1046,7 @@ struct repository_format {int is_bare;int hash_algo;char *work_tree; +char *ref_storage;struct string_list unknown_extensions;struct string_list v1_only_extensions; };
config.mak.uname +1 -1
@@ -701,7 +701,7 @@ vcxproj: # Make .vcxproj files and add them unset QUIET_GEN QUIET_BUILT_IN; \ perl contrib/buildsystems/generate -g Vcxproj - git add -f git.sln {*,*/lib,t/helper/*}/*.vcxproj + git add -f git.sln {*,*/lib,*/libreftable,t/helper/*}/*.vcxproj # Generate the LinkOrCopyBuiltins.targets and LinkOrCopyRemoteHttp.targets file (echo '<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">' && \
contrib/buildsystems/Generators/Vcxproj.pm +10 -1
@@ -77,7 +77,7 @@ sub createProject { my $libs_release = "\n "; my $libs_debug = "\n "; if (!$static_library) { - $libs_release = join(";", sort(grep /^(?!libgit\.lib|xdiff\/lib\.lib|vcs-svn\/lib\.lib)/, @{$$build_structure{"$prefix${name}_LIBS"}})); + $libs_release = join(";", sort(grep /^(?!libgit\.lib|xdiff\/lib\.lib|vcs-svn\/lib\.lib|reftable\/libreftable\.lib)/, @{$$build_structure{"$prefix${name}_LIBS"}})); $libs_debug = $libs_release; $libs_debug =~ s/zlib\.lib/zlibd\.lib/g; $libs_debug =~ s/libexpat\.lib/libexpatd\.lib/g;
@@ -232,6 +232,7 @@ sub createProject { EOM if (!$static_library || $target =~ 'vcs-svn' || $target =~ 'xdiff') { my $uuid_libgit = $$build_structure{"LIBS_libgit_GUID"}; + my $uuid_libreftable = $$build_structure{"LIBS_reftable/libreftable_GUID"}; my $uuid_xdiff_lib = $$build_structure{"LIBS_xdiff/lib_GUID"}; print F << "EOM";
@@ -241,6 +242,14 @@ sub createProject { <ReferenceOutputAssembly>false</ReferenceOutputAssembly> </ProjectReference> EOM + if (!($name =~ /xdiff|libreftable/)) { + print F << "EOM"; + <ProjectReference Include="$cdup\\reftable\\libreftable\\libreftable.vcxproj"> + <Project>$uuid_libreftable</Project> + <ReferenceOutputAssembly>false</ReferenceOutputAssembly> + </ProjectReference> +EOM + } if (!($name =~ 'xdiff')) { print F << "EOM"; <ProjectReference Include="$cdup\\xdiff\\lib\\xdiff_lib.vcxproj">
refs.c +18 -8
@@ -19,10 +19,15 @@#include "repository.h" #include "sigchain.h" +const char *default_ref_storage (void ) +{ +return git_env_bool ("GIT_TEST_REFTABLE" ,0 ) ?"reftable" :"files" ; +} +/* * List of all available backends */ -static struct ref_storage_be *refs_backends = &refs_be_files; +static struct ref_storage_be *refs_backends = &refs_be_reftable;static struct ref_storage_be *find_ref_storage_backend (const char *name) {
@@ -1875,13 +1880,13 @@ static struct ref_store *lookup_ref_store_map(struct hashmap *map,* Create, record, and return a ref_store instance for the specified * gitdir. */ -static struct ref_store *ref_store_init (const char *gitdir, +static struct ref_store *ref_store_init (const char *gitdir,const char *be_name,unsigned int flags) { -const char *be_name ="files" ; -struct ref_storage_be *be =find_ref_storage_backend (be_name); +struct ref_storage_be *be;struct ref_store *refs; + be =find_ref_storage_backend (be_name);if (!be)BUG ("reference backend %s is unknown" , be_name);
@@ -1897,7 +1902,11 @@ struct ref_store *get_main_ref_store(struct repository *r)if (!r->gitdir)BUG ("attempting to get main_ref_store outside of repository" ); - r->refs_private =ref_store_init (r->gitdir,REF_STORE_ALL_CAPS ); + r->refs_private =ref_store_init (r->gitdir, + r->ref_storage_format ? + r->ref_storage_format : +default_ref_storage (), +REF_STORE_ALL_CAPS ); r->refs_private =maybe_debug_wrap_ref_store (r->gitdir, r->refs_private);return r->refs_private; }
@@ -1953,7 +1962,7 @@ struct ref_store *get_submodule_ref_store(const char *submodule) goto done;/* assume that add_submodule_odb() has been called */ - refs =ref_store_init (submodule_sb.buf, + refs =ref_store_init (submodule_sb.buf,default_ref_storage (),REF_STORE_READ |REF_STORE_ODB );register_ref_store_map (&submodule_ref_stores,"submodule" , refs, submodule);
@@ -1967,6 +1976,7 @@ struct ref_store *get_submodule_ref_store(const char *submodule)struct ref_store *get_worktree_ref_store (const struct worktree *wt) { +const char *format =default_ref_storage ();struct ref_store *refs;const char *id;
@@ -1980,9 +1990,9 @@ struct ref_store *get_worktree_ref_store(const struct worktree *wt)if (wt->id) refs =ref_store_init (git_common_path ("worktrees/%s" , wt->id), -REF_STORE_ALL_CAPS ); + format,REF_STORE_ALL_CAPS );else - refs =ref_store_init (get_git_common_dir (), + refs =ref_store_init (get_git_common_dir (), format,REF_STORE_ALL_CAPS );if (refs)
refs.h +3 -0
@@ -11,6 +11,9 @@ struct string_list;struct string_list_item ;struct worktree ; +/* Returns the ref storage backend to use by default. */ +const char *default_ref_storage (void ); +/* * Resolve a reference, recursively following symbolic refererences. *
refs/refs-internal.h +1 -0
@@ -676,6 +676,7 @@ struct ref_storage_be { };extern struct ref_storage_be refs_be_files; +extern struct ref_storage_be refs_be_reftable;extern struct ref_storage_be refs_be_packed;/*
refs/reftable-backend.c new file +1616 -0
1,616 lines not on this page · raw message
repository.c +2 -0
@@ -174,6 +174,8 @@ int repo_init(struct repository *repo,if (worktree)repo_set_worktree (repo, worktree); + repo->ref_storage_format =xstrdup_or_null (format.ref_storage); +clear_repository_format (&format);return 0 ;
repository.h +3 -0
@@ -78,6 +78,9 @@ struct repository {*/ struct ref_store *refs_private; +/* The format to use for the ref database. */ +char *ref_storage_format; +/* * Contains path to often used file names. */
setup.c +8 -1
@@ -500,6 +500,9 @@ static enum extension_result handle_extension(const char *var,return error ("invalid value for 'extensions.objectformat'" ); data->hash_algo = format;return EXTENSION_OK ; + }else if (!strcmp (ext,"refstorage" )) { + data->ref_storage =xstrdup (value); +return EXTENSION_OK ; }return EXTENSION_UNKNOWN ; }
@@ -651,6 +654,7 @@ void clear_repository_format(struct repository_format *format)string_list_clear (&format->v1_only_extensions,0 );free (format->work_tree);free (format->partial_clone); +free (format->ref_storage);init_repository_format (format); }
@@ -1300,8 +1304,11 @@ const char *setup_git_directory_gently(int *nongit_ok) gitdir =DEFAULT_GIT_DIR_ENVIRONMENT ;setup_git_env (gitdir); } -if (startup_info->have_repository) +if (startup_info->have_repository) {repo_set_hash_algo (the_repository, repo_fmt.hash_algo); + the_repository->ref_storage_format = +xstrdup_or_null (repo_fmt.ref_storage); + } }/* * Since precompose_string_if_needed() needs to look at
t/t0031-reftable.sh new file +271 -0
271 lines not on this page · raw message
t/t1409-avoid-packing-refs.sh +6 -0
@@ -4,6 +4,12 @@ test_description='avoid rewriting packed-refs unnecessarily' . ./test-lib.sh +if test_have_prereq REFTABLE +then + skip_all='skipping pack-refs tests; incompatible with reftable' + test_done +fi + # Add an identifying mark to the packed-refs file header line. This # shouldn't upset readers, and it should be omitted if the file is # ever rewritten.
t/t1450-fsck.sh +6 -0
@@ -8,6 +8,12 @@ test_description='git fsck random collection of tests . ./test-lib.sh +if test_have_prereq REFTABLE +then + skip_all='skipping tests; incompatible with reftable' + test_done +fi + test_expect_success setup ' git config gc.auto 0 && git config i18n.commitencoding ISO-8859-1 &&
t/t3210-pack-refs.sh +6 -0
@@ -14,6 +14,12 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME . ./test-lib.sh +if test_have_prereq REFTABLE +then + skip_all='skipping pack-refs tests; incompatible with reftable' + test_done +fi + test_expect_success 'enable reflogs' ' git config core.logallrefupdates true '
t/test-lib.sh +5 -0
@@ -1481,6 +1481,11 @@ parisc* | hppa*) ;; esac +if test -n "$GIT_TEST_REFTABLE" +then + test_set_prereq REFTABLE +fi + ( COLUMNS=1 && test $COLUMNS = 1 ) && test_set_prereq COLUMNS_CAN_BE_1 test -z "$NO_PERL" && test_set_prereq PERL test -z "$NO_PTHREADS" && test_set_prereq PTHREADS
Signature · 1 line
gitgitgadget
Re: [PATCH v7 23/28] Reftable support for git-core
t/test-lib.sh parisc* | hppa*) · line 1487
Quoted 4 lines · Han-Wen Nienhuys via GitGitGadget
t/test-lib.sh +4
-0+if test -n "$GIT_TEST_REFTABLE" +then + test_set_prereq REFTABLE +fi
How would this interact with what the test prep series did which was to unconditionally add
test_seq_prereq REFFILES
around the same place? Should $GIT_TEST_REFTABLE disable REFFILES? IOW, do you want the above to read
if test -n "$GIT_TEST_REFTABLE"
then
test_set_prereq REFTABLE
else
test_set_prereq REFFILES
fi
when both series are in effect?
Thanks.
Re: [PATCH v7 23/28] Reftable support for git-core
Quoted 13 lines · Junio C Hamano
t/test-lib.sh parisc* | hppa*) · line 1487
Quoted 4 lines
t/test-lib.sh +4
-0+if test -n "$GIT_TEST_REFTABLE" +then + test_set_prereq REFTABLE +fiHow would this interact with what the test prep series did which was to unconditionally add
test_seq_prereqREFFILESaround the same place? Should $GIT_TEST_REFTABLE disable REFFILES? IOW, do you want the above to read
Quoted 8 lines
if test -n "$GIT_TEST_REFTABLE"
then
test_set_prereqREFTABLE
else
test_set_prereqREFFILES
fiwhen both series are in effect?
Yes, but on 2nd thought it's probably better to stick with just REFFILES, and rewrite any reftable specific tests as !REFFILES.
Signature · 11 lines
Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Re: [PATCH v7 23/28] Reftable support for git-core
Quoted 9 lines · Han-Wen Nienhuys
Quoted 9 lines
IOW, do you want the above to read
if test -n "$GIT_TEST_REFTABLE"
then
test_set_prereqREFTABLE
else
test_set_prereqREFFILES
fi
Quoted 4 lines
Quoted 1 line
when both series are in effect?
Yes, but on 2nd thought it's probably better to stick with just REFFILES, and rewrite any reftable specific tests as !REFFILES.
I guess that is both good enough and simpler. The only possible downside is that it would be cumbersome if we ever need to support the third ref backend, but I expect that it won't happen before the reftable support solidifies, and under that assumption, the simpler the better.
Thanks.
Re: [PATCH v7 23/28] Reftable support for git-core
In reply to Han-Wen Nienhuys via GitGitGadgetQuoted 1 line · Han-Wen Nienhuys via GitGitGadget
From: Han-Wen Nienhuys <[email protected]>
[...snip...]> diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
Quoted 4 lines
new file mode 100644
index 000000000000..55d053e5ca65refs/reftable-backend.c new file +0
-0
[...snip...]
Quoted 31 lines
refs/reftable-backend.c new file +31
-0+static int write_transaction_table (struct reftable_writer *writer,void *arg) +{ +struct ref_transaction *transaction = (struct ref_transaction *)arg; +struct git_reftable_ref_store *refs = + (struct git_reftable_ref_store *)transaction->ref_store; +struct reftable_stack *stack = +stack_for (refs, transaction->updates[0 ]->refname); +uint64_t ts =reftable_stack_next_update_index (stack); +int err =0 ; +int i =0 ; +struct reftable_log_record *logs = +calloc (transaction->nr,sizeof (*logs)); +struct ref_update **sorted = +malloc (transaction->nr *sizeof (struct ref_update *)); +struct reftable_merged_table *mt =reftable_stack_merged_table (stack); +struct reftable_table tab = {NULL }; +struct reftable_ref_record ref = {NULL }; +reftable_table_from_merged_table (&tab, mt); +COPY_ARRAY (sorted, transaction->updates, transaction->nr); +QSORT (sorted, transaction->nr, ref_update_cmp); +reftable_writer_set_limits (writer, ts, ts); + +for (i =0 ; i < transaction->nr; i++) { +struct ref_update *u = sorted[i]; +struct reftable_log_record *log = &logs[i]; +struct object_id old_id; +fill_reftable_log_record (log); + log->update_index = ts; + log->value_type =REFTABLE_LOG_UPDATE ; + log->refname = (char *)u->refname; + log->update.new_hash = u->new_oid.hash;
refs/reftable-backend.c line 547
Quoted 10 lines
refs/reftable-backend.c new file +10
-0+ log->update.message = u->msg; + + err =reftable_table_read_ref (&tab, u->refname, &ref); +if (err <0 ) + goto done; +else if (err >0 ) { + old_id = null_oid; + }else { +oidread (&old_id, ref.value.val1); + }
This seems to assume that 'ref.value_type == REFTABLE_REF_VAL1' - but do we expect to have to handle the other types (REFTABLE_REF_VAL2/REFTABLE_REF_SYMREF)? When I run tests in seen against ASAN I see the following errors in t0031, which suggests we're running this code against REFTABLE_REF_SYMREF too - but I don't know if that means that this code should be able to handle the other ref types or if there's a bug higher up the stack. (AFAIUI REFTABLE_REF_DELETION is already handled because reftable_table_read_ref() already returns 1 for deletion, but the other cases seem valid?)
ASAN error output:
==25352==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000003353 at pc 0x000000499d17 bp 0x7fff0ea0a210 sp 0x7fff0ea099d8 READ of size 32 at 0x603000003353 thread T0
#0 0x499d16 in __asan_memcpy
../projects/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:22:3
#1 0x97ed0a in oidread hash.h:292:2
#2 0x97ed0a in write_transaction_table refs/reftable-backend.c:548:4
#3 0xb5537a in reftable_addition_add reftable/stack.c:650:8
#4 0x97b024 in git_reftable_transaction_finish
refs/reftable-backend.c:618:9
#5 0x95f9bf in ref_transaction_commit refs.c:2218:8
#6 0x9d3aa6 in update_head_with_reflog sequencer.c:1138:6
#7 0x52ffbf in cmd_commit builtin/commit.c:1814:6
#8 0x4ce8fe in run_builtin git.c:461:11
#9 0x4ccbc8 in handle_builtin git.c:718:3
#10 0x4cb0cc in run_argv git.c:785:4
#11 0x4cb0cc in cmd_main git.c:916:19
#12 0x6beded in main common-main.c:52:11
#13 0x7f415c762349 in __libc_start_main (/lib64/libc.so.6+0x24349)
#14 0x420769 in _start ../sysdeps/x86_64/start.S:1200x603000003353 is located 0 bytes to the right of 19-byte region [0x603000003340,0x603000003353) allocated by thread T0 here:
#0 0x4868b4 in strdup
../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
#1 0xaa08e8 in xstrdup wrapper.c:29:14
#2 0xb4b280 in reftable_ref_record_copy_from reftable/record.c:229:23
#3 0xb46754 in merged_iter_next_entry reftable/merged.c:132:2
#4 0xb46754 in merged_iter_next reftable/merged.c:141:13
#5 0xb46754 in merged_iter_next_void reftable/merged.c:157:9
#6 0xb500ae in iterator_next reftable/generic.c:147:9
#7 0xb500ae in reftable_iterator_next_ref reftable/generic.c:134:9
#8 0xb500ae in reftable_table_read_ref reftable/generic.c:46:8
#9 0x97ec67 in write_transaction_table refs/reftable-backend.c:542:9
#10 0xb5537a in reftable_addition_add reftable/stack.c:650:8
#11 0x97b024 in git_reftable_transaction_finish
refs/reftable-backend.c:618:9
#12 0x95f9bf in ref_transaction_commit refs.c:2218:8
#13 0x9d3aa6 in update_head_with_reflog sequencer.c:1138:6
#14 0x52ffbf in cmd_commit builtin/commit.c:1814:6
#15 0x4ce8fe in run_builtin git.c:461:11
#16 0x4ccbc8 in handle_builtin git.c:718:3
#17 0x4cb0cc in run_argv git.c:785:4
#18 0x4cb0cc in cmd_main git.c:916:19
#19 0x6beded in main common-main.c:52:11
#20 0x7f415c762349 in __libc_start_main (/lib64/libc.so.6+0x24349)SUMMARY: AddressSanitizer: heap-buffer-overflow ../projects/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:22:3 in __asan_memcpy
Produced using:
make CC=clang-11 SANITIZE=address COPTS="-Og -g" GIT_TEST_OPTS=-v
T=t0031-reftable.sh test
Quoted 48 lines
refs/reftable-backend.c new file +48
-0+ +/* XXX fold together with the old_id check below? */ + + log->update.old_hash = old_id.hash; +if (u->flags &REF_LOG_ONLY ) { +continue ; + } + +if (u->flags &REF_HAVE_NEW ) { +struct reftable_ref_record ref = {NULL }; +struct object_id peeled; + +int peel_error =peel_object (&u->new_oid, &peeled); + ref.refname = (char *)u->refname; + ref.update_index = ts; + +if (!peel_error) { + ref.value_type =REFTABLE_REF_VAL2 ; + ref.value.val2.target_value = peeled.hash; + ref.value.val2.value = u->new_oid.hash; + }else if (!is_null_oid (&u->new_oid)) { + ref.value_type =REFTABLE_REF_VAL1 ; + ref.value.val1 = u->new_oid.hash; + } + + err =reftable_writer_add_ref (writer, &ref); +if (err <0 ) { + goto done; + } + } + } + +for (i =0 ; i < transaction->nr; i++) { + err =reftable_writer_add_log (writer, &logs[i]); +clear_reftable_log_record (&logs[i]); +if (err <0 ) { + goto done; + } + } + +done: +assert (err !=REFTABLE_API_ERROR ); +reftable_ref_record_release (&ref); +free (logs); +free (sorted); +return err; +} +
[...snip...]
Re: [PATCH v7 23/28] Reftable support for git-core
Quoted 4 lines · Andrzej Hunt
refs/reftable-backend.c line 547
Quoted 3 lines
refs/reftable-backend.c new file +3
-0+ }else { +oidread (&old_id, ref.value.val1); + }
Quoted 9 lines
This seems to assume that 'ref.value_type ==
REFTABLE_REF_VAL1' - but do we expect to have to handle the other types (REFTABLE_REF_VAL2/REFTABLE_REF_SYMREF)? When I run tests in seen against ASAN I see the following errors in t0031, which suggests we're running this code againstREFTABLE_REF_SYMREFtoo - but I don't know if that means that this code should be able to handle the other ref types or if there's a bug higher up the stack. (AFAIUIREFTABLE_REF_DELETIONis already handled becausereftable_table_read_ref()already returns 1 for deletion, but the other cases seem valid?)
Curious. I didn't know it was supported to create symrefs in a transaction, but I've added an assert to make sure this doesn't trigger any sanitizers.
Signature · 11 lines
Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Re: [PATCH v7 23/28] Reftable support for git-core
Quoted 10 lines · Han-Wen Nienhuys
Quoted 9 lines
This seems to assume that 'ref.value_type ==
REFTABLE_REF_VAL1' - but do we expect to have to handle the other types (REFTABLE_REF_VAL2/REFTABLE_REF_SYMREF)? When I run tests in seen against ASAN I see the following errors in t0031, which suggests we're running this code againstREFTABLE_REF_SYMREFtoo - but I don't know if that means that this code should be able to handle the other ref types or if there's a bug higher up the stack. (AFAIUIREFTABLE_REF_DELETIONis already handled becausereftable_table_read_ref()already returns 1 for deletion, but the other cases seem valid?)
Quoted 3 lines
Curious. I didn't know it was supported to create symrefs in a transaction, but I've added an assert to make sure this doesn't trigger any sanitizers.
This happens on repo initialization, where we create HEAD and its referents at the same time.
I've fixed this provisionally for now in my series.
Signature · 11 lines
Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
PATCH v7 24/28 git-prompt: prepare for reftable refs backend
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: =?UTF-8?q?SZEDER=20G=C3=A1bor?= <[email protected]>
In our git-prompt script we strive to use Bash builtins wherever possible, because fork()-ing subshells for command substitutions and fork()+exec()-ing Git commands are expensive on some platforms. We even read and parse '.git/HEAD' using Bash builtins to get the name of the current branch [1]. However, the upcoming reftable refs backend won't use '.git/HEAD' at all, but will write an invalid refname as placeholder for backwards compatibility instead, which will break our git-prompt script.
Update the git-prompt script to recognize the placeholder '.git/HEAD' written by the reftable backend (its content is specified in the reftable specs), and then fall back to use 'git symbolic-ref' to get the name of the current branch.
[1] 3a43c4b5bd (bash prompt: use bash builtins to find out current
branch, 2011-03-31)
--- contrib/completion/git-prompt.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
contrib/completion/git-prompt.sh +6 -1
@@ -478,10 +478,15 @@ __git_ps1 () if ! __git_eread "$g/HEAD" head; then return $exit fi - # is it a symbolic ref? b="${head#ref: }" if [ "$head" = "$b" ]; then detached=yes + elif [ "$b" = "refs/heads/.invalid" ]; then + # Reftable + b="$(git symbolic-ref HEAD 2>/dev/null)" || + detached=yes + fi + if [ "$detached" = yes ]; then b="$( case "${GIT_PS1_DESCRIBE_STYLE-}" in (contains)
Signature · 1 line
gitgitgadget
PATCH v7 25/28 Add "test-tool dump-reftable" command.
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
This command dumps individual tables or a stack of of tables.
--- Makefile | 1 + t/helper/test-reftable.c | 5 +++++ t/helper/test-tool.c | 1 + t/helper/test-tool.h | 1 + 4 files changed, 8 insertions(+)
Makefile +1 -0
@@ -2427,6 +2427,7 @@ REFTABLE_OBJS += reftable/writer.o REFTABLE_TEST_OBJS += reftable/basics_test.o REFTABLE_TEST_OBJS += reftable/block_test.o +REFTABLE_TEST_OBJS += reftable/dump.o REFTABLE_TEST_OBJS += reftable/merged_test.o REFTABLE_TEST_OBJS += reftable/pq_test.o REFTABLE_TEST_OBJS += reftable/record_test.o
t/helper/test-reftable.c +5 -0
@@ -14,3 +14,8 @@ int cmd__reftable(int argc, const char **argv)tree_test_main (argc, argv);return 0 ; } + +int cmd__dump_reftable (int argc,const char **argv) +{ +return reftable_dump_main (argc, (char *const *)argv); +}
t/helper/test-tool.c +1 -0
@@ -58,6 +58,7 @@ static struct test_cmd cmds[] = { { "read-midx", cmd__read_midx }, { "ref-store", cmd__ref_store }, { "reftable", cmd__reftable }, + { "dump-reftable", cmd__dump_reftable }, { "regex", cmd__regex }, { "repository", cmd__repository }, { "revision-walking", cmd__revision_walking },
t/helper/test-tool.h +1 -0
@@ -19,6 +19,7 @@ int cmd__dump_cache_tree(int argc, const char **argv);int cmd__dump_fsmonitor (int argc,const char **argv);int cmd__dump_split_index (int argc,const char **argv);int cmd__dump_untracked_cache (int argc,const char **argv); +int cmd__dump_reftable (int argc,const char **argv);int cmd__example_decorate (int argc,const char **argv);int cmd__fast_rebase (int argc,const char **argv);int cmd__genrandom (int argc,const char **argv);
Signature · 1 line
gitgitgadget
PATCH v7 26/28 t1301: document what needs to be done for REFTABLE
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
--- t/t1301-shared-repo.sh | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
t/t1301-shared-repo.sh +5 -3
@@ -22,9 +22,10 @@ test_expect_success 'shared = 0400 (faulty permission u-w)' ' ) ' +# TODO(hanwen): for REFTABLE should inspect group-readable of .git/reftable/ for u in 002 022 do - test_expect_success POSIXPERM "shared=1 does not clear bits preset by umask $u" ' + test_expect_success REFFILES,POSIXPERM "shared=1 does not clear bits preset by umask $u" ' mkdir sub && ( cd sub && umask $u &&
@@ -114,7 +115,8 @@ test_expect_success POSIXPERM 'info/refs respects umask in unshared repo' ' test_cmp expect actual ' -test_expect_success POSIXPERM 'git reflog expire honors core.sharedRepository' ' +# For reftable, the check on .git/reftable/ is sufficient. +test_expect_success REFFILES,POSIXPERM 'git reflog expire honors core.sharedRepository' ' umask 077 && git config core.sharedRepository group && git reflog expire --all &&
@@ -201,7 +203,7 @@ test_expect_success POSIXPERM 're-init respects core.sharedrepository (remote)' test_cmp expect actual ' -test_expect_success POSIXPERM 'template can set core.sharedrepository' ' +test_expect_success REFFILES,POSIXPERM 'template can set core.sharedrepository' ' rm -rf child.git && umask 0022 && git config core.sharedrepository 0666 &&
Signature · 1 line
gitgitgadget
PATCH v7 27/28 t1401,t2011: parameterize HEAD.lock for REFTABLE
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
--- t/t1401-symbolic-ref.sh | 11 +++++++++-- t/t2011-checkout-invalid-head.sh | 11 +++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-)
t/t1401-symbolic-ref.sh +9 -2
@@ -99,9 +99,16 @@ test_expect_success LONG_REF 'we can parse long symbolic ref' ' test_cmp expect actual ' +if test_have_prereq REFTABLE +then + HEAD_LOCK=reftable/tables.list.lock +else + HEAD_LOCK=HEAD.lock +fi + test_expect_success 'symbolic-ref reports failure in exit code' ' - test_when_finished "rm -f .git/HEAD.lock" && - >.git/HEAD.lock && + test_when_finished "rm -f .git/$HEAD_LOCK" && + >.git/$HEAD_LOCK && test_must_fail git symbolic-ref HEAD refs/heads/whatever '
t/t2011-checkout-invalid-head.sh +9 -2
@@ -22,9 +22,16 @@ test_expect_success 'checkout main from invalid HEAD' ' git checkout main -- ' +if test_have_prereq REFTABLE +then + HEAD_LOCK=reftable/tables.list.lock +else + HEAD_LOCK=HEAD.lock +fi + test_expect_success 'checkout notices failure to lock HEAD' ' - test_when_finished "rm -f .git/HEAD.lock" && - >.git/HEAD.lock && + test_when_finished "rm -f .git/$HEAD_LOCK" && + >.git/$HEAD_LOCK && test_must_fail git checkout -b other '
Signature · 1 line
gitgitgadget
PATCH v7 28/28 t1404: annotate test cases with REFFILES
In reply to Han-Wen Nienhuys via GitGitGadgetFrom: Han-Wen Nienhuys <[email protected]>
* Reftable for now lacks detailed error messages for directory/file conflicts. Skip message comparisons.
* Mark tests that muck with .git directly as REFFILES.
--- t/t1404-update-ref-errors.sh | 86 ++++++++++++++++++++++++------------ 1 file changed, 57 insertions(+), 29 deletions(-)
t/t1404-update-ref-errors.sh +57 -29
@@ -27,7 +27,9 @@ test_update_rejected () { fi && printf "create $prefix/%s $C\n" $create >input && test_must_fail git update-ref --stdin <input 2>output.err && - test_i18ngrep -F "$error" output.err && + if test_have_prereq REFFILES ; then + test_i18ngrep -F "$error" output.err + fi && git for-each-ref $prefix >actual && test_cmp unchanged actual }
@@ -101,7 +103,9 @@ df_test() { printf "%s\n" "delete $delname" "create $addname $D" fi >commands && test_must_fail git update-ref --stdin <commands 2>output.err && - test_cmp expected-err output.err && + if test_have_prereq REFFILES ; then + test_cmp expected-err output.err + fi && printf "%s\n" "$C $delref" >expected-refs && git for-each-ref --format="%(objectname) %(refname)" $prefix/r >actual-refs && test_cmp expected-refs actual-refs
@@ -189,7 +193,7 @@ test_expect_success 'one new ref is a simple prefix of another' ' ' -test_expect_success 'empty directory should not fool rev-parse' ' +test_expect_success REFFILES 'empty directory should not fool rev-parse' ' prefix=refs/e-rev-parse && git update-ref $prefix/foo $C && git pack-refs --all &&
@@ -199,7 +203,7 @@ test_expect_success 'empty directory should not fool rev-parse' ' test_cmp expected actual ' -test_expect_success 'empty directory should not fool for-each-ref' ' +test_expect_success REFFILES 'empty directory should not fool for-each-ref' ' prefix=refs/e-for-each-ref && git update-ref $prefix/foo $C && git for-each-ref $prefix >expected &&
@@ -209,14 +213,14 @@ test_expect_success 'empty directory should not fool for-each-ref' ' test_cmp expected actual ' -test_expect_success 'empty directory should not fool create' ' +test_expect_success REFFILES 'empty directory should not fool create' ' prefix=refs/e-create && mkdir -p .git/$prefix/foo/bar/baz && printf "create %s $C\n" $prefix/foo | git update-ref --stdin ' -test_expect_success 'empty directory should not fool verify' ' +test_expect_success REFFILES 'empty directory should not fool verify' ' prefix=refs/e-verify && git update-ref $prefix/foo $C && git pack-refs --all &&
@@ -225,7 +229,7 @@ test_expect_success 'empty directory should not fool verify' ' git update-ref --stdin ' -test_expect_success 'empty directory should not fool 1-arg update' ' +test_expect_success REFFILES 'empty directory should not fool 1-arg update' ' prefix=refs/e-update-1 && git update-ref $prefix/foo $C && git pack-refs --all &&
@@ -234,7 +238,7 @@ test_expect_success 'empty directory should not fool 1-arg update' ' git update-ref --stdin ' -test_expect_success 'empty directory should not fool 2-arg update' ' +test_expect_success REFFILES 'empty directory should not fool 2-arg update' ' prefix=refs/e-update-2 && git update-ref $prefix/foo $C && git pack-refs --all &&
@@ -243,7 +247,7 @@ test_expect_success 'empty directory should not fool 2-arg update' ' git update-ref --stdin ' -test_expect_success 'empty directory should not fool 0-arg delete' ' +test_expect_success REFFILES 'empty directory should not fool 0-arg delete' ' prefix=refs/e-delete-0 && git update-ref $prefix/foo $C && git pack-refs --all &&
@@ -252,7 +256,7 @@ test_expect_success 'empty directory should not fool 0-arg delete' ' git update-ref --stdin ' -test_expect_success 'empty directory should not fool 1-arg delete' ' +test_expect_success REFFILES 'empty directory should not fool 1-arg delete' ' prefix=refs/e-delete-1 && git update-ref $prefix/foo $C && git pack-refs --all &&
@@ -336,7 +340,9 @@ test_expect_success 'missing old value blocks update' ' EOF printf "%s\n" "update $prefix/foo $E $D" | test_must_fail git update-ref --stdin 2>output.err && - test_cmp expected output.err + if test_have_prereq REFFILES ; then + test_cmp expected output.err + fi ' test_expect_success 'incorrect old value blocks update' '
@@ -347,7 +353,9 @@ test_expect_success 'incorrect old value blocks update' ' EOF printf "%s\n" "update $prefix/foo $E $D" | test_must_fail git update-ref --stdin 2>output.err && - test_cmp expected output.err + if test_have_prereq REFFILES ; then + test_cmp expected output.err + fi ' test_expect_success 'existing old value blocks create' '
@@ -358,7 +366,9 @@ test_expect_success 'existing old value blocks create' ' EOF printf "%s\n" "create $prefix/foo $E" | test_must_fail git update-ref --stdin 2>output.err && - test_cmp expected output.err + if test_have_prereq REFFILES ; then + test_cmp expected output.err + fi ' test_expect_success 'incorrect old value blocks delete' '
@@ -369,7 +379,9 @@ test_expect_success 'incorrect old value blocks delete' ' EOF printf "%s\n" "delete $prefix/foo $D" | test_must_fail git update-ref --stdin 2>output.err && - test_cmp expected output.err + if test_have_prereq REFFILES ; then + test_cmp expected output.err + fi ' test_expect_success 'missing old value blocks indirect update' '
@@ -380,7 +392,9 @@ test_expect_success 'missing old value blocks indirect update' ' EOF printf "%s\n" "update $prefix/symref $E $D" | test_must_fail git update-ref --stdin 2>output.err && - test_cmp expected output.err + if test_have_prereq REFFILES ; then + test_cmp expected output.err + fi ' test_expect_success 'incorrect old value blocks indirect update' '
@@ -392,7 +406,9 @@ test_expect_success 'incorrect old value blocks indirect update' ' EOF printf "%s\n" "update $prefix/symref $E $D" | test_must_fail git update-ref --stdin 2>output.err && - test_cmp expected output.err + if test_have_prereq REFFILES ; then + test_cmp expected output.err + fi ' test_expect_success 'existing old value blocks indirect create' '
@@ -404,7 +420,9 @@ test_expect_success 'existing old value blocks indirect create' ' EOF printf "%s\n" "create $prefix/symref $E" | test_must_fail git update-ref --stdin 2>output.err && - test_cmp expected output.err + if test_have_prereq REFFILES ; then + test_cmp expected output.err + fi ' test_expect_success 'incorrect old value blocks indirect delete' '
@@ -416,7 +434,9 @@ test_expect_success 'incorrect old value blocks indirect delete' ' EOF printf "%s\n" "delete $prefix/symref $D" | test_must_fail git update-ref --stdin 2>output.err && - test_cmp expected output.err + if test_have_prereq REFFILES ; then + test_cmp expected output.err + fi ' test_expect_success 'missing old value blocks indirect no-deref update' '
@@ -427,7 +447,9 @@ test_expect_success 'missing old value blocks indirect no-deref update' ' EOF printf "%s\n" "option no-deref" "update $prefix/symref $E $D" | test_must_fail git update-ref --stdin 2>output.err && - test_cmp expected output.err + if test_have_prereq REFFILES ; then + test_cmp expected output.err + fi ' test_expect_success 'incorrect old value blocks indirect no-deref update' '
@@ -439,7 +461,9 @@ test_expect_success 'incorrect old value blocks indirect no-deref update' ' EOF printf "%s\n" "option no-deref" "update $prefix/symref $E $D" | test_must_fail git update-ref --stdin 2>output.err && - test_cmp expected output.err + if test_have_prereq REFFILES ; then + test_cmp expected output.err + fi ' test_expect_success 'existing old value blocks indirect no-deref create' '
@@ -451,7 +475,9 @@ test_expect_success 'existing old value blocks indirect no-deref create' ' EOF printf "%s\n" "option no-deref" "create $prefix/symref $E" | test_must_fail git update-ref --stdin 2>output.err && - test_cmp expected output.err + if test_have_prereq REFFILES ; then + test_cmp expected output.err + fi ' test_expect_success 'incorrect old value blocks indirect no-deref delete' '
@@ -463,10 +489,12 @@ test_expect_success 'incorrect old value blocks indirect no-deref delete' ' EOF printf "%s\n" "option no-deref" "delete $prefix/symref $D" | test_must_fail git update-ref --stdin 2>output.err && - test_cmp expected output.err + if test_have_prereq REFFILES ; then + test_cmp expected output.err + fi ' -test_expect_success 'non-empty directory blocks create' ' +test_expect_success REFFILES 'non-empty directory blocks create' ' prefix=refs/ne-create && mkdir -p .git/$prefix/foo/bar && : >.git/$prefix/foo/bar/baz.lock &&
@@ -485,7 +513,7 @@ test_expect_success 'non-empty directory blocks create' ' test_cmp expected output.err ' -test_expect_success 'broken reference blocks create' ' +test_expect_success REFFILES 'broken reference blocks create' ' prefix=refs/broken-create && mkdir -p .git/$prefix && echo "gobbledigook" >.git/$prefix/foo &&
@@ -504,7 +532,7 @@ test_expect_success 'broken reference blocks create' ' test_cmp expected output.err ' -test_expect_success 'non-empty directory blocks indirect create' ' +test_expect_success REFFILES 'non-empty directory blocks indirect create' ' prefix=refs/ne-indirect-create && git symbolic-ref $prefix/symref $prefix/foo && mkdir -p .git/$prefix/foo/bar &&
@@ -524,7 +552,7 @@ test_expect_success 'non-empty directory blocks indirect create' ' test_cmp expected output.err ' -test_expect_success 'broken reference blocks indirect create' ' +test_expect_success REFFILES 'broken reference blocks indirect create' ' prefix=refs/broken-indirect-create && git symbolic-ref $prefix/symref $prefix/foo && echo "gobbledigook" >.git/$prefix/foo &&
@@ -543,7 +571,7 @@ test_expect_success 'broken reference blocks indirect create' ' test_cmp expected output.err ' -test_expect_success 'no bogus intermediate values during delete' ' +test_expect_success REFFILES 'no bogus intermediate values during delete' ' prefix=refs/slow-transaction && # Set up a reference with differing loose and packed versions: git update-ref $prefix/foo $C &&
@@ -600,7 +628,7 @@ test_expect_success 'no bogus intermediate values during delete' ' test_must_fail git rev-parse --verify --quiet $prefix/foo ' -test_expect_success 'delete fails cleanly if packed-refs file is locked' ' +test_expect_success REFFILES 'delete fails cleanly if packed-refs file is locked' ' prefix=refs/locked-packed-refs && # Set up a reference with differing loose and packed versions: git update-ref $prefix/foo $C &&
@@ -616,7 +644,7 @@ test_expect_success 'delete fails cleanly if packed-refs file is locked' ' test_cmp unchanged actual ' -test_expect_success 'delete fails cleanly if packed-refs.new write fails' ' +test_expect_success REFFILES 'delete fails cleanly if packed-refs.new write fails' ' # Setup and expectations are similar to the test above. prefix=refs/failed-packed-refs && git update-ref $prefix/foo $C &&
Signature · 1 line
gitgitgadget
Quoted 2 lines · Han-Wen Nienhuys via GitGitGadget
The commits up to "
hash.h: provide constants for the hash IDs" should be good to merge to 'next'.
With how Junio queues things up perhaps submitting this as another "prep" series would be good, to reduce future reviewer fatigue, i.e. anything we can make land on master makes re-rolls that much smaller.
Quoted 2 lines
There are several test fixups, but I've put them in another series because GGG enforces max 30 commits.
I left a bunch of comments on the test prep series now. Probably good to have it split up regardless of GGG limits.
Re the comments I left on the test series. I'm very happy to see the start of addressing the "it must be tested" concerns I had in
https://lore.kernel.org/git/[email protected]/
But running the test suite with GIT_TEST_REFTABLE=true still yields a wall of failed tests. I tested with a merger of this and pr-git-1008/hanwen/reffiles-v1, i.e. what's on "seen" currently.
I don't see the point of having re-rolls of this topic while the test changes topic it's based on hasn't finished marking/splitting/refactoring the various tests that do and don't depend on the file backend.
At least when I review it I'm just left with going in circles digging into one of those failing test, figuring out if it's really refs/files-backend.c specific or not etc., and as long as we can't turn on GIT_TEST_REFTABLE=true in CI as part of this series I don't see a path to making it advance to next->master.
Quoted 2 lines
Due to using uncompress2, this build fails on the Linux32 builder; what is the magic incantation to run autoconf?
Doesn't this just need a NO_UNCOMPRESS2 for Linux32 in ci/lib.sh? See the lines just below that for e.g. NO_REGEX for another CI target.
Quoted 7 lines · Ævar Arnfjörð Bjarmason
Quoted 2 lines
The commits up to "
hash.h: provide constants for the hash IDs" should be good to merge to 'next'.With how Junio queues things up perhaps submitting this as another "prep" series would be good, to reduce future reviewer fatigue, i.e. anything we can make land on master makes re-rolls that much smaller.
will do.
Quoted 9 lines
Quoted 2 lines
There are several test fixups, but I've put them in another series because GGG enforces max 30 commits.
I left a bunch of comments on the test prep series now. Probably good to have it split up regardless of GGG limits.
Re the comments I left on the test series. I'm very happy to see the start of addressing the "it must be tested" concerns I had in
https://lore.kernel.org/git/[email protected]/
It may look like the start, but I've been improving the number of tests that pass continuously since I posted the first version of this code over a year ago.
Quoted 10 lines
I don't see the point of having re-rolls of this topic while the test changes topic it's based on hasn't finished marking/splitting/refactoring the various tests that do and don't depend on the file backend.
At least when I review it I'm just left with going in circles digging into one of those failing test, figuring out if it's really
refs/files-backend.cspecific or not etc., and as long as we can't turn on GIT_TEST_REFTABLE=true in CI as part of this series I don't see a path to making it advance tonext->master.
The point of posting updates is to garner feedback, especially from people familiar with the Git code itself. If you would like this effort to move forward faster, I am most in need of review for the part that glues the library together with the git code itself (ie. the commit introducing refs/reftable-backend.c).
Quoted 5 lines
Quoted 2 lines
Due to using uncompress2, this build fails on the Linux32 builder; what is the magic incantation to run autoconf?
Doesn't this just need a
NO_UNCOMPRESS2for Linux32 inci/lib.sh? See the lines just below that for e.g.NO_REGEXfor another CI target.
Thanks!
Signature · 11 lines
Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Quoted 18 lines · Han-Wen Nienhuys
Quoted 7 lines · Ævar Arnfjörð Bjarmason
Quoted 2 lines
The commits up to "
hash.h: provide constants for the hash IDs" should be good to merge to 'next'.With how Junio queues things up perhaps submitting this as another "prep" series would be good, to reduce future reviewer fatigue, i.e. anything we can make land on master makes re-rolls that much smaller.
will do.
Quoted 6 lines
Quoted 2 lines
There are several test fixups, but I've put them in another series because GGG enforces max 30 commits.
I left a bunch of comments on the test prep series now. Probably good to have it split up regardless of GGG limits.
Quoted 7 lines
Quoted 3 lines
Re the comments I left on the test series. I'm very happy to see the start of addressing the "it must be tested" concerns I had in
https://lore.kernel.org/git/[email protected]/It may look like the start, but I've been improving the number of tests that pass continuously since I posted the first version of this code over a year ago.
FWIW I meant or meant to say something closer to "a start at the numerous failures I noted in the v6 discussion", not "no work has been done at all on this front". Sorry.
Quoted 5 lines
Quoted 5 lines
I don't see the point of having re-rolls of this topic while the test changes topic it's based on hasn't finished marking/splitting/refactoring the various tests that do and don't depend on the file backend.
Quoted 8 lines
Quoted 5 lines
At least when I review it I'm just left with going in circles digging into one of those failing test, figuring out if it's really
refs/files-backend.cspecific or not etc., and as long as we can't turn on GIT_TEST_REFTABLE=true in CI as part of this series I don't see a path to making it advance tonext->master.The point of posting updates is to garner feedback, especially from people familiar with the Git code itself.
So you agree that we should make the tests pass first, then shouldn't these me marked as RFC/PATCH?
I for one would find that a lot less confusing, "PATCH" means "the author considers this ready to land on master sans undiscovered bugs etc.
Quoted 1 line
If you would like this effort to move forward faster,
Yes, I'm keen to help move it forward.
I am saying that I think for me or anyone else to do that in any sensible way the path forward is to make the tests pass with GIT_TEST_REFTABLE=true.
IOW a large part of the feedback you're looking for is already part of the codebase. Nobody can keep all of it in their head, but we've encoded all the tricky edge cases we could think of in the tests.
So in particular per my feedback on the test series: It's only when we start digging into those tests that we discover the interesting bits, i.e. how things like .git/SOME_FILE behaves per[1], and my comments about reflog behavior in [2].
Anyway, I think I'm just repeating myself at this point, but part of the reason is that I don't think I've gotten a straight answer about the point-by-point questions I had in [3].
I.e. reaching some consensus on things like whether GIT_TEST_REFTABLE=true passing under CI being a hard-prereq for this series or not is surely one of the first things to sketch out before figuring out how to move this forward.
Also, with how unhappy Junio is with my patch-dumping I'm probably the last person to give advice about managing mailing list attention, but
still: here's an attempt:
In v6 I noted that t5510-fetch.sh had a segfault[4], you said you'd check it out, and reading your cover letter nothing stood out about that, so I assumed it was sorted out somehow.
But running it now yields a BUG() instead:
BUG: refs.c:1038: free called on a prepared reference transaction
Aborted
[...]
not ok 18 - fetch --atomic aborts all reference updates if hook abortsAnd trying the whole test suite with --verbose-log yields:
$ grep -e 'Segmentation fault' -e BUG: test-results/*
test-results/t0210-trace2-normal.out:BUG: t/helper/test-trace2.c:206: the bug message
test-results/t1400-update-ref.out:BUG: refs.c:1038: free called on a prepared reference transaction
test-results/t1400-update-ref.out:BUG: refs.c:1038: free called on a prepared reference transaction
test-results/t4058-diff-duplicates.out:Segmentation fault
test-results/t4058-diff-duplicates.out:Segmentation fault
test-results/t4058-diff-duplicates.out:Segmentation fault
test-results/t4058-diff-duplicates.out:Segmentation fault
test-results/t5510-fetch.out:BUG: refs.c:1038: free called on a prepared reference transaction
test-results/t5510-fetch.out:BUG: refs.c:1038: free called on a prepared reference transaction
test-results/t5600-clone-fail-cleanup.out:Segmentation fault
test-results/t5600-clone-fail-cleanup.out:Segmentation fault
test-results/t5600-clone-fail-cleanup.out:Segmentation fault
test-results/t5601-clone.out:Segmentation fault
test-results/t6423-merge-rename-directories.out: test_i18ngrep ! BUG: err &&Not all of that is yours, FWIW "seen" is currently doing, and the "diff" failures are ac14de13b2 (t4058: explore duplicate tree entry handling in a bit more detail, 2020-12-11).
$ grep -e 'Segmentation fault' -e BUG: test-results/*
test-results/t0210-trace2-normal.out:BUG: t/helper/test-trace2.c:206: the bug message
test-results/t1406-submodule-ref-store.out:BUG: refs/files-backend.c:139: operation pack_refs requires abilities 0x6, but only have 0x5
test-results/t1406-submodule-ref-store.out:BUG: refs/files-backend.c:139: operation create_symref requires abilities 0x2, but only have 0x5
test-results/t1406-submodule-ref-store.out:BUG: refs/files-backend.c:139: operation delete_refs requires abilities 0x2, but only have 0x5
test-results/t1406-submodule-ref-store.out:BUG: refs/files-backend.c:139: operation rename_ref requires abilities 0x2, but only have 0x5
test-results/t1406-submodule-ref-store.out:BUG: refs/files-backend.c:139: operation delete_reflog requires abilities 0x2, but only have 0x5
test-results/t1406-submodule-ref-store.out:BUG: refs/files-backend.c:139: operation create_reflog requires abilities 0x2, but only have 0x5
test-results/t4058-diff-duplicates.out:Segmentation fault
test-results/t4058-diff-duplicates.out:Segmentation fault
test-results/t4058-diff-duplicates.out:Segmentation fault
test-results/t4058-diff-duplicates.out:Segmentation fault
test-results/t6423-merge-rename-directories.out: test_i18ngrep ! BUG: err &&Anyway, the "clone"/"fetch"/"update-ref" failures look new with GIT_TEST_REFTABLE=true.
So to close up the attempt at getting feedback: I think we'd probably be better off still discussing "I tried this to fix the segfault, but now I get this BUG" rather than a 30 patch re-roll.
Quoted 3 lines
I am most in need of review for the part that glues the library
together with the git code itself (ie. the commit introducingrefs/reftable-backend.c).
In other and briefer words I don't think this series is in need of review at this point, it's in need of *addressing* review, where review is both the ghosts from the past of failing tests, and outstanding segfaults/BUG()s being hit.
1. https://lore.kernel.org/git/[email protected]/
2. https://lore.kernel.org/git/[email protected]/
3. https://lore.kernel.org/git/[email protected]/
4. https://lore.kernel.org/git/[email protected]/
Quoted 11 lines · Ævar Arnfjörð Bjarmason
Quoted 11 lines
Quoted 10 lines
I don't see the point of having re-rolls of this topic while the test changes topic it's based on hasn't finished marking/splitting/refactoring the various tests that do and don't depend on the file backend.
At least when I review it I'm just left with going in circles digging into one of those failing test, figuring out if it's really
refs/files-backend.cspecific or not etc., and as long as we can't turn on GIT_TEST_REFTABLE=true in CI as part of this series I don't see a path to making it advance tonext->master.
Quoted 5 lines
Quoted 2 lines
The point of posting updates is to garner feedback, especially from people familiar with the Git code itself.
So you agree that we should make the tests pass first, then shouldn't these me marked as RFC/PATCH?
I added RFC to the patch introducing reftable-backend.c
I consider the code under reftable/*.c to be of production quality, even if it's stylistically different from the git code.
Quoted 3 lines
IOW a large part of the feedback you're looking for is already part of the codebase. Nobody can keep all of it in their head, but we've encoded all the tricky edge cases we could think of in the tests.
That's disappointing. I know that I can just fix test failures until there are none left, but it's not a terribly efficient method if there is someone who knows how this works. Is there no other way to solicit feedback?
Quoted 4 lines
I.e. reaching some consensus on things like whether GIT_TEST_REFTABLE=true passing under CI being a hard-prereq for this series or not is surely one of the first things to sketch out before figuring out how to move this forward.
I've had conflicting feedback about this, but it's ultimately not my call to make. Jonathan Nieder suggested that this was asking too much, and it would be OK to have a credible starting point where the effort to address remaining test failures could be shared across multiple people. Jun also expressed he didn't want to put all the work on my shoulders, but so far I've not received much help.
Quoted 3 lines
In v6 I noted that
t5510-fetch.shhad a segfault[4], you said you'd check it out, and reading your cover letter nothing stood out about that, so I assumed it was sorted out somehow.
I fixed a number of segfaults, so it's quite likely (I did see the freed transaction BUG).
One of the problems of working with a large pending series like this, is that it is a project in itself, and keeping track of which change fixes which bug is something I really need version control for, but the frequent rebasing/squashing erases this kind of information.
Signature · 11 lines
Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado