On Tue, 11 Jul 2017 15:02:09 -0700 Stefan Beller <sbel...@google.com> wrote:
> Here I wondered what this file looks like, in a later patch you > add documentation: > > +objects/promisedblob:: > + This file records the sha1 object names and sizes of promised > + blobs. > + > > but that leaves more fundamental questions: > * Is that a list of sha1s, separated by LF? (CRLF? How would Windows > users interact with it, are they supposed to ever modify this file?) > * Similar to .git/packed-refs, would we want to have a first line > that has some sort of comment? > * In the first question I assumed a linear list, will that be a sorted list, > or do we want to have some fancy data structure here? > (critbit tree, bloom filter) > * is the contents in ASCII or binary? (What are the separators?) > * In the future I presume we'd want to quickly answer "Is X in the > promised blobs list?" so would it be possible (in the future) to > improve the searching e.g. binary search? > * ... I'll read on to see my questions answered, but I'd guess > others would prefer to see it in the docs. :) I'll write more documentation once the file format is finalized. At the time this patch was written, this was a sorted binary file, where each entry consists of a 20-byte SHA-1 and an 8-byte size. Now each entry is a 20-byte SHA-1, a 1-byte type, and an 8-byte size (I will send this patch out soon). > Similar to other files, would we want to prefix the file with > a 4 letter magic number and a version number? That's a good idea. > Later down the road, do we want to have a > (plumbing) command to move an object from > standard blob to promised blob (as of now I'd think > it would perform this rm call as well as an insert into > the promised blobs file) ? > (Well, we'd also have to think about how to get objects > out of a pack) > > With such a command you can easily write your own custom > filter to free up blobs again. This sounds reasonable, but probably not in the initial set of patches. > > + test_must_fail git -C repo fsck > > test_i18n_grep missing out ? > > maybe, too? (Maybe that is already tested elsewhere, > so no need for it) This test is just meant to show that configuration can make fsck pass, not the existing behavior of fsck when it fails (which is tested elsewhere - I guess that was your question).
