On 04/19/2012 09:26 AM, Hyrum K Wright wrote:
> I've been poking around import, and I discovered this snippet of
> decade-old code:
>
> /* The repository doesn't know about the reserved administrative
> directory. */
> if (new_entries->nelts
> /* What's this, what's this? This assignment is here because we
> use the value to construct the error message just below. It
> may not be aesthetically pleasing, but it's less ugly than
> calling APR_ARRAY_IDX twice. */
> && svn_wc_is_adm_dir(temp = APR_ARRAY_IDX(new_entries,
> new_entries->nelts - 1,
> const char *),
> scratch_pool))
> return svn_error_createf
> (SVN_ERR_CL_ADM_DIR_RESERVED, NULL,
> _("'%s' is a reserved name and cannot be imported"),
> svn_dirent_local_style(temp, scratch_pool));
>
> new_entries is the list of basenames of to-be-created parents (similar
> to what happens with 'mkdir --parents'). It appears this is a very
> specific check that if the second-to-last of those basenames is the wc
> admin directory, don't do the import.
>
> I'm trying to figure out if this is still a valid check. It feels
> like it's looking at a very specific case, and there are any number of
> ways a determined user to circumvent this check. And even if they
> did, it's not illegal, folks would just end up with '.svn' in the
> repository.
>
> It feels like this is just a relic of a bygone era when users were
> still experimenting with Subversion, and I just have a hard time
> seeing it as needed these days.
>
> Thoughts?
As you know, code doesn't become useless just because it's a decade old. :-)
We decided as a project to disallow imports of the .svn/ directory, not
because the repository couldn't handle it, or because the project was young
and naive (as you imply), but because a subsequent checkout of the
repository at the parent at the .svn/ directory would be impossible. (The
checkout would fail to create the versioned .svn/ child because .svn/ would
already be present on disk, etc.)
Certainly, with Subversion 1.7 that's a bit less of an issue because a .svn/
subdirectory in the checkout dataset is, in theory, problematic only when it
would appear at the top-level of the working copy. But admins can't really
control which version of Subversion is used to access their repositories,
and this just seems like one of those areas where it's more likely that
folks will accidentally try something that's stupid than that they are
intentionally trying something clever. Besides, as you say, if they
*really* want to do this, there are workarounds.
Summary: I see no reason to relax the check. As I've said before: "Guns
don't kill people; people kill people -- but that doesn't mean we hand out
loaded guns as party favors."
--
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Received on 2012-04-19 17:30:27 CEST