Re: svn commit: r1604421 - in /subversion/trunk: subversion/libsvn_fs_fs/ subversion/tests/cmdline/ subversion/tests/libsvn_fs_fs/ tools/server-side/svnfsfs/

Stefan Fuhrmann wrote:
> I see three alternative ways to code that function
>
> 1. As hard coded string / byte sequence (current implementation).
>
>   Cons:
>   * Hard to write, hard to review by just looking at it (applies to time
>     until initial release only).
>
>   Pros:
>   * Explicitly coded as constant, deterring people from changing it.

There's no need to obfuscate code in order to deter people from changing it. We have review, regression testing, social responsibility, and such like.

>   * Independent of other code, i.e. unintended changes to the format /
>     encoding generated by the normal code usually become apparent
>     when running the test suite.

Ack.

> 2. Use our txn code to write r0. This should be simple and might
>    at most require some special ID handling.
>
>   Cons:
>   * Generating incompatible r0s is likely not caught by our test suite
>     (assuming that reader and writer functions are in sync). Basically
>     all the risk that comes with dynamically generating a "constant".
>
>   * Test cases must make sure our backward compat repo creation
>     options create repos that can actually be used by old releases.
>     (we might want explicit test for that anyway, though)

So it sounds like you're comfortable that explicit test cases would easily cover this.

I simply don't understand why you think writing r0 is special, compared with writing everything else in a repository. Everything that's written has to be backwards compatible with its claimed format, not just the r0 record.

Nor do I understand why you think it is especially "constant" -- why are we not allowed to change the order of a couple of records in it or introduce a leading zero or a space or some such change that would otherwise be backward-compatible but would break the idea that this file is strictly "constant"?

>   Pros:
>   * No or very little special code for r0 (not sure, yet).
>
>   * Format / encoding changes don't require new r0 templates.

And:

   * Software engineers can take the code apart and put it back together in new and interesting ways, such as to develop format 8, without unreasonable extra effort.

> 3. Write code to "stitch" r0 together, e.g. string_add(md5("END\n")).
>
>   Cons:
>   * No more robust than 1. but requires much more code.
>
>   * May _look_ easy to understand but an actual offline review is still hard.
>
>   Pros:
>   * Widely independent of other code, although not as much as 1.
>
> Do you generally agree with the range of options and their assessment?
>
> Which one would you pick and why?
>
> I'd be fine with switching to option 2 as long as everyone understands
> the implications.

Sounds good to me.

- Julian
Received on 2014-06-26 19:27:37 CEST