[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] - Fix for issue #3792

From: Noorul Islam K M <noorul_at_collab.net>
Date: Wed, 09 Feb 2011 14:51:05 +0530

Noorul Islam K M <noorul_at_collab.net> writes:

> Daniel Shahaf <d.s_at_daniel.shahaf.name> writes:
>
>> Noorul Islam K M wrote on Sat, Feb 05, 2011 at 12:55:55 +0530:
>>
>>> Daniel Shahaf <d.s_at_daniel.shahaf.name> writes:
>>> > Noorul Islam K M wrote on Thu, Feb 03, 2011 at 14:15:48 +0530:
>>> >> + if (info->depth == svn_depth_exclude)
>>> >> + SVN_ERR(svn_cmdline_printf(pool, _("Depth: exclude\n")));
>>> >>
>>> >
>>> > I know that there is already some other place in the code that prints
>>> > the Depth: line, why didn't you reuse that place?
>>> >
>>>
>>> It is inside the if block 'if (info->has_wc_info)'. In our case flow
>>> will not reach there so I had to put it here.
>>>
>>
>> ... and? Why won't it enter that block? i.e., why is has_wc_info false?
>>
>> Some of the fields there (eg, checksum and copyfrom) make perfect sense
>> even for excluded nodes. (I don't know whether the wc can supply those
>> values even for excluded nodes.)
>>
>
> I don't think we have much information in DB to be printed.
>
> Below is the values from NODES table for excluded path 'A'
>
> wc_id = 1
> local_relpath =
> op_depth = 0
> parent_relpath =
> repos_id = 1
> repos_path =
> revision = 1
> presence = normal
> moved_here =
> moved_to =
> kind = dir
> properties = ()
> depth = infinity
> checksum =
> symlink_target =
> changed_revision = 1
> changed_date = 1297240607634523
> changed_author = noorul
> translated_size =
> last_mod_time =
> dav_cache =
> file_external =
>

My bad. I pasted wrong information. Here it is.

           wc_id = 1
   local_relpath = A
        op_depth = 0
  parent_relpath =
        repos_id = 1
      repos_path = A
        revision = 1
        presence = excluded
      moved_here =
        moved_to =
            kind = dir
      properties =
           depth = unknown
        checksum =
  symlink_target =
changed_revision =
    changed_date = 0
  changed_author =
 translated_size =
   last_mod_time =
       dav_cache =
   file_external =

Thanks and Regards
Noorul

>>> >> + if (tree_conflict || exclude)
>>> >> {
>>> >> svn_info_t *info;
>>> >> svn_error_clear(err);
>>> >>
>>> >> SVN_ERR(build_info_for_unversioned(&info, pool));
>>> >> - info->tree_conflict = svn_wc__cd2_to_cd(tree_conflict, pool);
>>> >>
>>> >> + if (tree_conflict)
>>> >> + info->tree_conflict = svn_wc__cd2_to_cd(tree_conflict, pool);
>>> >> + else
>>> >> + info->depth = svn_depth_exclude;
>>> >
>>> > s/else/if (exclude)/, please.
>>> >
>>>
>>> There is actually an outer if condition which makes this not useful.
>>
>> I know (it's about 5 lines above this line). But IMO this way is more
>> readable/maintainable.
>>
>
> I agree.
>
>>> >> +
>>> >> SVN_ERR(svn_wc__node_get_repos_info(&(info->repos_root_URL),
>>> >> - NULL,
>>> >> + exclude ?
>>> >> + &(info->repos_UUID) : NULL,
>>> >
>>> > Why?
>>> >
>>>
>>> I thought I should not make changes to existing behaviour. I think it is
>>> safe to just pass &(info->repos_UUID) in both cases.
>>>
>>
>> ... and? What is the "change to existing behaviour" you're talking about? (I
>> guess it's printing the repository UUID for excluded nodes?)
>
> As of now in trunk for this call NULL is passed reference to repository
> UUID. I thought I will keep that as such and pass a reference in the
> case of excluded. That is why I initially included that condition. Later
> I found that it is okay to pass a reference in both cases.
>
> Thanks and Regards
> Noorul
Received on 2011-02-09 10:23:23 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.