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

Re: 1.6.x backport of "svnadmin hotcopy" for symlinks - API change?

From: C. Michael Pilato <cmpilato_at_collab.net>
Date: Wed, 16 Jun 2010 16:26:12 -0400

Julian Foad wrote:
> Previously, svn_io_dir_walk() reported only files and dirs. With this
> change it reports symlinks in the same way that it reports files (even
> if they point to a directory).

The function has always reported files and directories using exactly the
same construct: a call to the callback function with the apr_finto_t for
that item. The implementer of the callback had to examine the apr_finfo_t
to see whether the item in question was a file or a directory.

The change is simply to return additional types of information. That it
does so in the same way it reports files is consistent with the prior
behavior (that is, not meaningful to note). Whether or not a symlink points
to a directory (or to nothing at all, for that matter) is further
irrelevant. So, let's rephrase the change to only include the relevant
information:

   Previously, svn_io_dir_walk() reported only files and dirs. With this
   change it reports symlinks, too.

> The question is: do we regard that change as an acceptable bug fix and
> allow it, or do we require the behaviour of the existing API to be left
> unchanged and get the "hotcopy" behaviour change some other way?

*shrug*. I highly doubt that anyone except Subversion (and then only the
hotcopy code) is using this extremely low-level API. We don't expose the
function through our swig bindings, even. That's not an argument for or
against compatibility breaking -- just a bit of a reality check in terms of
damage control. Any implementer of an svn_io_dir_walk() callback would be
looking at the apr_finfo_t.type value anyway, though perhaps assuming that
!REG == DIR (or vice-versa).

If we decide that we need to rev this API, though, then let's do the sane
thing and teach the function to return *all* the known APR file types so we
don't have to revisit this API again later. We can backport a private
implementation of the function for the purposes of fixing the bug in 1.6.x.

(While we're at it, I wonder if we shouldn't reimplement as a high-level
wrapper around a static recursive function that operates using APR-paths
instead of doing UTF8-conversion back and forth all over the place.)

-- 
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

Received on 2010-06-16 22:26:55 CEST

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