Re: [RFC PATCH 1/3] promised-blob, fsck: introduce promised blobs
- From
- Jonathan Tan <jonathantanmy@google.com>
- Date
- Jul 19, 2017, 23:37 UTC
- Message-ID
- <20170719163713.2d01f6e7@twelve2.svl.corp.google.com>
- In-Reply-To
- <CAGZ79kZ8i0Xqp2Yft19JKzPFMUdbzEDNJ01zER9RoM8ZzKPu8A@mail.gmail.com>
On Tue, 11 Jul 2017 15:02:09 -0700 Stefan Beller <sbeller@google.com> wrote:
Show 22 quoted lines
> 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.
Show 10 quoted lines
> 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.
Show 6 quoted lines
> > + 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).