You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by kf...@collab.net on 2006/01/30 14:11:44 UTC

Re: svn commit: r18291 - in trunk/subversion: libsvn_wc tests/cmdline

lundblad@tigris.org writes:
> Log:
> Fix crash in svn status -u when an unversioned and ignored directory
> is in the repository.
> 
> Found by: Stefan Küng <to...@gmail.com>
> 
> * subversion/libsvn_wc/status.c (make_dir_baton): Don't try to get status
>   of ignored directory.
> 
> * subversion/tests/cmdline/stat_tests.py
>   (status_ignored_dir): New test.
>   (test_list): Run it.

The log says the crash happens "when an unversioned and ignored
directory is in the repository".

First, did you mean "working copy" instead of "repository"? :-)

Second, a question about the code, see below...

> --- trunk/subversion/libsvn_wc/status.c	(original)
> +++ trunk/subversion/libsvn_wc/status.c	Mon Jan 30 08:32:54 2006
> @@ -1155,13 +1155,14 @@
>    /* Order is important here.  We can't depend on parent_status->entry
>       being non-NULL until after we've checked all the conditions that
>       might indicate that the parent is unversioned ("unversioned" for
> -     our purposes includes being an external). */
> +     our purposes includes being an external or ignored item). */
>    if (parent_status
>        && (parent_status->text_status != svn_wc_status_unversioned)
>        && (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)
> +      && (parent_status->text_status != svn_wc_status_ignored)
>        && (parent_status->entry->kind == svn_node_dir)
>        && (eb->descend || (! pb)))
>      {

This conditional gets the entry's status if parent_status is *neither*
unversioned nor ignored.  Is it the case that if an object is both
ignored and unversioned, its text_status will be 'ignored' not
'unversioned'?  In other words, the crash was happening because even
though the item *was* unversioned, the 'ignore' part dominated, so the
text_status said nothing about its being unversioned?

(I have no objection to the solution, I just want to make sure I
understand what's going on.)

Thanks,
-Karl

-- 
www.collab.net  <>  CollabNet  |  Distributed Development On Demand

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: svn commit: r18291 - in trunk/subversion: libsvn_wc tests/cmdline

Posted by Julian Foad <ju...@btopenworld.com>.
Peter N. Lundblad wrote:
>>Julian Foad <ju...@btopenworld.com> writes:
[...]
>>>+    /** is unversioned but configured to be ignored */
>>>      svn_wc_status_ignored,
[...]
> +1, after checking assemble_status ins status.c.  Something similar
> applies to externals, which, for the parent directory, are unversioned.

r18297.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r18291 - in trunk/subversion: libsvn_wc tests/cmdline

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Mon, 30 Jan 2006 kfogel@collab.net wrote:

> Julian Foad <ju...@btopenworld.com> writes:
> > Since this is not entirely obvious, and furthermore I remember getting
> > confused by it (possibly by the corresponding output of "svn status"
> > at the user level) when I was new to Subversion, I think we want a
> > comment somewhere.  How about this:
> >
> > [[[
> > Clarify that the "ignored" status only applies to unversioned items.
> >
> > * subversion/include/svn_wc.h
> >    (svn_wc_status_ignored): Expand the doc string.
> > ]]]
> >
> > Index: subversion/include/svn_wc.h
> > ===================================================================
> > --- subversion/include/svn_wc.h (revision 18266)
> > +++ subversion/include/svn_wc.h (working copy)
> > @@ -1612,7 +1612,7 @@
> >       /** local mods received conflicting repos mods */
> >       svn_wc_status_conflicted,
> >
> > -    /** a resource marked as ignored */
> > +    /** is unversioned but configured to be ignored */
> >       svn_wc_status_ignored,
> >
> >       /** an unversioned resource is in the way of the versioned resource */
>
> +1 (assuming it's accurate, which I'm trusting your word on :-) ).
>
+1, after checking assemble_status ins status.c.  Something similar
applies to externals, which, for the parent directory, are unversioned.

Also, I found that svn_wc_status_{missing,obstructed} is cases where the
status item will have an entry, but we don't want to try to traverse into
a subdirectory.  That's a late answer to Malcolm's question.

Thanks,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r18291 - in trunk/subversion: libsvn_wc tests/cmdline

Posted by kf...@collab.net.
Julian Foad <ju...@btopenworld.com> writes:
> Since this is not entirely obvious, and furthermore I remember getting
> confused by it (possibly by the corresponding output of "svn status"
> at the user level) when I was new to Subversion, I think we want a
> comment somewhere.  How about this:
> 
> [[[
> Clarify that the "ignored" status only applies to unversioned items.
> 
> * subversion/include/svn_wc.h
>    (svn_wc_status_ignored): Expand the doc string.
> ]]]
> 
> Index: subversion/include/svn_wc.h
> ===================================================================
> --- subversion/include/svn_wc.h (revision 18266)
> +++ subversion/include/svn_wc.h (working copy)
> @@ -1612,7 +1612,7 @@
>       /** local mods received conflicting repos mods */
>       svn_wc_status_conflicted,
> 
> -    /** a resource marked as ignored */
> +    /** is unversioned but configured to be ignored */
>       svn_wc_status_ignored,
> 
>       /** an unversioned resource is in the way of the versioned resource */

+1 (assuming it's accurate, which I'm trusting your word on :-) ).

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r18291 - in trunk/subversion: libsvn_wc tests/cmdline

