On Sat, Sep 8, 2012 at 7:41 PM, Daniel Shahaf <d...@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 <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. > 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> ! *
