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

Using ra->set_path() over svn:// disallows SVN_INVALID_REVNUM?

From: Karl Fogel <kfogel_at_red-bean.com>
Date: Fri, 12 Sep 2008 14:47:38 -0400

(Issue #3282 is some kind of cosmic can-opener... The more I turn the
handle, the more other bugs get revealed.)

The documentation for svn_ra_reporter3_t.svn_ra_set_path() in
include/svn_ra.h claims that the 'revision' parameter can be

   * [...]
   * @a revision may be SVN_INVALID_REVNUM if (for example) @a path
   * represents a locally-added path with no revision number, or @a
   * depth is @c svn_depth_exclude.
   * [...]
  svn_error_t *(*set_path)(void *report_baton,
                           const char *path,
                           svn_revnum_t revision,
                           svn_depth_t depth,
                           svn_boolean_t start_empty,
                           const char *lock_token,
                           apr_pool_t *pool);

But watch what happens if you actually try it over svn://...

I have a patch to libsvn_wc/adm_crawler.c that causes some code to
look like this:

          /* ... or report a differing revision or lock token, or the
             mere presence of the file in a depth-empty dir, or a
             replaced-without-history file. */
          else if (current_entry->revision != dir_rev
                   || current_entry->lock_token
                   || dot_entry->depth == svn_depth_empty
                   || (current_entry->schedule == svn_wc_schedule_replace
                       && current_entry->copyfrom_url == NULL))

The new 'effective_rev' argument is sometimes SVN_INVALID_REVNUM
(i.e., -1), to indicate that what we have locally is a
replaced-without-history file. The above code leads directly to this
in libsvn_client/status.c:reporter_set_path():

   return rb->wrapped_reporter->set_path(rb->wrapped_report_baton, path,
                                         revision, depth, start_empty,
                                         lock_token, pool);

Which leads to this in libsvn_ra_svn/client.c:ra_svn_set_path():

   SVN_ERR(svn_ra_svn_write_cmd(b->conn, pool, "set-path", "crb(?c)w",
                                path, rev, start_empty, lock_token,

Which leads to this code in svn_ra_svn_write_cmd():

   va_start(ap, fmt);
   err = vwrite_tuple(conn, pool, fmt, ap);

Which leads to this in vwrite_tuple():

   else if (*fmt == 'r')
       rev = va_arg(ap, svn_revnum_t);
       if (SVN_IS_VALID_REVNUM(rev))
         SVN_ERR(svn_ra_svn_write_number(conn, pool, rev));

And there's our problem: 'rev' is obviously -1, and 'opt' is 0,
because the controlling fmt string here is "crb(?c)w", in which the
"r" is not 'opt'ional.

I can't just change the "r" to "(?r)", because old servers won't
expect the "r" to be optional inside its own tuple -- which is the way
SVN_INVALID_REVNUM should be represented, if I understand the
documentation to svn_ra_svn_write_tuple() correctly.

So there's a mismatch between how we intended ra->set_path() to work,
and how svnserve actually expects it to work. We intended to be able
to represent SVN_INVALID_REVNUM; in practice, I think we can't.

Are there any brilliant, compatible solutions to this that I'm missing?

In the meantime, I think I may be able to solve my issue #3282 problem
by sending the base revision of the replaced file (that is, the
shadowed file), since we still have its .svn-revert bases sitting
around and can use them if the server sends a delta against the old
text base.


To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-09-12 20:47:52 CEST

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