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 2005/04/29 03:30:53 UTC

Re: svn status -u crash w/ moved external

"Rowell, Geoff" <gr...@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.

-Karl

> Here's the simplest way to reproduce the problem.
> 
> System:
>   XP SP2
>   svn, version 1.1.4 (r13838)
> 
> How to reproduce:
>   c:
>   chdir \
>   mkdir repos
>   svnadmin create repos
>   svn mkdir file:///repos/main -m "Created main"
>   svn mkdir file:///repos/other -m "Created other"
>   svn co file:///repos/main main
>   svn ps svn:externals "other file:///repos/other" main
>   svn ci main -m "Added external other"
>   svn co file:///repos/main main2
>   svn cp file:///repos/other file:///repos/main/other -m "Moved other"
>   svn ps svn:externals "" main
>   svn up main
>   svn ci main -m "Removed external other"
>   svn status -u main2
> 
> BTW, the developer was running SVN 1.1.3.
> 
> Geoff Rowell
> Quality Assurance
> EnvoyWorldWide, Inc.
> www.envoyworldwide.com
>  
> Notifications on time, every time. Guaranteed.
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: users-help@subversion.tigris.org

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


Re: [PATCH]: Re: svn status -u crash w/ moved external

Posted by Philip Martin <ph...@codematters.co.uk>.
kfogel@collab.net writes:

> "C. Michael Pilato" <cm...@collab.net> writes:
>
>> and, what's more, would prefer
>> that the code also explicitly check for the SEGFAULT cause -- that
>> parent_status->entry is NULL.  
>
> I'd rather not do that; we could cover up a lot of bugs that way.
>
> Basically, we have every right to expect that parent_status->entry
> exists *unless* one of several possible unversioned scenarios is true.
> We should check for those scenarios, but not cover up the problem if
> parent_status->entry is unexpectedly NULL.

I remember Steve King reporting some TSVN crashes where the immediate
cause was something like parent_status->entry being NULL, but when we
looked at the code we could not determine how it could happen.  I
never considered svn:externals.

-- 
Philip Martin

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

Re: [PATCH]: Re: svn status -u crash w/ moved external

Posted by kf...@collab.net.
"C. Michael Pilato" <cm...@collab.net> writes:
> In that case, yes, I think this code is correct.  I'm not fond of the
> mid-boolean-logic comment placement, 

Happy to change that.

> and, what's more, would prefer
> that the code also explicitly check for the SEGFAULT cause -- that
> parent_status->entry is NULL.  

I'd rather not do that; we could cover up a lot of bugs that way.

Basically, we have every right to expect that parent_status->entry
exists *unless* one of several possible unversioned scenarios is true.
We should check for those scenarios, but not cover up the problem if
parent_status->entry is unexpectedly 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.

Thanks.  I also did some more investigating and realized that one of
my fears about the patch was groundless: status on the external
directory within the target is still performed as usual.  That was
what I'd worried might break.

Committed in r14514.

-Karl

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

Re: Mail threading [was: Authentication/authorization issues]

Posted by Julian Foad <ju...@btopenworld.com>.
kfogel@collab.net wrote:
> Julian Foad <ju...@btopenworld.com> writes:
>>This is sound advice.  I suggest two tweaks:
> 
> +1, go for it.  The verb tense in that new paragraph seemed a bit odd
> to me, you might try:

Done.

- Julian

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

Re: Mail threading [was: Authentication/authorization issues]

Posted by kf...@collab.net.
Julian Foad <ju...@btopenworld.com> writes:
> kfogel@collab.net wrote:
> > Please see
> >    http://subversion.tigris.org/mailing-list-guidelines.html#replying
> 
> This is sound advice.  I suggest two tweaks:

+1, go for it.  The verb tense in that new paragraph seemed a bit odd
to me, you might try:

   <p>(The root of the problem is really that some mail interfaces do
   not indicate that the message generated by the "Reply" function is
   different from a fresh message.  If you use such a program, consider
   submitting an enhancement request or a patch to its developers to make
   it show a distinction.)</p>

