Just doing a quick check to see what the status of this is? I am excited to
see a change like this introduced.
Thanks,
Jonathan Malek
On 8/17/05, Branko Èibej <brane@xbc.nu> wrote:
>
> kfogel@collab.net wrote:
>
> >Branko Èibej <brane@xbc.nu> writes:
> >
> >
> >>Index: subversion/libsvn_subr/opt.c
> >>===================================================================
> >>--- subversion/libsvn_subr/opt.c (revision 15318)
> >>+++ subversion/libsvn_subr/opt.c (working copy)
> >>@@ -654,6 +654,10 @@
> >> target a SVN admin dir unless svn_wc_check_wc passes on
> >> the target, too? */
> >> base_name = svn_path_basename (target, pool);
> >>+ /* FIXME: How to avoid the circular dependency between
> >>+ linsvn_wc and libsvn_subr here?
> >>+ if (svn_wc_is_adm_dir (base_name, pool))
> >>+ */
> >> if (! strcmp (base_name, SVN_WC_ADM_DIR_NAME))
> >> continue;
> >> }
> >>
> >>
> >
> >Yeah, this is a problem.
> >
> >We could break the svn_opt stuff out into its own library. While it
> >would be a bit odd for libsvn_opt to depend on libsvn_wc, it wouldn't
> >actually be circular.
> >
> >
> That would break ABI compatiblity, wouldn't it?
>
> Although, if we can work around that, I'd rather create the
> libsvn_cmdline I proposed ages ago. All the svn_cmdline and svn_opt
> stuff would go there.
>
> (And I can now take immense pleasure and say "I told you so" to everyone
> who opposed the libsvn_cmdline idea in the first place. :)
>
>
> >Another solution would be for svn_opt_args_to_target_array2() to stop
> >skipping these, and for its callers to instead call some new svn_wc
> >function that takes a target array and removes the ".svn" (etc)
> >members from it. At svn_opt_args_to_target_array2(), we'd simply
> >document that calling svn_wc_clean_targets() (or whatever) is highly
> >recommended.
> >
> >Thoughts?
> >
> >
> I don't think that's a good idea. It would be changing our API
> semantics, and not for a bug fix.
>
>
> >>+ svn_wc_is_adm_dir (temp = APR_ARRAY_IDX (new_entries, +
> >>new_entries->nelts - 1, +
> >>const char *),
> >>+ pool))
> >>
> >>
> >Uck, the in-call assignment of temp bothers me, but not enough to get
> >up and do anything about it :-).
> >
> >
> Oh, I can fix that, I suppose, although the change would make the code
> less elegant. :)
>
> >Nice change, overall, if we can get that circular-dependency issue
> >resolved.
> >
> >
> I suppose that, for 1.x, we could cheat. Indeed,
> svn_opt_args_to_target_array should filter out all possible admin
> directory names, and we do want to keep that list short (probably just
> .svn and _svn). Maybe we should just maintain the list in two places for
> now, with ample comments and warnings in both.
>
> -- Brane
>
>
Received on Sun Sep 18 18:56:10 2005