Hi,
I was working on a patch (attached) to fix issue #3361, which also fixes issue
#3294. I found in the update editor API, there was a check to see if the
copyfrom_url and the wc entries url are same by checking the ancestral
relation, which will not work for urls with different schemes. I discussed
about it in IRC whose transcript is as follows:
<snip>
(02:12:08 PM) stylesen: can anyone tell me what is the use of copyfrom_url in wc?
(02:15:29 PM) ehu`: stylesen: sure.
(02:15:48 PM) ehu`: it's the url of the item which has an "A+" state.
(02:16:00 PM) ehu`: the source of it, that is
(02:31:57 PM) stylesen: ehu`: this copyfrom_url is recorded with the absolute
path in the wc entries? like "http://svn.collab.net/trunk/somedir"
(02:32:11 PM) ehu`: yes
(02:32:21 PM) stylesen: or the relative path from root is alone recorded
"trunk/somedir"
(02:32:30 PM) ehu`: although it should probably be "root relative"
(02:32:42 PM) ehu`: I think it's absolute.
(02:32:54 PM) gstein: it is absolute
(02:33:04 PM) ehu`: which is wrong.
(02:33:06 PM) ehu`: imo
(02:33:13 PM) ehu`: root relative is much better.
(02:33:17 PM) stylesen: yes me too feel the same
(02:33:31 PM) stylesen: we need to record only the relative path from the root
and not the absolute path
(02:33:51 PM) stylesen: this will help in mixing different schemes while
merging which am trying to solve
(02:34:02 PM) gstein: that is true if *and only if* the copyfrom can *only*
come from the same repository
(02:34:12 PM) gstein: in wc-ng, we allow copyfrom to specify arbitrary repositories
(02:35:00 PM) gstein: we specify it as a 4-tuple: repository root url,
repository uuid, relative path from root, and revision
(02:35:23 PM) ***ehu` was writing that's better
(02:35:23 PM) stylesen: in
"/subversion-dev/trunk/subversion/libsvn_wc/update_editor.c", line 5358I am
trying to change the if check, to check for uuid and not ancestral relation
(02:35:37 PM) stylesen: which fails on different url schemes
(02:36:21 PM) gstein: you do not have enough information at that point in the
code to fix that check
(02:36:52 PM) gstein: there is no copyfrom_uuid, so you will not be able to fix
that
(02:36:53 PM) stylesen: gstein: I can find the uuids of the two urls and check?
(02:37:08 PM) gstein: and you certainly do not want to hit the server to fetch
a uuid
(02:38:01 PM) stylesen: then I shall come up with "svn_wc_add_repos_file4"
which will take the uuids?
(02:38:24 PM) gstein: don't even bother with it
(02:39:13 PM) gstein: wait until the wc-ng is more complete
(02:39:32 PM) ehu`: stylesen: I don't think we should be fixing too many issues
in wc-1, given that we're switching to wc-ng in the "near" future.
(02:39:34 PM) stylesen: gstein: will wc-ng support different schemes?
(02:39:47 PM) gstein: it has a better mechanism for doing so, yes
(02:40:04 PM) gstein: and it *does* record uuids for all the repositories that
it knows about,
(02:40:14 PM) stylesen: am trying to fix issues 3361 and 3294
(02:40:18 PM) gstein: so it will be possible to see N different root URLs, but
know they all refer to the same repository
(02:40:47 PM) gstein: for example, it would know that svn.apache.org/repos/asf
and svn.eu.apache.org/repos/asf and https://svn.apache.org/repos/asf are ALL
the same repository
(02:40:52 PM) gstein: different scheme, different host, etc
(02:41:04 PM) stylesen: that sounds cool
(02:41:09 PM) gstein: you don't want to just allow different schemes,
(02:41:18 PM) stylesen: that should ultimately fix scheme matching
(02:41:20 PM) gstein: because the repository might have a different root url
(02:41:42 PM) gstein: it could be http://svn.example.com/root/ and
https://svn.example.com/protected/
(02:42:35 PM) stylesen: yes I get it, so we need not want this check in
libsvn_client/merge.c at line 1454, which I feel is not correct
(02:42:56 PM) stylesen: may be I shall fix this scheme checking for now in
merge.c code to use the uuid check
(02:43:32 PM) gstein: yes, that check is incorrect
(02:43:45 PM) stylesen: or just remove that check altogether, from merge.c
code, because we already know we are working on same_repos
(02:43:52 PM) gstein: urm. wait...
(02:44:04 PM) gstein: it is fine to compare that the two schemes *match*
(02:44:21 PM) gstein: but you cannot expect to replace the scheme and get the
same repository,
(02:44:48 PM) gstein: so yah: that check is actually fine. it ensures that you
use the same scheme.
(02:45:19 PM) gstein: stylesen: I don't know what that code is doing, so I
can't answer the question
(02:45:36 PM) stylesen: gstein: okie, let me have a closer look
(02:47:42 PM) gstein: ah. I see.
(02:47:56 PM) gstein: that check is necessary, but insufficient
(02:48:42 PM) gstein: I'd not worry about fixing it,
(02:48:55 PM) gstein: and assume it will be more easily fixed after some wc-ng
work completes
(02:49:45 PM) stylesen: gstein: can you elaborate on the insufficient part,
what are the other checks that needs to be done?
(02:50:53 PM) gstein: it appears that code constructs a new copyfrom_url,
(02:50:59 PM) gstein: but *only* checks the scheme,
(02:51:12 PM) gstein: when it should be checking the entire repository root url
(02:51:53 PM) gstein: i.e. it could allow http://svn.eu.apache.org/ when it
should *only* allow http://svn.apache.org/
(02:52:05 PM) gstein: ie. it isn't checking the hostname in this example
</snip>
The attached patch removes the check for scheme (of url) during a merge, which
IMHO is not required, since we already know we are operating with the same
repository and all the mergeinfo that is getting recorded is relative to the
root and not an absolute path. I may be missing something seriously here.
I would like to get more comments on this patch whether we can go ahead in
fixing this (which will benefit 1.6.x and previous versions) or wait for wc-ng
in order to get a cleaner fix for future and don't bother about previous releases.
Though this patch fixes these issues (and passes all tests), *it is not ready
to commit*, It poses a general idea in which I would like to fix these issues.
Thank You.
--
Senthil Kumaran S
http://www.stylesen.org/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1725218
Received on 2009-04-15 12:34:57 CEST