On Wed, Jun 25, 2014 at 6:19 PM, Julian Foad <julianfoad_at_btopenworld.com>
wrote:
> Stefan Fuhrmann wrote:
>
> > Ivan Zhakov wrote:
> >> @@ -1449,15 +1449,35 @@ write_revision_zero(svn_fs_t *fs)
> >>>+ SVN_ERR(svn_io_file_create_binary(path_revision_zero,
> >>>+ "PLAIN\nEND\nENDREP\n"
> >>>+ "id: 0.0.r0/2\n"
> >>>+ "type: dir\n"
> >>>+ "count: 0\n"
> >>>+ "text: 0 3 4 4 "
> >>>+ "2d2977d1c96f487abe4a1e202dd03b4e\n"
> >>>+ "cpath: /\n"
> >>>+ "\n\n"
> >>>+
> >>>+ /* L2P index */
> >>>+ "\0\x80\x40" /* rev 0, 8k entries per page
> */
> >>>+ "\1\1\1" /* 1 rev, 1 page, 1 page in
> 1st rev */
> >>>+ "\6\4" /* page size: bytes, count */
> >>>+ "\0\xd6\1\xb1\1\x21" /* phys offsets + 1 */
> >>>+
> >>>+ /* P2L index */
> >>>+ "\0\x6b" /* start rev, rev file size */
> >>>+ "\x80\x80\4\1\x1D" /* 64k pages, 1 page using 29
> bytes */
> >>>+ "\0" /* offset entry 0 page 1 */
> >>>+ /* len, item & type, rev,
> checksum */
> >>>+ "\x11\x34\0\xf5\xd6\x8c\x81\x06"
> >>>+ "\x59\x09\0\xc8\xfc\xf6\x81\x04"
> >>>+ "\1\x0d\0\x9d\x9e\xa9\x94\x0f"
> >>>+ "\x95\xff\3\x1b\0\0" /* last entry fills up 64k
> page */
> >>>+
> >>>+ /* Footer */
> >>>+ "107 121\7",
> >>>+ 107 + 14 + 38 + 7 + 1, fs->pool));
> >>
> >> This constant makes code unreadable, unmaintainable and very error
> prone.
> >
> > How?
>
> "How?" Really? REALLY? I don't know if Ivan was referring to the numeric
> constant or the string constant or both, but I take issue with the string
> constant.
In fact, "really" ;).
What Ivan is certainly aware of as he claims to know FSFS
very well but you may not is, that this particular constant
describes a template with API-like guarantees. I.e. once
released, it must never, ever change. If it had to change
for some future extension, an independent variant must
be created and both currently available once be retained.
So, Ivan made two claims:
1. Unreadable.
I tried to give a rough "translation" or "orientation"
in the comments. The details can be found in the
FSFS spec. If you want to see "plain" numbers, run
'svnfsfs dump-index'.
2. Unmaintainable & error prone.
That makes only sense if we are talking change
scenarios (maybe as part of a bug fix). The only
way there could be a change to that template is
if it so broken that we cannot use it under any
circumstance. Otherwise, once released, it is set
in stone.
I wrote the template by hand and changed it by hand,
not fun but doable. What it learned is that even the
slightest mistake will cause the test suite to fail and
in many case just terminate the server tests.
As this is the "empty revision 0", there is little gray
area between "all contents (the root node) can be
read plus verified" and "virtually nothing works". So,
the only change scenario that I can see here is a
change in the index structure before release.
Ivan seems to have other scenarios in mind to (potentially)
change that code that I must obviously miss. Therefore:
"How?"
> Among everything you've written do you really have no code that could help
> to generate and write the indexes without having to spell it all out
> bit-for-bit by hand? I won't believe it if you say "no".
>
There is svnfsfs now.
>
> [...]
>
> > Changing the rev 0 template has never been a fun
> > activity and wont become one in the foreseeable
> > future.
>
> In 1.8.x it looked like this:
>
> SVN_ERR(svn_io_file_create(path_revision_zero,
> "PLAIN\nEND\nENDREP\n"
> "id: 0.0.r0/17\n"
> "type: dir\n"
> "count: 0\n"
> "text: 0 0 4 4 "
> "2d2977d1c96f487abe4a1e202dd03b4e\n"
> "cpath: /\n"
> "\n\n17 107\n", fs->pool));
>
>
> Yes it contained a few magic constants, some of which probably should have
> been generated, but still it was *much* simpler.
>
It is still there and has been unchanged since its
original release in 1.1. And that is my point.
-- Stefan^2.
Received on 2014-06-25 19:15:10 CEST