You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Peter Samuelson <pe...@p12n.org> on 2010/02/18 02:27:59 UTC

[PATCH] Re: svn: Can't check path '/.svn/wc.db': Permission denied

Jon Delay writes:
> I keep most of my file system under svn, so I have .svn directories all over
> the place owned by root, chmod 700.
> 
> Somewhere between 1.6.3 and 1.6.9, `svn checkout` started checking the root directory
> for the existence of an .svn directory and trying to look in it.

...and getting "Permission denied", and thus he cannot use 'svn checkout'
in his home directory.

The following (untested) patch will probably fix this - but is it a good
idea?  Are there situations where we _want_ to catch errors trying to
detect a svn 1.7 wc in svn 1.6?  I personally don't see why we need to.

Peter

[[[
* subversion/libsvn_wc/questions.c
  (is_inside_wc_ng): Ignore errors trying to access .svn/wc.db files in
   parent directories.  Worst case, we allow a user to create a 1.6 wc
   inside a 1.7 wc.
]]]

--- subversion/libsvn_wc/questions.c
+++ subversion/libsvn_wc/questions.c
@@ -51,11 +51,16 @@ is_inside_wc_ng(const char *abspath,
                 apr_pool_t *pool)
 {
   svn_node_kind_t kind;
+  svn_error_t *err;
   const char *wc_db_path = svn_path_join_many(pool, abspath, ".svn", "wc.db",
                                               NULL);
 
-  SVN_ERR(svn_io_check_path(wc_db_path, &kind, pool));
-
+  err = svn_io_check_path(wc_db_path, &kind, pool);
+  if (err)
+    {
+      svn_error_clear(err);
+      return SVN_NO_ERROR;
+    }
   if (kind == svn_node_file)
     {
       /* This value is completely bogus, but it is much higher than 1.6 will

Re: [PATCH] Re: svn: Can't check path '/.svn/wc.db': Permission denied

Posted by Jon Daley <jo...@limedaley.com>.
> On Thu, 18 Feb 2010, Julian Foad wrote:
>> Peter Samuelson wrote:
>>> [Stefan Sperling]
>>>> Have you seen ^/subversion/branches/1.6.x-wc-ng-check-override ?
>>>> I think that's the workaround you need. Log of the branch below.
>>>> It's already nominated for backport, so if you like it, please vote :)
>>> 
>>> I hadn't seen it.  Yes, this solves the problem - but I'm not sure this
>>> 'permission denied' should really require a workaround like
>>>
>>>     export 
>>> SVN_I_LOVE_CORRUPTED_WORKING_COPIES_SO_DISABLE_CHECK_FOR_WC_NG=yes
>>> 
>>> Jon, what's your opinion on Stefan's approach?
>> 
>> I'm not Jon but my opinion is that we need a bug fix, not a work-around,
>> and that Peter's patch
>> <http://svn.haxx.se/dev/archive-2010-02/0431.shtml> is the sort of fix
>> we need. (I haven't reviewed it, just glanced over it.)
>> 
>> Let's check I understood the problem correctly: User has 1.6 client and
>> 1.6 WCs, no 1.7 stuff involved at all. User's system has a WC rooted at
>> the file system root: '/.svn' exists and is not readable by this user.
>> In some normal operations that used to work with svn 1.6.x, svn 1.6.9
>> tries to look for a file '/.svn/wc.db' and throws an error because it's
>> denied access to that directory.

 	Not sure if this thread died - I thought maybe it was just because 
I wasn't subscribed to the list, but I don't see any responses on the web 
page.

 	I had another thought about this that I wanted to make sure people 
had thought of for 1.7 - I thought it strange that I experienced my 
problem with /.svn rather than /home/.svn and I'm wondering if the 
directories are being traversed in the right order when looking for the 
root .svn directory.  If I understand what 1.7 is going to do, then I 
think the search for the root node (if it isn't stored somewhere else) 
that you have to go up one directory at a time, and you can't start with 
/.svn first.  Just a thought, and probably you've already thought about 
it, but I'm not sure why svn 1.6 didn't complain about the permissions of 
a subdirectory first.


-- 
Jon Daley
http://jon.limedaley.com
~~
There are no traffic jams when you go the extra mile.
-- Anonymous

Re: [PATCH] Re: svn: Can't check path '/.svn/wc.db': Permission denied

Posted by Jon Daley <de...@jon.limedaley.com>.
On Thu, 18 Feb 2010, Julian Foad wrote:
> Peter Samuelson wrote:
>> [Stefan Sperling]
>>> Have you seen ^/subversion/branches/1.6.x-wc-ng-check-override ?
>>> I think that's the workaround you need. Log of the branch below.
>>> It's already nominated for backport, so if you like it, please vote :)
>>
>> I hadn't seen it.  Yes, this solves the problem - but I'm not sure this
>> 'permission denied' should really require a workaround like
>>
>>     export SVN_I_LOVE_CORRUPTED_WORKING_COPIES_SO_DISABLE_CHECK_FOR_WC_NG=yes
>>
>> Jon, what's your opinion on Stefan's approach?
>
> I'm not Jon but my opinion is that we need a bug fix, not a work-around,
> and that Peter's patch
> <http://svn.haxx.se/dev/archive-2010-02/0431.shtml> is the sort of fix
> we need. (I haven't reviewed it, just glanced over it.)
>
> Let's check I understood the problem correctly: User has 1.6 client and
> 1.6 WCs, no 1.7 stuff involved at all. User's system has a WC rooted at
> the file system root: '/.svn' exists and is not readable by this user.
> In some normal operations that used to work with svn 1.6.x, svn 1.6.9
> tries to look for a file '/.svn/wc.db' and throws an error because it's
> denied access to that directory.

Correct.  And Peter's simpler example in the debian bug page:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=570271
is a little different, but the same issue.

One other clarification is that the WC in one of the parent directories is 
completely unrelated to the WC in the subdirectory.

ie. the repository (and/or method and path) for / is completely different 
than the repository for /home/me/test/ and so anything it is looking for 
in /home would be wrong, even in the case of the new 1.7 feature where 
there is only one .svn directory for each checkout, because in my example 
both / and /home/me/test would both have a .svn directory in it.

I don't know the code base well enough (or even at all), but it seems to 
me that Peter's patch looked good.

I am also uneasy about the "I_LOVE_CORRUPTED_WCS" flag as a fix to this 
problem, as I don't particularly love them at all, and if there were 
corruption, I would want to know about it, and not silently ignore it.


-- 
Jon Daley
http://jon.limedaley.com
~~
I refuse to join any club that would have me as a member.
-- Groucho Marx

Re: [PATCH] Re: svn: Can't check path '/.svn/wc.db': Permission denied

Posted by Peter Samuelson <pe...@p12n.org>.
[Julian Foad]
> Let's check I understood the problem correctly: User has 1.6 client
> and 1.6 WCs, no 1.7 stuff involved at all. User's system has a WC
> rooted at the file system root: '/.svn' exists and is not readable by
> this user.  In some normal operations that used to work with svn
> 1.6.x, svn 1.6.9 tries to look for a file '/.svn/wc.db' and throws an
> error because it's denied access to that directory.

All correct.

The main thing to ask about my patch is that it ignores _all_ errors
trying to read .../.svn/wc.db, not just 'permission denied'.  Are there
any errors that we would _want_ to catch?  My feeling is no, if there's
any trouble of any sort, we should just stop looking for 1.7 WCs.
-- 
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/

Re: [PATCH] Re: svn: Can't check path '/.svn/wc.db': Permission denied

Posted by Julian Foad <ju...@wandisco.com>.
Peter Samuelson wrote:
> [Stefan Sperling]
> > Have you seen ^/subversion/branches/1.6.x-wc-ng-check-override ?
> > I think that's the workaround you need. Log of the branch below.
> > It's already nominated for backport, so if you like it, please vote :)
> 
> I hadn't seen it.  Yes, this solves the problem - but I'm not sure this
> 'permission denied' should really require a workaround like
> 
>     export SVN_I_LOVE_CORRUPTED_WORKING_COPIES_SO_DISABLE_CHECK_FOR_WC_NG=yes
> 
> Jon, what's your opinion on Stefan's approach?

I'm not Jon but my opinion is that we need a bug fix, not a work-around,
and that Peter's patch
<http://svn.haxx.se/dev/archive-2010-02/0431.shtml> is the sort of fix
we need. (I haven't reviewed it, just glanced over it.)

Let's check I understood the problem correctly: User has 1.6 client and
1.6 WCs, no 1.7 stuff involved at all. User's system has a WC rooted at
the file system root: '/.svn' exists and is not readable by this user.
In some normal operations that used to work with svn 1.6.x, svn 1.6.9
tries to look for a file '/.svn/wc.db' and throws an error because it's
denied access to that directory.

- Julian


Re: [PATCH] Re: svn: Can't check path '/.svn/wc.db': Permission denied

Posted by Peter Samuelson <pe...@p12n.org>.
[Stefan Sperling]
> Have you seen ^/subversion/branches/1.6.x-wc-ng-check-override ?
> I think that's the workaround you need. Log of the branch below.
> It's already nominated for backport, so if you like it, please vote :)

I hadn't seen it.  Yes, this solves the problem - but I'm not sure this
'permission denied' should really require a workaround like

    export SVN_I_LOVE_CORRUPTED_WORKING_COPIES_SO_DISABLE_CHECK_FOR_WC_NG=yes

Jon, what's your opinion on Stefan's approach?

Peter

> On the 1.6.x-wc-ng-check-override branch:
> 
> Allow overriding the check for WC-NG working copies in 1.6.x,
> by setting the environment variable
> SVN_I_LOVE_CORRUPTED_WORKING_COPIES_SO_DISABLE_CHECK_FOR_WC_NG to "yes".
> 
> Make the test suite run all tests with this override enabled.
> 
> This is useful when running the 1.6.x test suite in a working copy
> managed by a trunk svn client. Many tests fail to pass because of
> errors like:
>   svn: The path 'svn-test-work/working_copies/some_tests-42'
>        appears to be part of a Subversion 1.7 or greater working
>        copy rooted at '/home/user/svn-1.6.x/subversion/tests/cmdline'.
> 
> With this patch, all tests pass on my machine.

Re: [PATCH] Re: svn: Can't check path '/.svn/wc.db': Permission denied

Posted by Stefan Sperling <st...@elego.de>.
On Wed, Feb 17, 2010 at 08:27:59PM -0600, Peter Samuelson wrote:
> 
> Jon Delay writes:
> > I keep most of my file system under svn, so I have .svn directories all over
> > the place owned by root, chmod 700.
> > 
> > Somewhere between 1.6.3 and 1.6.9, `svn checkout` started checking the root directory
> > for the existence of an .svn directory and trying to look in it.
> 
> ...and getting "Permission denied", and thus he cannot use 'svn checkout'
> in his home directory.
> 
> The following (untested) patch will probably fix this - but is it a good
> idea?  Are there situations where we _want_ to catch errors trying to
> detect a svn 1.7 wc in svn 1.6?  I personally don't see why we need to.

Have you seen ^/subversion/branches/1.6.x-wc-ng-check-override ?
I think that's the workaround you need. Log of the branch below.
It's already nominated for backport, so if you like it, please vote :)

Stefan

------------------------------------------------------------------------
r910215 | stsp | 2010-02-15 14:16:49 +0100 (Mon, 15 Feb 2010) | 30 lines

On the 1.6.x-wc-ng-check-override branch:

Allow overriding the check for WC-NG working copies in 1.6.x,
by setting the environment variable
SVN_I_LOVE_CORRUPTED_WORKING_COPIES_SO_DISABLE_CHECK_FOR_WC_NG to "yes".

Make the test suite run all tests with this override enabled.

This is useful when running the 1.6.x test suite in a working copy
managed by a trunk svn client. Many tests fail to pass because of
errors like:
  svn: The path 'svn-test-work/working_copies/some_tests-42'
       appears to be part of a Subversion 1.7 or greater working
       copy rooted at '/home/user/svn-1.6.x/subversion/tests/cmdline'.

With this patch, all tests pass on my machine.

* subversion/libsvn_wc/questions.c
  (SVN_WC_NG_CHECK_ENV_VAR): New.
  (is_inside_wc_ng): Check for override. While here, replace use of
   literal ".svn" with the proper macro definition.

* subversion/tests/cmdline/svntest/actions.py
  (no_check_for_wc_ng, do_check_for_wc_ng): New. The second function
   is not used yet, but could be used for tests which fail to pass
   with the override enabled. I have found no such test.

* subversion/tests/cmdline/svntest/main.py
  (TestRunner.run): Always enable the override.

------------------------------------------------------------------------
r910213 | stsp | 2010-02-15 14:07:10 +0100 (Mon, 15 Feb 2010) | 2 lines

Create a branch for a patch I have for 1.6.x.

------------------------------------------------------------------------

Re: [PATCH] Re: svn: Can't check path '/.svn/wc.db': Permission denied

Posted by Peter Samuelson <pe...@p12n.org>.
> Jon Delay writes:
> > I keep most of my file system under svn, so I have .svn directories all over
> > the place owned by root, chmod 700.
> > 
> > Somewhere between 1.6.3 and 1.6.9, `svn checkout` started checking the root directory
> > for the existence of an .svn directory and trying to look in it.

[Peter Samuelson]
> ...and getting "Permission denied", and thus he cannot use 'svn checkout'
> in his home directory.
> 
> The following (untested) patch will probably fix this - but is it a good
> idea?  Are there situations where we _want_ to catch errors trying to
> detect a svn 1.7 wc in svn 1.6?  I personally don't see why we need to.

Stefan reminded me of this issue, so I created branch 1.6.x-wc-ng-error
with the patch (tested now) and nominated it for 1.6.10.

Daniel Shahaf asked me the same question I quoted above: are there
specific errors we would _want_ to catch in is_inside_wc_ng()?  I still
think the answer is probably no.  It is not is_inside_wc_ng's job to
discover random errors related to accessing things in parent
directories, errors that likely have nothing to do with wc-ng or
Subversion.

...Either way, since Hyrum announced he's rolling 1.6.10 tomorrow, this
patch may or may not make it in time.

Peter