[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: Stefan Sperling <stsp_at_elego.de>
Date: Tue, 1 Mar 2011 20:48:10 +0100

On Sat, Feb 05, 2011 at 12:55:55PM +0530, Noorul Islam K M wrote:
> Daniel Shahaf <d.s_at_daniel.shahaf.name> writes:
> >> @@ -363,6 +364,9 @@
> >> SVN_ERR(svn_cmdline_printf(pool, _("Copied From Rev: %ld\n"),
> >> info->copyfrom_rev));
> >> }
> >> +
> >> + 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.

'has_wc_info' is documented as follows:

typedef struct svn_info_t
{

[...]

  /** Whether or not to ignore the next 10 wc-specific fields. */
  svn_boolean_t has_wc_info;

And below, we have this group of fields:

  /**
   * @name Working-copy path fields
   * These things only apply to a working-copy path.
   * See svn_wc_entry_t for explanations.
   * @{
   */
  svn_wc_schedule_t schedule;
  const char *copyfrom_url;
  svn_revnum_t copyfrom_rev;
  apr_time_t text_time;
  apr_time_t prop_time; /* will always be 0 for svn 1.4 and later */
  const char *checksum;
  const char *conflict_old;
  const char *conflict_new;
  const char *conflict_wrk;
  const char *prejfile;
  /** @since New in 1.5. */
  const char *changelist;
  /** @since New in 1.5. */
  svn_depth_t depth;

There are in fact 12 fields in this group.
So this comment is probably inaccurate -- the number 10 should now really
be 12. (We should probably just remove this number from the comment...)

Note that this group of fields includes the 'depth' field.
So accessing info->depth if has_wc_info is FALSE violates this API.

I think real problem is at the beginning of build_info_for_entry():

  SVN_ERR(svn_wc_read_kind(&kind, wc_ctx, local_abspath, FALSE, pool));

  if (kind == svn_node_none)
    return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
                             _("The node '%s' was not found."),
                             svn_dirent_local_style(local_abspath, pool));

It looks for a path on disk, and if that path isn't there it gives up
and returns an error. It should consider nodes with depth excluded
before giving up, though!
Does that make sense? If you fixed that, so that has_wc_info is TRUE for
excluded nodes, you could use the existing switch statement in info-cmd.c
to print the depth value:

        case svn_depth_unknown:
          /* Unknown depth is the norm for remote directories anyway
             (although infinity would be equally appropriate). Let's
             not bother to print it. */
          break;
 
+ case svn_depth_exclude:
+ SVN_ERR(svn_cmdline_printf(pool, _("Depth: exclude\n")));
+ break;
+
         case svn_depth_empty:
           SVN_ERR(svn_cmdline_printf(pool, _("Depth: empty\n")));
           break;

> Index: subversion/libsvn_client/info.c
> ===================================================================
> --- subversion/libsvn_client/info.c (revision 1067395)
> +++ subversion/libsvn_client/info.c (working copy)
> @@ -405,20 +405,30 @@

[...]

> SVN_ERR(svn_wc__node_get_repos_info(&(info->repos_root_URL),
> - NULL,
> + &(info->repos_UUID),

+1 on this change (printing the UUID is good). But you didn't mention
it in the log message. You should do so, because you are changing the
behaviour in case the node is a tree-conflict victim. Something like
"Also, provide the repository UUID in info about tree conflict victims"
would be sufficient.

> ctx->wc_ctx,
> local_abspath, FALSE, FALSE,
> pool, pool));
Received on 2011-03-01 20:48:52 CET

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