Posted by Julian Foad <ju...@btopenworld.com>.
kfogel@collab.net wrote:
> lundblad@tigris.org writes:
> 
>>Log:
>>Fix crash in svn status -u when an unversioned and ignored directory
>>is in the repository.
>>
>>Found by: Stefan Küng <to...@gmail.com>
>>
>>* subversion/libsvn_wc/status.c (make_dir_baton): Don't try to get status
>>  of ignored directory.
>>
>>* subversion/tests/cmdline/stat_tests.py
>>  (status_ignored_dir): New test.
>>  (test_list): Run it.
> 
> 
> The log says the crash happens "when an unversioned and ignored
> directory is in the repository".
> 
> First, did you mean "working copy" instead of "repository"? :-)
> 
> Second, a question about the code, see below...
> 
> 
>>--- trunk/subversion/libsvn_wc/status.c	(original)
>>+++ trunk/subversion/libsvn_wc/status.c	Mon Jan 30 08:32:54 2006
>>@@ -1155,13 +1155,14 @@
>>   /* Order is important here.  We can't depend on parent_status->entry
>>      being non-NULL until after we've checked all the conditions that
>>      might indicate that the parent is unversioned ("unversioned" for
>>-     our purposes includes being an external). */
>>+     our purposes includes being an external or ignored item). */
>>   if (parent_status
>>       && (parent_status->text_status != svn_wc_status_unversioned)
>>       && (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)
>>+      && (parent_status->text_status != svn_wc_status_ignored)
>>       && (parent_status->entry->kind == svn_node_dir)
>>       && (eb->descend || (! pb)))
>>     {
> 
> 
> This conditional gets the entry's status if parent_status is *neither*
> unversioned nor ignored.  Is it the case that if an object is both
> ignored and unversioned, its text_status will be 'ignored' not
> 'unversioned'?  In other words, the crash was happening because even
> though the item *was* unversioned, the 'ignore' part dominated, so the
> text_status said nothing about its being unversioned?

I believe the status "..._ignored" denotes a sub-set of unversioned items, and 
"..._unversioned" the rest of the unversioned items.  Therefore, yes, the 
status of such an item will be "..._ignored".

Since this is not entirely obvious, and furthermore I remember getting confused 
by it (possibly by the corresponding output of "svn status" at the user level) 
when I was new to Subversion, I think we want a comment somewhere.  How about this:

[[[
Clarify that the "ignored" status only applies to unversioned items.

* subversion/include/svn_wc.h
   (svn_wc_status_ignored): Expand the doc string.
]]]

Index: subversion/include/svn_wc.h
===================================================================
--- subversion/include/svn_wc.h (revision 18266)
+++ subversion/include/svn_wc.h (working copy)
@@ -1612,7 +1612,7 @@
      /** local mods received conflicting repos mods */
      svn_wc_status_conflicted,

-    /** a resource marked as ignored */
+    /** is unversioned but configured to be ignored */
      svn_wc_status_ignored,

      /** an unversioned resource is in the way of the versioned resource */

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r18291 - in trunk/subversion: libsvn_wc tests/cmdline

Posted by kf...@collab.net.
"Peter N. Lundblad" <pe...@famlundblad.se> writes:

> On Mon, 30 Jan 2006 kfogel@collab.net wrote:
> 
> > lundblad@tigris.org writes:
> > > Log:
> > > Fix crash in svn status -u when an unversioned and ignored directory
> > > is in the repository.
> > >
> > > Found by: Stefan Küng <to...@gmail.com>
> > >
> > > * subversion/libsvn_wc/status.c (make_dir_baton): Don't try to get status
> > >   of ignored directory.
> > >
> > > * subversion/tests/cmdline/stat_tests.py
> > >   (status_ignored_dir): New test.
> > >   (test_list): Run it.
> >
> > The log says the crash happens "when an unversioned and ignored
> > directory is in the repository".
> >
> > First, did you mean "working copy" instead of "repository"? :-)
> >
> No, I menat repository. The crash happens when there is an item that is
> ignored (i.e. matches the ignore patterns), and thereby, by definition
> unversioned (see below) and then an entry by the same name is in the
> repository.  This will be reported as "added" by the repository status
> code.  Maybe I was too terse here.

