Hi Stefan.
Compared with the extent of what you have achieved, this is a minor thing, so I don't want to go on about it much.
However the point about readable code is not just whether it will need to be changed within the lifetime of "format 7" deployments, but about whether other software engineers have the reasonable ability to read the code, take it apart and put it together "in new and interesting ways". That's the part that's difficult here: if I want to experiment with "format 8" changes here, I would have a hard time putting this section of code back together.
Instructions how to run some external tool(s) to generate this constant would be sufficient to address my concern.
FWIW I think your logical addressing work is excellent and I am really looking forward to having it available.
- Julian
Stefan Fuhrmann wrote:
> 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.
Received on 2014-06-25 20:29:29 CEST