kfogel@collab.net writes:
> kfogel@collab.net writes:
> > "Rowell, Geoff" <growell@ENVOYWW.COM> writes:
> > > I repositioned some externals in our corporate repository. A few
> > > subfolders that used to be externals are now actual subfolders. One
> > > of the developers ran a "svn status -u" command and it crashed.
> >
> > Thank you! I have reproduced this with the latest trunk code, using
> > your recipe. I am debugging now.
>
> Here's a patch. It passes 'make check', but I haven't committed yet
> because I'm not completely confident it's the right solution. I'm
> worried that it might prevent status from doing the right thing on
> svn:externals in some situations, situations that happen not to be
> covered by our test suite.
>
> If anyone has any thoughts about this fix, please follow up.
>
> [[[
> Fix bug in status's handling of svn:externals directories.
>
> Thanks to Geoff Rowell <growell@envoyww.com>, who described this bug in
> http://subversion.tigris.org/servlets/ReadMsg?list=users&msgNo=30489,
> <5C58E90D72EB654D9C62E216338A1E5D70056B@Clavin.corp01.envoyww.com>,
> subject "svn status -u crash w/ moved external".
>
> * subversion/libsvn_wc/status.c
> (make_dir_baton): Don't get status for children of external directories.
> ]]]
I'm confused as to how this actually solves the problem. As far as I
can tell, svn_wc_status_external is only ever set as a last minute
step (in send_unversioned_item()) just prior to transmitting that one
status object through the status_func/baton. I've not run through a
debugger yet, but I can't see a codepath with my sleepy eyes that
would actually have any status structure which is stored and used from
within the libsvn_wc/status.cparent_status->text_status actually set to
svn_wc_status_external.
Oh, wait. I see. Sometimes status_func isn't the "real" callback --
it is sometimes set to hash_stash(), which squirrels the hash away in
a parent baton's status hash. Clever coder, that C-Mike... ;-)
In that case, yes, I think this code is correct. I'm not fond of the
mid-boolean-logic comment placement, and, what's more, would prefer
that the code also explicitly check for the SEGFAULT cause -- that
parent_status->entry is NULL.
But I think you can confidently commit your solution knowing that
svn_wc_status_external is only ever set on unversioned items. In
other words, svn_wc_status_external is just a particular flavor of
unversioned item -- we chose to reveal an item as external by using
unique status->text_status value instead of some new boolean
status->is_external. And this conditional's job is to rule out (among
other things) unversioned items.
>
> Index: subversion/libsvn_wc/status.c
> ===================================================================
> --- subversion/libsvn_wc/status.c (revision 14510)
> +++ subversion/libsvn_wc/status.c (working copy)
> @@ -1043,6 +1043,10 @@
> && (parent_status->text_status != svn_wc_status_deleted)
> && (parent_status->text_status != svn_wc_status_missing)
> && (parent_status->text_status != svn_wc_status_obstructed)
> + && (parent_status->text_status != svn_wc_status_external)
> + /* Order is important here. If parent_status->text_status is
> + svn_wc_status_external, then parent_status->entry is NULL,
> + so check the former before looking inside the latter. */
> && (parent_status->entry->kind == svn_node_dir)
> && (eb->descend || (! pb)))
> {
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Apr 29 09:45:40 2005