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.
- "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.
Sorry for the brevity yesterday, I think that commit review would have
benefited from being written today rather than yesterday.
> -- 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>
> !
> *
Received on 2012-09-08 19:42:09 CEST