You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2015/01/05 18:15:27 UTC
svn commit: r1649592 - /subversion/trunk/subversion/libsvn_fs_x/fs_x.c
Author: stefan2
Date: Mon Jan 5 17:15:27 2015
New Revision: 1649592
URL: http://svn.apache.org/r1649592
Log:
Follow-up to r1645907: Fix pack test failures seen on macos buildbot.
* subversion/libsvn_fs_x/fs_x.c
(write_revision_zero): Fix off-by-4 in noderev size.
Modified:
subversion/trunk/subversion/libsvn_fs_x/fs_x.c
Modified: subversion/trunk/subversion/libsvn_fs_x/fs_x.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/fs_x.c?rev=1649592&r1=1649591&r2=1649592&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/fs_x.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/fs_x.c Mon Jan 5 17:15:27 2015
@@ -863,7 +863,7 @@ write_revision_zero(svn_fs_t *fs,
"2d2977d1c96f487abe4a1e202dd03b4e\n"
"cpath: /\n"
"\n\n",
- 0x8b, subpool));
+ 0x87, subpool));
/* Construct the index P2L contents: describe the 3 items we have.
Be sure to create them in on-disk order. */
@@ -881,7 +881,7 @@ write_revision_zero(svn_fs_t *fs,
entry = apr_pcalloc(subpool, sizeof(*entry));
entry->offset = 0x1d;
- entry->size = 0x6d;
+ entry->size = 0x69;
entry->type = SVN_FS_X__ITEM_TYPE_NODEREV;
entry->item_count = 1;
entry->items = apr_pcalloc(subpool, sizeof(*entry->items));
@@ -890,7 +890,7 @@ write_revision_zero(svn_fs_t *fs,
APR_ARRAY_PUSH(index_entries, svn_fs_x__p2l_entry_t *) = entry;
entry = apr_pcalloc(subpool, sizeof(*entry));
- entry->offset = 0x1d + 0x6d;
+ entry->offset = 0x1d + 0x69;
entry->size = 1;
entry->type = SVN_FS_X__ITEM_TYPE_CHANGES;
entry->item_count = 1;
Re: svn commit: r1649592 - /subversion/trunk/subversion/libsvn_fs_x/fs_x.c
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Mon, Jan 5, 2015 at 8:13 PM, Branko Čibej <br...@wandisco.com> wrote:
> On 05.01.2015 18:15, stefan2@apache.org wrote:
> > Author: stefan2
> > Date: Mon Jan 5 17:15:27 2015
> > New Revision: 1649592
> >
> > URL: http://svn.apache.org/r1649592
> > Log:
> > Follow-up to r1645907: Fix pack test failures seen on macos buildbot.
> >
> > * subversion/libsvn_fs_x/fs_x.c
> > (write_revision_zero): Fix off-by-4 in noderev size.
> >
> > Modified:
> > subversion/trunk/subversion/libsvn_fs_x/fs_x.c
> >
> > Modified: subversion/trunk/subversion/libsvn_fs_x/fs_x.c
> > URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/fs_x.c?rev=1649592&r1=1649591&r2=1649592&view=diff
> >
> ==============================================================================
> > --- subversion/trunk/subversion/libsvn_fs_x/fs_x.c (original)
> > +++ subversion/trunk/subversion/libsvn_fs_x/fs_x.c Mon Jan 5 17:15:27
> 2015
> > @@ -863,7 +863,7 @@ write_revision_zero(svn_fs_t *fs,
> > "2d2977d1c96f487abe4a1e202dd03b4e\n"
> > "cpath: /\n"
> > "\n\n",
> > - 0x8b, subpool));
> > + 0x87, subpool));
>
>
> Yah. This is why such magic numbers should never appear in the code in
> the first place. We had a heated discussion this, or some similar code
> in fsfs7, a while ago ...
>
Yah, yah ;)
The key here is that I plan to change r0 for FSX
in the future while there is no intent to do so for
FSFS. And not storing the directory rep brought
the code back to using plain strings plus produces
a smaller and simpler r0.
> Is it really that hard to write a rev file constructor function that
> calculates the magic numbers, including checksum, based on the revision
> contents? I'd feel a lot safer that way.
>
My main argument in the past has been to keep
the template as it has always been and to use it
as a tiny format 1 splinter even in new repositories.
I remember a few instances where new code would
fail for r0 specifically because old format quirks.
Now Stefan kind of suggested to add regression
tests for r0 - which would be an obvious thing to do.
With those in place, we can still guarantee to write
old-style r0 content. IOW, I'll give it a try.
-- Stefan^2.
Re: svn commit: r1649592 - /subversion/trunk/subversion/libsvn_fs_x/fs_x.c
Posted by Branko Čibej <br...@wandisco.com>.
On 05.01.2015 18:15, stefan2@apache.org wrote:
> Author: stefan2
> Date: Mon Jan 5 17:15:27 2015
> New Revision: 1649592
>
> URL: http://svn.apache.org/r1649592
> Log:
> Follow-up to r1645907: Fix pack test failures seen on macos buildbot.
>
> * subversion/libsvn_fs_x/fs_x.c
> (write_revision_zero): Fix off-by-4 in noderev size.
>
> Modified:
> subversion/trunk/subversion/libsvn_fs_x/fs_x.c
>
> Modified: subversion/trunk/subversion/libsvn_fs_x/fs_x.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/fs_x.c?rev=1649592&r1=1649591&r2=1649592&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_fs_x/fs_x.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_x/fs_x.c Mon Jan 5 17:15:27 2015
> @@ -863,7 +863,7 @@ write_revision_zero(svn_fs_t *fs,
> "2d2977d1c96f487abe4a1e202dd03b4e\n"
> "cpath: /\n"
> "\n\n",
> - 0x8b, subpool));
> + 0x87, subpool));
Yah. This is why such magic numbers should never appear in the code in
the first place. We had a heated discussion this, or some similar code
in fsfs7, a while ago ...
Is it really that hard to write a rev file constructor function that
calculates the magic numbers, including checksum, based on the revision
contents? I'd feel a lot safer that way.
-- Brane