-K

> Index: ../subversion/www/mailing-list-guidelines.html
> ===================================================================
> --- ../subversion/www/mailing-list-guidelines.html      (revision 14462)
> +++ ../subversion/www/mailing-list-guidelines.html      (working copy)
> @@ -123,7 +123,8 @@
>   <a name="fresh-post"></a>
> 
>   <p>Don't start a new thread (subject) by replying to an existing
> -post.  Instead, start a fresh mail, and write out the list address by
> +post.  Instead, start a fresh mail, even if that means you have to
> +write out the list address by
>   hand.  If you reply to an existing post, your mailreader may include
>   metadata that marks your post as a followup in that thread.  Changing
>   the <tt>Subject</tt> header is not enough to prevent this!  Many
> @@ -132,7 +133,14 @@
>   your post (because they're ignoring that thread), but people who are
>   reading the thread will waste their time with your off-topic post.
>   The safest way to avoid this is to never use "reply" to start a new
> -topic.</p></li>
> +topic.</p>
> +
> +<p>(The root of the problem is really that mail user interfaces are
> +not indicating that the message generated by the "Reply" function is
> +different from a fresh message.  If you use such a program, consider
> +submitting an enhancement request or a patch to its developers to make
> +it show a distinction.)</p>
> +</li>
> 
>   <li>
>   <a name="rethreading"></a>
> 
> - Julian
> 
> ---------------------------------------------------------------------
> 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

Mail threading [was: Authentication/authorization issues]

Posted by Julian Foad <ju...@btopenworld.com>.
kfogel@collab.net wrote:
> Please see
> 
>    http://subversion.tigris.org/mailing-list-guidelines.html#replying

This is sound advice.  I suggest two tweaks:

Index: ../subversion/www/mailing-list-guidelines.html
===================================================================
--- ../subversion/www/mailing-list-guidelines.html      (revision 14462)
+++ ../subversion/www/mailing-list-guidelines.html      (working copy)
@@ -123,7 +123,8 @@
  <a name="fresh-post"></a>

  <p>Don't start a new thread (subject) by replying to an existing
-post.  Instead, start a fresh mail, and write out the list address by
+post.  Instead, start a fresh mail, even if that means you have to
+write out the list address by
  hand.  If you reply to an existing post, your mailreader may include
  metadata that marks your post as a followup in that thread.  Changing
  the <tt>Subject</tt> header is not enough to prevent this!  Many
@@ -132,7 +133,14 @@
  your post (because they're ignoring that thread), but people who are
  reading the thread will waste their time with your off-topic post.
  The safest way to avoid this is to never use "reply" to start a new
-topic.</p></li>
+topic.</p>
+
+<p>(The root of the problem is really that mail user interfaces are
+not indicating that the message generated by the "Reply" function is
+different from a fresh message.  If you use such a program, consider
+submitting an enhancement request or a patch to its developers to make
+it show a distinction.)</p>
+</li>

  <li>
  <a name="rethreading"></a>

- Julian

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

Re: Authentication/authorization issues

Posted by kf...@collab.net.
Please see

   http://subversion.tigris.org/mailing-list-guidelines.html#replying

for why it's not good to start a new subject by replying to an
existing thread:

   Don't start a new thread (subject) by replying to an existing
   post. Instead, start a fresh mail, and write out the list address
   by hand. If you reply to an existing post, your mailreader may
   include metadata that marks your post as a followup in that
   thread. Changing the Subject header is not enough to prevent this!
   Many mailreaders will still preserve enough metadata to put your
   post in the wrong thread. If this happens, not only will some
   people not see your post (because they're ignoring that thread),
   but people who are reading the thread will waste their time with
   your off-topic post. The safest way to avoid this is to never use
   "reply" to start a new topic.

Thank you,
-Karl

"Gino Marckx" <de...@xodiac.be> writes:
> Hi there,
> 
> 1. I have a mixed authentication and anonymous access setup for my
> Subversion installation and wanted to do the following for the test
> repository (which everybody can use to fool around with and get acquainted
> with Subversion in our company).
> So it is obvious that everybody has got write access, so the following
> line is added to the [test:/] section of my subversion access policy file:
> 
>   * = rw
> 
> Now, for obvious reasons, locking cannot be used.  So I added the
> following line to the section for the developers to be able to test
> locking when they wanted to:
> 
>   @developers = rw
> 
> Apparently this does not work.  Apache always seems to take the anonymous
> account in this case, and I get the message:
> 
>   svn: Lock request failed: 401 Authorization Required
> 
> Is there any way to do what I want to do?  Because I don't want to specify
> all non-developer users in the subversion access policy file...
> 
> 2. Another thing.  I think it would be very useful to be able to refer to
> user groups when specifying other users.  E.g. like in the following
> example:
> 
>   admins = admin1, admin2
>   developers = @admins, developer1, developer2, ...
> 
> 3. Currently it is not possible to automatically secure (write lock) all
> tags.  If it would be possible to specify something like:
> 
>   [test:/tags/*]
>   * = r
> 
> Then this would automatically be the case.
> 
> Regards,
> Gino.
> 
> 
> ---------------------------------------------------------------------
> 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

Re: Authentication/authorization issues

Posted by Marcus Rueckert <da...@web.de>.
On 2005-04-29 11:27:25 +0200, Gino Marckx wrote:
how does this relate to the svn status bug?

> 1.  use 2 location blocks. less trouble.

> 2. Another thing.  I think it would be very useful to be able to refer to
> user groups when specifying other users.  E.g. like in the following
> example:
> 
>   admins = admin1, admin2
>   developers = @admins, developer1, developer2, ...

fixed in 1.2
 
> 3. Currently it is not possible to automatically secure (write lock) all
> tags.  If it would be possible to specify something like:
> 
>   [test:/tags/*]
>   * = r

[test:/tags/]
* = r
@releasemanager = rw

done

darix

-- 
irssi - the client of the smart and beautiful people

              http://www.irssi.de/


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

Authentication/authorization issues

Posted by Gino Marckx <de...@xodiac.be>.
Hi there,

1. I have a mixed authentication and anonymous access setup for my
Subversion installation and wanted to do the following for the test
repository (which everybody can use to fool around with and get acquainted
with Subversion in our company).
So it is obvious that everybody has got write access, so the following
line is added to the [test:/] section of my subversion access policy file:

  * = rw

Now, for obvious reasons, locking cannot be used.  So I added the
following line to the section for the developers to be able to test
locking when they wanted to:

  @developers = rw

Apparently this does not work.  Apache always seems to take the anonymous
account in this case, and I get the message:

  svn: Lock request failed: 401 Authorization Required

Is there any way to do what I want to do?  Because I don't want to specify
all non-developer users in the subversion access policy file...

2. Another thing.  I think it would be very useful to be able to refer to
user groups when specifying other users.  E.g. like in the following
example:

  admins = admin1, admin2
  developers = @admins, developer1, developer2, ...

3. Currently it is not possible to automatically secure (write lock) all
tags.  If it would be possible to specify something like:

  [test:/tags/*]
  * = r

Then this would automatically be the case.

Regards,
Gino.


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

Re: [PATCH]: Re: svn status -u crash w/ moved external

Posted by "C. Michael Pilato" <cm...@collab.net>.
kfogel@collab.net writes:

> kfogel@collab.net writes:
> > "Rowell, Geoff" <gr...@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 <gr...@envoyww.com>, who described this bug in
> http://subversion.tigris.org/servlets/ReadMsg?list=users&msgNo=30489,
> <5C...@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

[PATCH]: Re: svn status -u crash w/ moved external

Posted by kf...@collab.net.
kfogel@collab.net writes:
> "Rowell, Geoff" <gr...@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 <gr...@envoyww.com>, who described this bug in
http://subversion.tigris.org/servlets/ReadMsg?list=users&msgNo=30489,
<5C...@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.
]]]

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