Stefan Fuhrmann wrote on Sat, Sep 08, 2012 at 01:32:54 +0200: > On Fri, Sep 7, 2012 at 9:36 PM, Daniel Shahaf <danie...@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> > ! > *
