You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Bert Huijben <be...@qqmail.nl> on 2008/04/06 20:52:45 UTC
[Patch] RE: Why was svn_client_mergeinfo_get_merged removed?
Hi,
Here is a patch that reintroduces the svn_client_mergeinfo_get_merged() and
svn_client_mergeinfo_get_eligible() (former
svn_client_mergeinfo_get_available()) API's.
I extended the svn_client_mergeinfo_get_available() API with the peg
revision introduced in the new api's.
(I tested the implementation of the commands in the SharpSvn development
environment in that they return the same data as before on the collab merge
demo repository).
An other change is that svn_client_suggest_merge_sources() started returning
repository local paths instead of full urls, while the documentation still
says it should return full urls. (This change was merged to 1.5.x in the
same merge as the other changes)
Bert
> -----Original Message-----
> From: Stefan Küng <to...@gmail.com> [mailto:=?ISO-8859-
> 1?Q?Stefan_K=FCng_<to...@gmail.com>?=]
> Sent: zondag 6 april 2008 11:05
> To: dev@subversion.tigris.org
> Cc: Bert Huijben (TCG)
> Subject: Re: Why was svn_client_mergeinfo_get_merged removed?
>
> Bert Huijben (TCG) wrote:
> >> -----Original Message-----
> >> From: Stefan Küng <to...@gmail.com> [mailto:=?ISO-8859-
> >
> >> Hi,
> >>
> >> When I updated today, I discovered that the API
> >> svn_client_mergeinfo_get_merged() got removed from the 1.5.x branch.
> I
> >> was using this API in TSVN to mark already merged revisions in the
> log
> >> dialog.
> >>
> >> Is there a reason why this was removed? And why from an .x branch? I
> >> now
> >> have to either find another way to get that feature back, or drop it
> >> completely :(
> >
> > svn_error_t *
> > svn_client_mergeinfo_log_merged(const char *path_or_url,
> > const svn_opt_revision_t
> *peg_revision,
> > const char *merge_source_url,
> > const svn_opt_revision_t
> *src_peg_revision,
> > svn_log_entry_receiver_t receiver,
> > void *receiver_baton,
> > svn_boolean_t discover_changed_paths,
> > const apr_array_header_t *revprops,
> > svn_client_ctx_t *ctx,
> > apr_pool_t *pool);
> >
> > and
> >
> > svn_error_t *
> > svn_client_mergeinfo_log_eligible(const char *path_or_url,
> > const svn_opt_revision_t
> *peg_revision,
> > const char *merge_source_url,
> > const svn_opt_revision_t
> > *src_peg_revision,
> > svn_log_entry_receiver_t receiver,
> > void *receiver_baton,
> > svn_boolean_t
> discover_changed_paths,
> > const apr_array_header_t *revprops,
> > svn_client_ctx_t *ctx,
> > apr_pool_t *pool);
> >
> > Are the replacement methods for svn_client_mergeinfo_get_merged() and
> > svn_client_mergeinfo_get_available()
>
> These may be replacements, but they work and are used completely
> different. IMHO a replacement is something I can use instead of the
> replaced thing without changing too much. These are not replacements,
> these are alternatives.
>
> > The log messages talk about cleaning up the api, but I see some
> problem in
> > using the new api's over the old ones.
> >
> > The new ones always call svn_client_log*() to get the results, which
> has at
> > least some and in some cases a major performance impact. (And in the
> case of
> > your new TortoiseSVN doesn't allow re using the existing knowledge in
> the
> > log message cache).
>
> That's what I discovered too. Right now, I'm thinking of dropping the
> feature in TSVN.
>
> Stefan
>
> --
> ___
> oo // \\ "De Chelonian Mobile"
> (_,\/ \_/ \ TortoiseSVN
> \ \_/_\_/> The coolest Interface to (Sub)Version Control
> /_/ \_\ http://tortoisesvn.net
Re: [Patch] RE: Why was svn_client_mergeinfo_get_merged removed?
Posted by "C. Michael Pilato" <cm...@collab.net>.
Bert Huijben wrote:
>> -----Original Message----- From: C. Michael Pilato
>> [mailto:cmpilato@collab.net] Sent: maandag 7 april 2008 16:38 To: Bert
>> Huijben Cc: dev@subversion.tigris.org Subject: Re: [Patch] RE: Why was
>> svn_client_mergeinfo_get_merged removed?
>>
>> Bert Huijben wrote:
>>> Here is a patch that reintroduces the
>> svn_client_mergeinfo_get_merged() and
>>> svn_client_mergeinfo_get_eligible() (former
>>> svn_client_mergeinfo_get_available()) API's.
>> I'm not interested in carrying forward a naive implementation of
>> get_eligible(). Most of the use-cases for that functionality that I'm
>> aware of are either a) pointless if not paired with additional info
>> (like 'svn log' output) or b) answerable using the get_merged() API.
>
> The log output is a very slow api when using over webdav. I know of at
> least one current common subversion client (TortoiseSVN) which caches svn
> log results to speed up the processing; and I expect AnkhSVN will do the
> same in a future version.
Yeah, yeah, I get that. I'm only talking about the get_eligible() API,
though. I'm fine with re-adding the get_merged() one.
> One of the problems is: A full log of *all revisions* between the first
> and last available revision are retrieved. For webdav this tells me that
> mod_dav_svn first logs all log messages in server ram before transferring
> the first to the webdav client. And then the client ignores most of the
> results.
Sure, I could have done repeated log requests for each range in the
rangelist, paying careful attention to renames in the history of the merge
source/target (whichever is being crawled), and so one. Are 300 full
network turnarounds going to be cheaper than a single request with largish
response? That depends largely on things Subversion can't know.
Fortunately these aren't matters of interest to the design of the API, and
be addressed incrementally as performance enhancements in 1.5.1 or later.
>> Is that true? Looking at the code for this function, there are two
>> places where the list of returned values is appended to. Both cases
>> are appending the result of a path-join with the repository root URL
>> and a repos- relative path. Did some bug suddenly appear there?
>
> It seems to only have slipped in the 1.5.x branch... (The last lines of
> mergeinfo.c are different in appending a url prefix)
Uh ... sorry, not seeing it. http://pastebin.ca/975439
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
RE: [Patch] RE: Why was svn_client_mergeinfo_get_merged removed?
Posted by Bert Huijben <be...@vmoo.com>.
> -----Original Message-----
> From: C. Michael Pilato [mailto:cmpilato@collab.net]
> Sent: maandag 7 april 2008 16:38
> To: Bert Huijben
> Cc: dev@subversion.tigris.org
> Subject: Re: [Patch] RE: Why was svn_client_mergeinfo_get_merged
> removed?
>
> Bert Huijben wrote:
> > Here is a patch that reintroduces the
> svn_client_mergeinfo_get_merged() and
> > svn_client_mergeinfo_get_eligible() (former
> > svn_client_mergeinfo_get_available()) API's.
>
> I'm not interested in carrying forward a naive implementation of
> get_eligible(). Most of the use-cases for that functionality that I'm
> aware
> of are either a) pointless if not paired with additional info (like
> 'svn
> log' output) or b) answerable using the get_merged() API.
The log output is a very slow api when using over webdav. I know of at least one current common subversion client (TortoiseSVN) which caches svn log results to speed up the processing; and I expect AnkhSVN will do the same in a future version.
One of the problems is: A full log of *all revisions* between the first and last available revision are retrieved. For webdav this tells me that mod_dav_svn first logs all log messages in server ram before transferring the first to the webdav client. And then the client ignores most of the results.
I haven't thoroughly tested the impact in speed of a log operation without properties and files; but some tests with the commandline client seem to slow things down by about a factor of 2. (I would have expected a far worse result).
(My merge tests in the SharpSvn framework only test mergeinfo on a local repository)
I would love to have just one api with just an off switch on the svn_client_log operation, so caching can be used.. Just returning the raw revisions over the receiver. (Including the ones that don't exist on the target)
(If (b) is the way to go.. we could drop the entire svn_client api in 2.0... Everything can be accomplished by using the more low level api's ;-))
>
> > An other change is that svn_client_suggest_merge_sources() started
> returning
> > repository local paths instead of full urls, while the documentation
> still
> > says it should return full urls. (This change was merged to 1.5.x in
> the
> > same merge as the other changes)
>
> Is that true? Looking at the code for this function, there are two
> places
> where the list of returned values is appended to. Both cases are
> appending
> the result of a path-join with the repository root URL and a repos-
> relative
> path. Did some bug suddenly appear there?
It seems to only have slipped in the 1.5.x branch... (The last lines of mergeinfo.c are different in appending a url prefix)
Bert
>
> --
> C. Michael Pilato <cm...@collab.net>
> CollabNet <> www.collab.net <> Distributed Development On
> Demand
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [Patch] RE: Why was svn_client_mergeinfo_get_merged removed?
Posted by "C. Michael Pilato" <cm...@collab.net>.
Bert Huijben wrote:
> Here is a patch that reintroduces the svn_client_mergeinfo_get_merged() and
> svn_client_mergeinfo_get_eligible() (former
> svn_client_mergeinfo_get_available()) API's.
I'm not interested in carrying forward a naive implementation of
get_eligible(). Most of the use-cases for that functionality that I'm aware
of are either a) pointless if not paired with additional info (like 'svn
log' output) or b) answerable using the get_merged() API.
> An other change is that svn_client_suggest_merge_sources() started returning
> repository local paths instead of full urls, while the documentation still
> says it should return full urls. (This change was merged to 1.5.x in the
> same merge as the other changes)
Is that true? Looking at the code for this function, there are two places
where the list of returned values is appended to. Both cases are appending
the result of a path-join with the repository root URL and a repos-relative
path. Did some bug suddenly appear there?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand