You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Hyrum K Wright <hy...@hyrumwright.org> on 2011/04/14 17:47:20 UTC

Re: svn commit: r1091928 - in /subversion/trunk/subversion: libsvn_client/commit.c tests/cmdline/commit_tests.py

On Wed, Apr 13, 2011 at 4:47 PM,  <rh...@apache.org> wrote:
> Author: rhuijben
> Date: Wed Apr 13 21:47:24 2011
> New Revision: 1091928
>
> URL: http://svn.apache.org/viewvc?rev=1091928&view=rev
> Log:
> Fix issue #2381: "Cannot commit multiple WC paths which lack a common parent
> directory" by making the commit processor determine which locks it needs in
> which working copy.

I've not looked at the code, but am just wondering about a theoretical
point here.  Could the approach fall victim to the same cause as issue
#3242?  Specifically, if all the commits are done in the same editor
drive, and that editor drive is rooted somewhere outside of the
readable or writable location for the user doing the commit, would we
get authz denials?

I can just envision the scenario where users try to commit multiple
working copies rooted somewhere they can't write to, and it ends up in
all kinds of confusion.  (Just another reason to introduce the eXecute
bit in our authz paradigm...)

-Hyrum

Re: svn commit: r1091928 - in /subversion/trunk/subversion: libsvn_client/commit.c tests/cmdline/commit_tests.py

Posted by "C. Michael Pilato" <cm...@collab.net>.
On 04/14/2011 11:47 AM, Hyrum K Wright wrote:
> On Wed, Apr 13, 2011 at 4:47 PM,  <rh...@apache.org> wrote:
>> Author: rhuijben
>> Date: Wed Apr 13 21:47:24 2011
>> New Revision: 1091928
>>
>> URL: http://svn.apache.org/viewvc?rev=1091928&view=rev
>> Log:
>> Fix issue #2381: "Cannot commit multiple WC paths which lack a common parent
>> directory" by making the commit processor determine which locks it needs in
>> which working copy.
> 
> I've not looked at the code, but am just wondering about a theoretical
> point here.  Could the approach fall victim to the same cause as issue
> #3242?  Specifically, if all the commits are done in the same editor
> drive, and that editor drive is rooted somewhere outside of the
> readable or writable location for the user doing the commit, would we
> get authz denials?

I would expect that this could happen, yes.  But the same could happen in a
single working copy, too, couldn't it?  I'm thinking of a sparse checkout at
the root with authz like this:

   [repos:/]
   username = r

   [repos:/trunk]
   username = rw

   [repos:/branches]
   username = rw

And then 'username' tries to commit to both the trunk and a branch at the
same time.

> (Just another reason to introduce the eXecute bit in our authz paradigm...)

+1!

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: svn commit: r1091928 - in /subversion/trunk/subversion: libsvn_client/commit.c tests/cmdline/commit_tests.py

Posted by "C. Michael Pilato" <cm...@collab.net>.
On 04/14/2011 11:47 AM, Hyrum K Wright wrote:
> On Wed, Apr 13, 2011 at 4:47 PM,  <rh...@apache.org> wrote:
>> Author: rhuijben
>> Date: Wed Apr 13 21:47:24 2011
>> New Revision: 1091928
>>
>> URL: http://svn.apache.org/viewvc?rev=1091928&view=rev
>> Log:
>> Fix issue #2381: "Cannot commit multiple WC paths which lack a common parent
>> directory" by making the commit processor determine which locks it needs in
>> which working copy.
> 
> I've not looked at the code, but am just wondering about a theoretical
> point here.  Could the approach fall victim to the same cause as issue
> #3242?  Specifically, if all the commits are done in the same editor
> drive, and that editor drive is rooted somewhere outside of the
> readable or writable location for the user doing the commit, would we
> get authz denials?

I would expect that this could happen, yes.  But the same could happen in a
single working copy, too, couldn't it?  I'm thinking of a sparse checkout at
the root with authz like this:

   [repos:/]
   username = r

   [repos:/trunk]
   username = rw

   [repos:/branches]
   username = rw

And then 'username' tries to commit to both the trunk and a branch at the
same time.

> (Just another reason to introduce the eXecute bit in our authz paradigm...)

+1!

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: svn commit: r1091928 - in /subversion/trunk/subversion: libsvn_client/commit.c tests/cmdline/commit_tests.py

Posted by "C. Michael Pilato" <cm...@collab.net>.
On 04/14/2011 02:08 PM, Hyrum K Wright wrote:
> Oh, and I also coded up the original implementation of issue #3242. (Not
> the> fix, the implementation....)

Heheh.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: svn commit: r1091928 - in /subversion/trunk/subversion: libsvn_client/commit.c tests/cmdline/commit_tests.py

Posted by "C. Michael Pilato" <cm...@collab.net>.
On 04/14/2011 02:08 PM, Hyrum K Wright wrote:
> Oh, and I also coded up the original implementation of issue #3242. (Not
> the> fix, the implementation....)

Heheh.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: svn commit: r1091928 - in /subversion/trunk/subversion: libsvn_client/commit.c tests/cmdline/commit_tests.py

Posted by Hyrum K Wright <hy...@hyrumwright.org>.
On Thu, Apr 14, 2011 at 12:49 PM, Bert Huijben <be...@qqmail.nl> wrote:
>
>
>> -----Original Message-----
>> From: Hyrum K Wright [mailto:hyrum@hyrumwright.org]
>> Sent: donderdag 14 april 2011 17:47
>> To: dev@subversion.apache.org
>> Cc: rhuijben@apache.org; commits@subversion.apache.org
>> Subject: Re: svn commit: r1091928 - in /subversion/trunk/subversion:
>> libsvn_client/commit.c tests/cmdline/commit_tests.py
>>
>> On Wed, Apr 13, 2011 at 4:47 PM,  <rh...@apache.org> wrote:
>> > Author: rhuijben
>> > Date: Wed Apr 13 21:47:24 2011
>> > New Revision: 1091928
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1091928&view=rev
>> > Log:
>> > Fix issue #2381: "Cannot commit multiple WC paths which lack a common
>> parent
>> > directory" by making the commit processor determine which locks it needs
>> in
>> > which working copy.
>>
>> I've not looked at the code, but am just wondering about a theoretical
>> point here.  Could the approach fall victim to the same cause as issue
>> #3242?  Specifically, if all the commits are done in the same editor
>> drive, and that editor drive is rooted somewhere outside of the
>> readable or writable location for the user doing the commit, would we
>> get authz denials?
>>
>> I can just envision the scenario where users try to commit multiple
>> working copies rooted somewhere they can't write to, and it ends up in
>> all kinds of confusion.  (Just another reason to introduce the eXecute
>> bit in our authz paradigm...)
>
> By default commit works exactly as before. It's just that you can pass
> explicit targets from multiple working copies.
>
> Yes, that might give you issue #3242, but like Michael said you can have
> that issue in any tree.
>
> The fact that you can now commit in multiple working copies at once, doesn't
> say that it is the right thing to do: it's just an option.
> (And in many cases it is NOT the right thing to do)

Oh certainly.  I'm not claiming we should back out this new feature,
just that we should be aware of potential uncertainties that users may
encounter when trying to use it.

FWIW, I had similar concerns about a patch I proposed for issue #1199,
and it never got applied because of them, so I'm a bit sensitive to
this class of issue.  The patch has long since bitrotted. :(  Oh, and
I also coded up the original implementation of issue #3242.  (Not the
fix, the implementation....)

-Hyrum

Re: svn commit: r1091928 - in /subversion/trunk/subversion: libsvn_client/commit.c tests/cmdline/commit_tests.py

Posted by Hyrum K Wright <hy...@hyrumwright.org>.
On Thu, Apr 14, 2011 at 12:49 PM, Bert Huijben <be...@qqmail.nl> wrote:
>
>
>> -----Original Message-----
>> From: Hyrum K Wright [mailto:hyrum@hyrumwright.org]
>> Sent: donderdag 14 april 2011 17:47
>> To: dev@subversion.apache.org
>> Cc: rhuijben@apache.org; commits@subversion.apache.org
>> Subject: Re: svn commit: r1091928 - in /subversion/trunk/subversion:
>> libsvn_client/commit.c tests/cmdline/commit_tests.py
>>
>> On Wed, Apr 13, 2011 at 4:47 PM,  <rh...@apache.org> wrote:
>> > Author: rhuijben
>> > Date: Wed Apr 13 21:47:24 2011
>> > New Revision: 1091928
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1091928&view=rev
>> > Log:
>> > Fix issue #2381: "Cannot commit multiple WC paths which lack a common
>> parent
>> > directory" by making the commit processor determine which locks it needs
>> in
>> > which working copy.
>>
>> I've not looked at the code, but am just wondering about a theoretical
>> point here.  Could the approach fall victim to the same cause as issue
>> #3242?  Specifically, if all the commits are done in the same editor
>> drive, and that editor drive is rooted somewhere outside of the
>> readable or writable location for the user doing the commit, would we
>> get authz denials?
>>
>> I can just envision the scenario where users try to commit multiple
>> working copies rooted somewhere they can't write to, and it ends up in
>> all kinds of confusion.  (Just another reason to introduce the eXecute
>> bit in our authz paradigm...)
>
> By default commit works exactly as before. It's just that you can pass
> explicit targets from multiple working copies.
>
> Yes, that might give you issue #3242, but like Michael said you can have
> that issue in any tree.
>
> The fact that you can now commit in multiple working copies at once, doesn't
> say that it is the right thing to do: it's just an option.
> (And in many cases it is NOT the right thing to do)

Oh certainly.  I'm not claiming we should back out this new feature,
just that we should be aware of potential uncertainties that users may
encounter when trying to use it.

FWIW, I had similar concerns about a patch I proposed for issue #1199,
and it never got applied because of them, so I'm a bit sensitive to
this class of issue.  The patch has long since bitrotted. :(  Oh, and
I also coded up the original implementation of issue #3242.  (Not the
fix, the implementation....)

-Hyrum

RE: svn commit: r1091928 - in /subversion/trunk/subversion: libsvn_client/commit.c tests/cmdline/commit_tests.py

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Hyrum K Wright [mailto:hyrum@hyrumwright.org]
> Sent: donderdag 14 april 2011 17:47
> To: dev@subversion.apache.org
> Cc: rhuijben@apache.org; commits@subversion.apache.org
> Subject: Re: svn commit: r1091928 - in /subversion/trunk/subversion:
> libsvn_client/commit.c tests/cmdline/commit_tests.py
> 
> On Wed, Apr 13, 2011 at 4:47 PM,  <rh...@apache.org> wrote:
> > Author: rhuijben
> > Date: Wed Apr 13 21:47:24 2011
> > New Revision: 1091928
> >
> > URL: http://svn.apache.org/viewvc?rev=1091928&view=rev
> > Log:
> > Fix issue #2381: "Cannot commit multiple WC paths which lack a common
> parent
> > directory" by making the commit processor determine which locks it needs
> in
> > which working copy.
> 
> I've not looked at the code, but am just wondering about a theoretical
> point here.  Could the approach fall victim to the same cause as issue
> #3242?  Specifically, if all the commits are done in the same editor
> drive, and that editor drive is rooted somewhere outside of the
> readable or writable location for the user doing the commit, would we
> get authz denials?
> 
> I can just envision the scenario where users try to commit multiple
> working copies rooted somewhere they can't write to, and it ends up in
> all kinds of confusion.  (Just another reason to introduce the eXecute
> bit in our authz paradigm...)

By default commit works exactly as before. It's just that you can pass
explicit targets from multiple working copies.

Yes, that might give you issue #3242, but like Michael said you can have
that issue in any tree.

The fact that you can now commit in multiple working copies at once, doesn't
say that it is the right thing to do: it's just an option.
(And in many cases it is NOT the right thing to do)

	Bert


RE: svn commit: r1091928 - in /subversion/trunk/subversion: libsvn_client/commit.c tests/cmdline/commit_tests.py

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Hyrum K Wright [mailto:hyrum@hyrumwright.org]
> Sent: donderdag 14 april 2011 17:47
> To: dev@subversion.apache.org
> Cc: rhuijben@apache.org; commits@subversion.apache.org
> Subject: Re: svn commit: r1091928 - in /subversion/trunk/subversion:
> libsvn_client/commit.c tests/cmdline/commit_tests.py
> 
> On Wed, Apr 13, 2011 at 4:47 PM,  <rh...@apache.org> wrote:
> > Author: rhuijben
> > Date: Wed Apr 13 21:47:24 2011
> > New Revision: 1091928
> >
> > URL: http://svn.apache.org/viewvc?rev=1091928&view=rev
> > Log:
> > Fix issue #2381: "Cannot commit multiple WC paths which lack a common
> parent
> > directory" by making the commit processor determine which locks it needs
> in
> > which working copy.
> 
> I've not looked at the code, but am just wondering about a theoretical
> point here.  Could the approach fall victim to the same cause as issue
> #3242?  Specifically, if all the commits are done in the same editor
> drive, and that editor drive is rooted somewhere outside of the
> readable or writable location for the user doing the commit, would we
> get authz denials?
> 
> I can just envision the scenario where users try to commit multiple
> working copies rooted somewhere they can't write to, and it ends up in
> all kinds of confusion.  (Just another reason to introduce the eXecute
> bit in our authz paradigm...)

By default commit works exactly as before. It's just that you can pass
explicit targets from multiple working copies.

Yes, that might give you issue #3242, but like Michael said you can have
that issue in any tree.

The fact that you can now commit in multiple working copies at once, doesn't
say that it is the right thing to do: it's just an option.
(And in many cases it is NOT the right thing to do)

	Bert