On 25 June 2014 19:54, Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com> wrote:
> On Wed, Jun 25, 2014 at 5:46 PM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
>>
>> On 25 June 2014 19:24, Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com>
>> wrote:
>> > On Wed, Jun 25, 2014 at 4:57 PM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
>> >>
>> >> On 21 June 2014 17:15, <stefan2_at_apache.org> wrote:
>> >> > Author: stefan2
>> >> > Date: Sat Jun 21 15:15:30 2014
>> >> > New Revision: 1604421
>> >> >
>> >> > URL: http://svn.apache.org/r1604421
>> >> > Log:
>> >> > Append index data to the rev data file instead of using separate
>> >> > files.
>> >> >
>> >> > This gives unpacked FSFS format 7 similar I/O characteristics and
>> >> > disk
>> >> > space usage as earlier formats. It also eliminates the need for
>> >> > retries
>> >> > after a potential background pack run because each file is now always
>> >> > consistent with itself (earlier, data or index files might already
>> >> > have
>> >> > been deleted while the respective other was still valid). Overall,
>> >> > most of this patch is removing code necessary to handle separate
>> >> > files.
>> >> >
>> >> > The new file format is simple:
>> >> >
>> >> > <rep data><l2p index><p2l index><footer><footer length>
>> >> >
>> >> > with the first three being identical what we had before. <footer
>> >> > length>
>> >> > is a single byte giving the length of the preceding footer, so it's
>> >> > easier to extract than the pre-f7 rev trailer and there is only one
>> >> > per pack / rev file.
>> >> >
>> >> [..]
>> >>
>> >>
>> >> >Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
>> >> >URL:
>> >> >
>> >> > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1604421&r1=1604420&r2=1604421&view=diff
>> >>
>> >> >
>> >> > > >==============================================================================
>> >> >--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
>> >> >+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Sat Jun 21
>> >> > 15:15:30
>> >> > 2014
>> >> >@@ -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?
>> >
>> > To make it more obvious that I intend to follow
>> > the svn_io_file_create_binary interface, I added
>> > some commentary explaining where the numbers
>> > come from. But even just placing the sum (without
>> > its elements) in there would be fine already.
>> >
>> > Changing the rev 0 template has never been a fun
>> > activity and wont become one in the foreseeable
>> > future.
>> >
>> I believe that the committer should be responsible forcommitting
>> readable and easy to maintain code, not the reviewer. So please fix or
>> revert.
>
>
> Sorry, I should have referenced r1605444 in my post.
>
I saw it, but it doesn't make code easier to maintain.
--
Ivan Zhakov
Received on 2014-06-25 18:04:05 CEST