Hi Neels, Just thought I would check with you - with respect to do you require me to keep an eye on this thread for the purposes of pinging the list etc?
Gavin. On 26/12/2009, at 12:48 PM, Neels J Hofmeyr wrote: > Sorry, hold this patch. > > Greg, > > I'm asking you since you said there may be a duplicate; but it looks like > it's more of a misnomer instead ("get_node_uri"), as Julian pointed out IIRC. > > Here is the "duplicate", with improved name: > > [[[ > static svn_error_t* > get_node_base_rev_and_url_components(svn_revnum_t *revision, > const char **repos_relpath, > const char **repos_root_url, > svn_wc__db_t *db, > const char *local_abspath, > apr_pool_t *result_pool, > apr_pool_t *scratch_pool) > { > ... svn_wc__db_base_get_info(... > ]]] > > And here what you suggested: > > [[[ > svn_error_t * > svn_wc__internal_node_get_url(const char **url, > svn_wc__db_t *db, > const char *local_abspath, > apr_pool_t *result_pool, > apr_pool_t *scratch_pool) > { > ... svn_wc__db_read_info(... > ]]] > > So the former returns REVISION, REPOS_RELPATH and REPOS_ROOT_URL, all needed > in check_tree_conflict() to mangle and feed to arguments. > The latter has only the complete URL string in store, no revision. > > In the patch of the previous mail, I got the URL separated into > REPOS_RELPATH and REPOS_ROOT_URL, but it turns out I also need a REVISION. I > also need specifically the BASE information, so AFAICT using > svn_wc__db_base_get_info() makes more sense than using svn_wc__db_read_info(). > > Do you agree that it's no duplicate after all? > > BTW, this is how get_node_base_rev_and_url_components() and its return > values would be used in check_tree_conflict(), libsvn_wc/update_editor.c: > [[[ > /* Source-left repository root URL and path in repository. > * The Source-right ones will be the same for update. > * For switch, only the path in repository will differ, because > * a cross-repository switch is not possible. */ > SVN_ERR(get_node_base_rev_and_url_components(&revision, > &path_in_repos, > &repos_url, eb->db, > local_abspath, pool, > pool)); > > src_left_version = svn_wc_conflict_version_create(repos_url, > path_in_repos, > revision, > left_kind, > pool); > > /* kind is both base kind and working kind, because schedule > * replace-by-different-kind is not supported. */ > /* ### TODO: but in case the entry is locally removed, entry->kind > * is svn_node_none and doesn't reflect the older kind. Then we > * need to find out the older kind in a different way! */ > > /* For switch, find out the proper PATH_IN_REPOS for source-right. */ > if (eb->switch_url != NULL) > { > if (their_url != NULL) > path_in_repos = svn_uri_is_child(repos_url, their_url, pool); > else > { > /* The complete source-right URL is not available, but it > * is somewhere below the SWITCH_URL. For now, just go > * without it. > * ### TODO: Construct a proper THEIR_URL in some of the > * delete cases that still pass NULL for THEIR_URL when > * calling this function. Do that on the caller's side. */ > path_in_repos = svn_uri_is_child(repos_url, eb->switch_url, > pool); > path_in_repos = apr_pstrcat( > pool, path_in_repos, > "_THIS_IS_INCOMPLETE", > NULL); > } > } > > src_right_version = svn_wc_conflict_version_create(repos_url, > path_in_repos, > *eb->target_revision, > their_node_kind, > pool); > > ... > svn_wc_conflict_description_create_tree2( > ... src_left_version, src_right_version, ...); > ... > } > ]]] > > Thanks, > ~Neels > > Neels J Hofmeyr wrote: >> Hi, >> >> maybe you remember that patch of mine with a function gstein said looks like >> a duplicate of svn_wc__internal_node_get_url(). >> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2415782 >> >> It almost *is* a duplicate, but there's a difference... >> >> svn_wc__internal_node_get_url() *combines* the repos_root_url and >> repos_relpath to a URL. But I need them *separate*. If you want to see why, >> look at get_node_uri(), which I want to eliminate, in the output of: >> >> svn diff -c-880555 ^/subversion/trunk | less >> >> (This had been committed before and reverted in r880555, and now I'm giving >> it some review to be able to commit it again. It "wc-ng"s the tree-conflict >> detection during update.) >> >> >> So, I split functionality off svn_wc__internal_node_get_url() to get a >> function that does everything except combining the two URL parts into a >> single string. >> >> See attached patch that does the function split I'd like to do, so I can >> then fix and commit -c-880555. -- if you like attached patch, feel free to >> commit. I don't have an apache account yet. >> >> Thanks, >> ~Neels >> >