You can just say "...is also in the repository" to clarify.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: svn commit: r18291 - in trunk/subversion: libsvn_wc tests/cmdline

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Mon, 30 Jan 2006 kfogel@collab.net wrote:

> lundblad@tigris.org writes:
> > Log:
> > Fix crash in svn status -u when an unversioned and ignored directory
> > is in the repository.
> >
> > Found by: Stefan Küng <to...@gmail.com>
> >
> > * subversion/libsvn_wc/status.c (make_dir_baton): Don't try to get status
> >   of ignored directory.
> >
> > * subversion/tests/cmdline/stat_tests.py
> >   (status_ignored_dir): New test.
> >   (test_list): Run it.
>
> The log says the crash happens "when an unversioned and ignored
> directory is in the repository".
>
> First, did you mean "working copy" instead of "repository"? :-)
>
No, I menat repository. The crash happens when there is an item that is
ignored (i.e. matches the ignore patterns), and thereby, by definition
unversioned (see below) and then an entry by the same name is in the
repository.  This will be reported as "added" by the repository status
code.  Maybe I was too terse here.

 > Second, a question about the code, see below...
>
> > --- trunk/subversion/libsvn_wc/status.c	(original)
> > +++ trunk/subversion/libsvn_wc/status.c	Mon Jan 30 08:32:54 2006
> > @@ -1155,13 +1155,14 @@
> >    /* Order is important here.  We can't depend on parent_status->entry
> >       being non-NULL until after we've checked all the conditions that
> >       might indicate that the parent is unversioned ("unversioned" for
> > -     our purposes includes being an external). */
> > +     our purposes includes being an external or ignored item). */
> >    if (parent_status
> >        && (parent_status->text_status != svn_wc_status_unversioned)
> >        && (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)
> > +      && (parent_status->text_status != svn_wc_status_ignored)
> >        && (parent_status->entry->kind == svn_node_dir)
> >        && (eb->descend || (! pb)))
> >      {
>
> This conditional gets the entry's status if parent_status is *neither*
> unversioned nor ignored.  Is it the case that if an object is both
> ignored and unversioned, its text_status will be 'ignored' not
> 'unversioned'?  In other words, the crash was happening because even
> though the item *was* unversioned, the 'ignore' part dominated, so the
> text_status said nothing about its being unversioned?
>
I think that's the definition of the ignored status. How could a path be
versioned and ignored? So it makes sense that this value dominates.

Regarding Malcolm's question about simplifying the conditional expression,
I could go either way.  I think (but am too lazy to check right now) that
other code in this file relies on entry being NULL to mean "unversioned".
But, this being a change intended for backport, I don't want to do this
simplicifaction in the fix.

Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org