Hi all,
I am sorry for the delayed reply and the carelessness. Initially, I did
*not* have much context to comment on the patch.
Just because the svn_client_relocate() was *deprecated* and it uses
svn_client_relocate2() to implement itself in deprecated.c by hardcoding
ignore_externals as TRUE, I thought that the patch would solve the
compilation warning.
But now, with help of Kamesh I got to know some context of how the code
goes and had some experimentation with one of our internal server which
allows "checkout" only through "https".
Here is the experiment:
1. checked out a directory from a repo using "https".
2. added a property "svn:externals"
"ext_dir1 ....url_to_other_dir_in_same_repo'.
3. now did "svn up" and got 'ext_dir1'
4. now again added a property "svn:externals"
"ext_dir2 ....url_to_some_other_dir_in_same_repo' on ext_dir1
Thereby acheiving an "external" from an "external",
direct--> external --> external
5. did "svn up" and got 'ext_dir1/ext_dir2'
6. Now, manually updated the wc.db(update repository set root='http...
where id=1') of all these three to point to "http"
(so that I can trigger the code, proving the redirection of URL to
"https" to get the "corrected_url")
Now just tried to catch the "svn_client__update_internal" in the GDB
debgger,
it was clear that
"subversion/libsvn_client/externals.c:switch_dir_external()" does the
relocation for all ext_dir1 and ext_dir2 'ignore-externals=FALSE'.
My patch gets exercised only for top-level target which is supposed to
relocate only itself not any of its deep down external items.
So I can say my patch holds good.
And as a side-note, I was able to observe that upon these
experimentation, the "repository" table in wc.db did not update the
existing record, rather inserted a new record with "https".
I doubt if this is the actual behaviour....?
On Wed, 2010-12-08 at 17:29 +0200, Daniel Shahaf wrote:
> FWIW, my concerns here are:
>
> * svn_client_relocate{,2} have the same signature. This might be
> confusing sometimes. (but probably should be left alone)
>
> * svn_client_relocate2() takes an IGNORE_EXTERNALS parameter. Should
> we pass TRUE always to that parameter
yes it should always be TRUE to this parameter as per my above
experiment/explanation.
> , or should we pass the
> identically-named parameter of the calling function? (the calling
> function *happens* to have an appropriately-named parameter, but
> I haven't checked its semantics)
>
> Daniel
Thanks and regards
Prabhu
