On 04/12/2011 05:11 PM, arfre...@apache.org wrote: > Author: arfrever > Date: Tue Apr 12 21:11:46 2011 > New Revision: 1091573 > > URL: http://svn.apache.org/viewvc?rev=1091573&view=rev > Log: > Followup to r1090784: > > * build.conf > (libsvn_ra.libs): Add libsvn_fs_util.
Whoa. This doesn't feel right to me. Well, let me clarify, as I can see why r1090784 makes this necessary. It's r1090784 which looks severely problematic to me. The RA loader module is not permitted to access svn_fs_ functions directly! It may only call the vtable functions provided by the individual RA plugins. Of those plugins, ra_local is the only one permitted to call svn_fs_ functions. And now that I'm looking at the svn_fs_get_cache_config(), I'm feeling wrong about that, too. If this cache stuff is an FSFS-only thing, then it needs to live inside libsvn_fs_fs. libsvn_fs_util exists *only* for use by FS implementations (libsvn_fs_fs and libsvn_fs_base, today) which need to share utility functions. Sorry I didn't catch this stuff earlier (yes, guilty of "well, *someone* must be reviewing this code" syndrome), but I'm seeing what appears to be a series of screaming layering violations here. -- C. Michael Pilato <cmpil...@collab.net> CollabNet <> www.collab.net <> Distributed Development On Demand
