Daniel Berlin wrote:
>> If above snip is the purpose of this implementation, I have the
>> following
>> thoughts,
>> a)Why to have it has a disk-based storage?
>> I am against it for the following reasons
>> * As this(svn:mergeinfo) data is not meant for sharing with other
>> tools like 'svnadmin' and
>> 'svnlook', this filebased storage is a wrong choice.
>> - I have a in-memory equivalent patch for the same.
>> * It is a joke to keep creating new hash in memory and dump to file
>> within the
>> same program execution context for each path having 'svn:mergeinfo'.
>> - I have another patch which does the same indexing from
>> 'write_final_rev' without caching at all.
>> * It is a data redundancy issue.
>
> We have been through this before. Different processes are allowed to
> reopen the transaction, and have no shared state between them. Thus,
> it must be stored on disk.
> I hate it as well, and I would not have implemented it this way if it
> was not required by our current API.
I could not see any need for 'svnlook', 'svnadmin',
bindings_from_different_process_context seeing this info unlike other
items like, paths_changed, props etc.
To make it simpler can you give one usecase where this info is useful. I
know would say don't expect proplist as eventually we will not have them
at all.
Yes it is eventual, that time we will have better place to retrieve the
same info and better usecases.
For now it seems like it exists as 'it may be useful kind of' which I
don't like.
My take from the log message is, this has been done this way to make it
accumulated in one hash and populate the 'mergeinfo' db at 'commit_body'
at one shot. If that is the case we can find better ways for the same.
>
> As i've also explained before, it was at one point, only stored in
> memory. But we can't do this and still support the API of reopening
> transactions betweens processes.
>
> You can argue that we should only keep this stuff in memory till you
> are blue in the face. I don't disagree, and as i've said, this was
> the way it was originally done. However, it is simply not legal in
> the face of what our current API allows. This will not change until
> 2.0, because of our API guarantees, so I highly suggest we just move
> on.
>
I think I have a working in-memory patch to achieve the same by making
use of the 'fs_fs_data_t' private fsap_data(which we already tweak for
merge-tracking). My only concern is 'is it a right place to do?'
With regards
Kamesh Jayachandran
> --Dan
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Sep 13 16:29:55 2006