On Sat, Sep 8, 2012 at 7:41 PM, Daniel Shahaf <d.s_at_daniel.shahaf.name>wrote:
> Stefan Fuhrmann wrote on Sat, Sep 08, 2012 at 01:32:54 +0200:
> > On Fri, Sep 7, 2012 at 9:36 PM, Daniel Shahaf <danielsh_at_elego.de> wrote:
> >
> > > I think this fix is wrong. svn_fs_fs__read_noderev() should return the
> > > parsed noderev as is --- if it's a revision file and the noderev
> > > contains an is-fresh-txn-root field, then it needs to reflect that.
> > >
> >
> > The problem is that the whole situation is very messy:
> >
> > * the flag basically says "this is still under construction,
> > redirect data lookup as required". For committed data,
> > this is clearly an invalid state or at least useless info.
> >
> > * there is some bug that will leave this flag in empty
> > revisions. This bug needs to be found an fixed (not
> > a big problem, just needs to be done).
> > * _existing_ repositories have cases of this otherwise
> > benign corruption.These cases need to be handled
> > gracefully. 1.8 checks for issue #4129 will report all
> > those repositories as corrupted.
> >
> > * the fix for issue #2608 makes svn_fs_fs__dag_get_revision
> > report the "wrong" revision.
> >
> > The only other way to make that work would be an
> > additional flag in noderev structure that allows for the
> > "is-fresh-txn-root" flag to be set but ignoring it in all
> > other code.
> >
> > Would you be more happy with that solution?
> >
>
> Well, I see two things here:
>
> - "It would be cleaner to have svn_fs_fs__read_noderev() a simple
> unserializer" --- done in r1382333.
>
Yup that plus your second commit looks like a better solution.
When I wrote the code, I assumed that the parser function
would be invoked from more places.
- "The users of is_fresh_txn_root should be refactored" --- I wonder if
> we couldn't just remove the need for the field altogether without much
> work; see my last comment on issue #4031 for details.
>
I don't know too much about the commit process. So, I have
to pass on that one for now.
> Sorry for the brevity yesterday, I think that commit review would have
> benefited from being written today rather than yesterday.
>
Well, I've been on working holidays. Your review didn't
trouble me too much ;)
-- Stefan^2.
> > --
> > *
> >
> > Join us this October at Subversion Live
> > 2012<http://www.wandisco.com/svn-live-2012>
> > for two days of best practice SVN training, networking, live demos,
> > committer meet and greet, and more! Space is limited, so get signed up
> > today<http://www.wandisco.com/svn-live-2012>
> > !
> > *
>
--
*
Join us this October at Subversion Live
2012<http://www.wandisco.com/svn-live-2012>
for two days of best practice SVN training, networking, live demos,
committer meet and greet, and more! Space is limited, so get signed up
today<http://www.wandisco.com/svn-live-2012>
!
*
Received on 2012-09-11 02:48:54 CEST