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
>> 
> 

Reply via email to