Improve git save format

Dirk Hohndel dirk at hohndel.org
Fri Mar 7 16:55:39 PST 2014


On Fri, 2014-03-07 at 15:16 -0800, Linus Torvalds wrote:
> Ok, I promised to flesh out the writing part by today, and here it is.
> 
> The first patch is just a trivial helper, the second patch is the
> "make the git save format actually save everything in a good format".
> 
> The second patch does a lot of different things, and in theory it
> could be many smaller patches, but quite frankly, the history for that
> patch is extremely messy, and any split-ups would be rather
> artificial. There's some trivial fixes that could have been done
> separately, but they aren't really interesting, and the bigger pieces
> are all tied together.
> 
> For example, it now generates the dive computer data in separate
> files, and that not only cause some new helper functions on the git
> blob writing side, but also causes the indentation of the dive
> computer data to go away. The parts could be separated out as patches,
> but it wouldn't make much sense.
> 
> So it's just one big "flesh out the write format" patch, but the good
> news is that I think the write format is now in good enough shape that
> future changes will be much more targeted. The file saves look really
> quite nice now, but I do suspect that as I start reading things back,
> I will notice some bugs or just plain omissions.

OK, after reading it three times I believe you that it was just not
worth separating it out into smaller patches. :-)

I have a few questions.

One thing I notice is that our put_<some term>() functions (so
put_temperature() or put_pressure(), but not put_milli()) quite
carefully return whether something was written or not (they are all of
type int and return 0 or 1).

Yet I don't see any code that would consume that information. In all my
grepping it seems to me that we always call them as if they were void
(just like put_milli()).
What am I missing?

I'm a bit unclear about when you append the shortened SHA to a tree node
name... or more specifically, when you DON'T. And why not use the
standard 12 digits of the SHA? I'm guessing you think no one will ever
have enough nodes to make this worth while?

Why does 

static int tree_insert(git_repository *repo, git_treebuilder *dir,
	const char *name, int mkunique, git_oid *id, unsigned mode)

have a 'repo' argument that isn't used at all?


> I'll start on the reading parts tomorrow.

Cool - that will make it SO MUCH more useful (but I'll admit that I love
poking around in the structures it stores right now :-) )

/D



More information about the subsurface mailing list