You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <da...@elego.de> on 2013/08/09 16:37:46 UTC

Re: IRC conversation #fail

[ Forwarding to list, with permission ]

C. Michael Pilato wrote on Fri, Aug 09, 2013 at 10:06:25 -0400:
> On 08/09/2013 03:06 AM, Daniel Shahaf wrote:
> >> Ooh... option 3 is kind cool, and your use-case isn't particularly
> >> far-fetched.  Though, I think if I were an administrator, I'd probably
> >> want to smack someone for divorcing the "here's the data" and "here's
> >> how you interpret the data" constructs like this.  Seems like it might
> >> lead to some confusion.
> > 
> > That is true.  I realised that point, and I also came up with
> > a counter-point for it: "do one thing and do it well".  SVNPath is the
> > data and SVNPathOptions is the interpretation; there's no need to start
> > guessing whether the argument is a path string, or a space-separated
> > options string followed by a path string.  This is here and that is
> > there.
> 
> Hrm...  In all realistic use-cases I can conceive, no one would tweak
> the SVNPathOptions for a <Location> without also tweaking SVNPath
> itself.  It's not as if you're going to have a single pathspec that's
> sometimes a template, sometimes a literal path, sometimes a virtual
> path, etc.  Furthermore, of the various modifiers we've discussed, there
> are two distinct classes:
> 

Does anyone use SVNPath outside of a <Location> block?  The directive's
declaration permits that (includes RSRC_CONF), but we don't document it.
(See next hunk...)

> * those which change how the path string is interpreted:  literal (the
> default), template, virtual
> 
> * those which change how the interpreted path string is applied and
> used:  repository path, repos parent path
> 

Agreed with this distinction.  And to answer my own question above, it
would be sensible to use SVNPath at global scope and a modifier of the
"post-interpretation" kind in Location scope; for example, it would be
entirely reasonable to set any of

   SVNPathOptions isparent=off
   SVNPathOptions anonymous_commits=off
   SVNPathOptions readonly=on

in Location scope, without changing (or, indeed, knowing) the
SVNPath/SVNParentPath value.

> Maybe the way to go here is to keep both SVNPath and SVNParentPath, take
> advantage of the need to escape whitespace in the pathspec, and simply
> add the optional modifier strings "template" or "virtual" to the ends of
> both directives:
> 
>    SVNPath pathspec ["template"|"virtual"|...]
>    SVNParentPath pathspec ["template"|"virtual"|...]
> 
> Thoughts?

I'm sold on the distinction between "Knobs that affect the path string
interpretation" knobs and "post-interpretation" knobs.  As to whether we
keep SVNPath and SVNParentPath directives separate, versus have one
directive that contains the pathspec + interpretation options and
another directive that contains the post-interpretation options, I am
not sure.

For the sake of concreteness, I suggest:

1. Add "template|virtual|..." to SVNPath (use "unescaped whitespace"
   as a mode selector, per earlier discussion) and deprecate SVNParentPath.

2. Add SVNPathOptions directive with knobs:
       isparent=[on|off]
       anonymous=[""|"r"|"rw"]
       readonly=[on|off]

WDYT?

Daniel

P.S.  Should we move this to dev@?  I don't mind if this email is quoted
on a public list.

P.P.S. In case I need to motivate the "anonymous=r" SVNPathOption
I suggested: the existing ways to make a repository read-only by
unauthenticated users involve either setting up an authz file or an
unwieldy <LimitExcept> block.  As a configurer I'd be very happy to have
a very clear statement: "This <Location> is read-only to unauthenticated
users."  mod_dav_svn can figure out the LimitExcept blacklist for me and
apply it.

Re: Syntax for templated SVNPath

Posted by Thomas Åkesson <th...@bafast.se>.
On 14 aug 2013, at 03:25, Ben Reser <be...@reser.org> wrote:

> On 8/13/13 4:41 PM, Thomas Åkesson wrote:
>> To make this enhancement complete, I believe all settings that take a "directory-path" should be handled identically (allow the templating). To me, the most obvious example is "AuthzSVNAccessFile". It wouldn't make sense to separate repositories for different vhosts but keep a single huge access file.
> 
> This probably isn't what you're thinking of but
> AuthzSVNReposRelativeAccessFile has been around since 1.7:
> http://subversion.apache.org/docs/release-notes/1.7.html#per-repos-authz

I know, and yes, it can be used with this enhancement in form 2 and 4, but not in form 1 and 3.

> 
>> Agree, duplicating the directives is not the way to go. But deprecating SVNParentPath seems a bit over the top. SVNPath vs SVNParentPath are 2 quite distinct modes of operation and many many configs use them so I get the feeling SVNParentPath can't be removed for a long time anyway.
>> 
>> I propose either of:
>> 
>> 1. SVNPathTemplates on | off
>> 2. SVNPathType default | template | ...
>> 
>> Where the option applies to all SVN* options that take a pathspec, including the authz options.
> 
> My vote is to technically eliminate SVNParentPath (though it would still
> be accepted, but we'd just treat it the same as SVNPath).  svnserve
> supports pointing things directly at the repository and at a parent
> paths without the user needing to specify this, I can't see any reason
> httpd needs this extra verbosity.

Ok

> Further, I don't see a reason to toggle templates on or off.  When we
> added in repos authz I didn't worry about someone having a relative path
> with starting with file: or ^, both of which would have been broken.
> 
> By the same account I don't think path names like %{SERVER_NAME} or %0
> are worth worrying about.

Yes, I agree with that. There really is no reason to have '%' characters in the path on servers. Minimal option proliferation... (None)

> Adding the verbosity only helps someone doing something that probably
> doesn't make any sense and hurts the vast majority.
> 
>> I would like to throw in a related issue that I was thinking about while studying the release notes of svn 1.8. We (Simonsoft) are _very_ exited about the "In repository authz". In our setups, the access file (currently one per server) is managed in a separate repository, but we have to manually ensure the latest one is on disk (svn up). All our servers have an "admin" repo containing server config including the access file. 
>> 
>> We would like to transition to:
>> - Separate access file for each repo.
>> - Separate groups file (AuthzSVNGroupsFile)
>> - Keep access files in admin repo.
>> - Reference the access/group files using file://...
>> 
>> However, there is a problem with referencing separate access files that way when using SVNParentPath. We would need the same templating concept:
>> file:///path/to/admin-repo/%{REPO_NAME}/file
>> or
>> file:///path/to/%{SERVER_NAME}/admin-repo/%{REPO_NAME}/file
>> 
>> This is somewhat out of scope for this discussion, but still a validation that the templating syntax can also address this limitation.
> 
> I remember thinking about doing this and decided not to.  I don't really
> see a huge problem with doing this.

Good

> While I like the human readable variable names like %{SERVER_NAME}, I do
> wonder if we shouldn't make our virtual hosting patterns match that of
> the mod_vhost_alias does, I can see at least some of the hostname
> splitting being useful.  I can also see someone using dynamic svn
> virutal hosting also using mod_vhost_alias for websites.  So some
> existing domain knowledge could be reused by users.

The mod-vhost syntax is not quite as future proof though. I personally prefer the mod-ssl syntax that Mike went with. 

> If we use human readable variable names REPO_NAME should be
> SVN-REPOS-NAME to match what we're using to allow logging.  e.g.
> http://svnbook.red-bean.com/en/1.7/svn.serverconfig.httpd.html#svn.serverconfig.httpd.extra.logging

Makes sense. 

Regards,
Thomas Å. 

Re: Syntax for templated SVNPath

Posted by Ben Reser <be...@reser.org>.
On 8/13/13 4:41 PM, Thomas Åkesson wrote:
> To make this enhancement complete, I believe all settings that take a "directory-path" should be handled identically (allow the templating). To me, the most obvious example is "AuthzSVNAccessFile". It wouldn't make sense to separate repositories for different vhosts but keep a single huge access file.

This probably isn't what you're thinking of but
AuthzSVNReposRelativeAccessFile has been around since 1.7:
http://subversion.apache.org/docs/release-notes/1.7.html#per-repos-authz

> Agree, duplicating the directives is not the way to go. But deprecating SVNParentPath seems a bit over the top. SVNPath vs SVNParentPath are 2 quite distinct modes of operation and many many configs use them so I get the feeling SVNParentPath can't be removed for a long time anyway.
> 
> I propose either of:
> 
> 1. SVNPathTemplates on | off
> 2. SVNPathType default | template | ...
> 
> Where the option applies to all SVN* options that take a pathspec, including the authz options.

My vote is to technically eliminate SVNParentPath (though it would still
be accepted, but we'd just treat it the same as SVNPath).  svnserve
supports pointing things directly at the repository and at a parent
paths without the user needing to specify this, I can't see any reason
httpd needs this extra verbosity.

Further, I don't see a reason to toggle templates on or off.  When we
added in repos authz I didn't worry about someone having a relative path
with starting with file: or ^, both of which would have been broken.

By the same account I don't think path names like %{SERVER_NAME} or %0
are worth worrying about.

Adding the verbosity only helps someone doing something that probably
doesn't make any sense and hurts the vast majority.

> I would like to throw in a related issue that I was thinking about while studying the release notes of svn 1.8. We (Simonsoft) are _very_ exited about the "In repository authz". In our setups, the access file (currently one per server) is managed in a separate repository, but we have to manually ensure the latest one is on disk (svn up). All our servers have an "admin" repo containing server config including the access file. 
> 
> We would like to transition to:
>  - Separate access file for each repo.
>  - Separate groups file (AuthzSVNGroupsFile)
>  - Keep access files in admin repo.
>  - Reference the access/group files using file://...
> 
> However, there is a problem with referencing separate access files that way when using SVNParentPath. We would need the same templating concept:
> file:///path/to/admin-repo/%{REPO_NAME}/file
> or
> file:///path/to/%{SERVER_NAME}/admin-repo/%{REPO_NAME}/file
> 
> This is somewhat out of scope for this discussion, but still a validation that the templating syntax can also address this limitation.

I remember thinking about doing this and decided not to.  I don't really
see a huge problem with doing this.

While I like the human readable variable names like %{SERVER_NAME}, I do
wonder if we shouldn't make our virtual hosting patterns match that of
the mod_vhost_alias does, I can see at least some of the hostname
splitting being useful.  I can also see someone using dynamic svn
virutal hosting also using mod_vhost_alias for websites.  So some
existing domain knowledge could be reused by users.

If we use human readable variable names REPO_NAME should be
SVN-REPOS-NAME to match what we're using to allow logging.  e.g.
http://svnbook.red-bean.com/en/1.7/svn.serverconfig.httpd.html#svn.serverconfig.httpd.extra.logging

Re: Syntax for templated SVNPath

Posted by Thomas Åkesson <th...@akesson.cc>.
On 9 aug 2013, at 17:52, C. Michael Pilato <cm...@collab.net> wrote:

> Put simply, CollabNet wants to start using dynamic virtual hosting with
> Apache HTTP Server in one of our offerings in order to avoid adding a
> literal <VirtualHost> block for every single virtual host on the system.

First of all, I like this enhancement, a lot.

> There are obviously many configuration settings which are common across
> all the virtual hosts, but there are some which need to vary for each
> host.  One of the latter is the path of the Subversion repository parent
> path, because each vhost keeps its own collection of repositories.

To make this enhancement complete, I believe all settings that take a "directory-path" should be handled identically (allow the templating). To me, the most obvious example is "AuthzSVNAccessFile". It wouldn't make sense to separate repositories for different vhosts but keep a single huge access file.

> 
> The solution I developed for this on the SVNParentPathTemplate branch is
> to introduce a new mod_dav_svn directive which is like SVNParentPath,
> but where you can insert the string "%{SERVER_NAME}" into the path
> specification and have mod_dav_svn replace that at runtime with the
> server name of the virtual host by which the request came in.  For example:
> 
>   SVNParentPathTemplate /usr/local/svn/%{SERVER_NAME}/repositories
> 
> So Daniel, Ben and I were talking in #httpd-dev about the proper syntax
> for my new directive, because I wanted to use something that matches
> conventions used by other httpd modules rather than trying to break new
> syntax ground here.  (I preferred to use a named variable instead of
> "%s" or "1" or somesuch with future expansion in mind.)

I like the choice of syntax, makes sense in httpd config and it's expandable, see related issue below.

> But along the way, our conversation led us to the problem of option
> proliferation, precisely because we might someday also add
> SVNPathTemplate (the not-parent form of my new thing), or an
> SVNVirtualPath/SVNVirtualParentPath pair (which behaves more like
> mod_vhost_alias's directives), etc.  And we sighed and proactively
> dreamt of days when configuring Subversion was simpler.

Agree, duplicating the directives is not the way to go. But deprecating SVNParentPath seems a bit over the top. SVNPath vs SVNParentPath are 2 quite distinct modes of operation and many many configs use them so I get the feeling SVNParentPath can't be removed for a long time anyway.

I propose either of:

1. SVNPathTemplates on | off
2. SVNPathType default | template | ...

Where the option applies to all SVN* options that take a pathspec, including the authz options.


> The discussion you are now privy to is all about how we can make our
> option set both useful and expandible to account for these immediate and
> future-maybe scenarios.

I would like to throw in a related issue that I was thinking about while studying the release notes of svn 1.8. We (Simonsoft) are _very_ exited about the "In repository authz". In our setups, the access file (currently one per server) is managed in a separate repository, but we have to manually ensure the latest one is on disk (svn up). All our servers have an "admin" repo containing server config including the access file. 

We would like to transition to:
 - Separate access file for each repo.
 - Separate groups file (AuthzSVNGroupsFile)
 - Keep access files in admin repo.
 - Reference the access/group files using file://...

However, there is a problem with referencing separate access files that way when using SVNParentPath. We would need the same templating concept:
file:///path/to/admin-repo/%{REPO_NAME}/file
or
file:///path/to/%{SERVER_NAME}/admin-repo/%{REPO_NAME}/file

This is somewhat out of scope for this discussion, but still a validation that the templating syntax can also address this limitation.


Thanks,
Thomas Å.


Re: Syntax for templated SVNPath

Posted by "C. Michael Pilato" <cm...@collab.net>.
On 08/09/2013 10:40 AM, Daniel Shahaf wrote:
> [ Just changing the subject.  The background is the branch cmpilato
> started a couple of days ago; I'll let him introduce that. ]

Can do.

Put simply, CollabNet wants to start using dynamic virtual hosting with
Apache HTTP Server in one of our offerings in order to avoid adding a
literal <VirtualHost> block for every single virtual host on the system.
 There are obviously many configuration settings which are common across
all the virtual hosts, but there are some which need to vary for each
host.  One of the latter is the path of the Subversion repository parent
path, because each vhost keeps its own collection of repositories.

The solution I developed for this on the SVNParentPathTemplate branch is
to introduce a new mod_dav_svn directive which is like SVNParentPath,
but where you can insert the string "%{SERVER_NAME}" into the path
specification and have mod_dav_svn replace that at runtime with the
server name of the virtual host by which the request came in.  For example:

   SVNParentPathTemplate /usr/local/svn/%{SERVER_NAME}/repositories

So Daniel, Ben and I were talking in #httpd-dev about the proper syntax
for my new directive, because I wanted to use something that matches
conventions used by other httpd modules rather than trying to break new
syntax ground here.  (I preferred to use a named variable instead of
"%s" or "1" or somesuch with future expansion in mind.)

But along the way, our conversation led us to the problem of option
proliferation, precisely because we might someday also add
SVNPathTemplate (the not-parent form of my new thing), or an
SVNVirtualPath/SVNVirtualParentPath pair (which behaves more like
mod_vhost_alias's directives), etc.  And we sighed and proactively
dreamt of days when configuring Subversion was simpler.

The discussion you are now privy to is all about how we can make our
option set both useful and expandible to account for these immediate and
future-maybe scenarios.


> Daniel Shahaf wrote on Fri, Aug 09, 2013 at 17:37:46 +0300:
>> [ Forwarding to list, with permission ]
>>
>> C. Michael Pilato wrote on Fri, Aug 09, 2013 at 10:06:25 -0400:
>>> On 08/09/2013 03:06 AM, Daniel Shahaf wrote:
>>>>> Ooh... option 3 is kind cool, and your use-case isn't particularly
>>>>> far-fetched.  Though, I think if I were an administrator, I'd probably
>>>>> want to smack someone for divorcing the "here's the data" and "here's
>>>>> how you interpret the data" constructs like this.  Seems like it might
>>>>> lead to some confusion.
>>>>
>>>> That is true.  I realised that point, and I also came up with
>>>> a counter-point for it: "do one thing and do it well".  SVNPath is the
>>>> data and SVNPathOptions is the interpretation; there's no need to start
>>>> guessing whether the argument is a path string, or a space-separated
>>>> options string followed by a path string.  This is here and that is
>>>> there.
>>>
>>> Hrm...  In all realistic use-cases I can conceive, no one would tweak
>>> the SVNPathOptions for a <Location> without also tweaking SVNPath
>>> itself.  It's not as if you're going to have a single pathspec that's
>>> sometimes a template, sometimes a literal path, sometimes a virtual
>>> path, etc.  Furthermore, of the various modifiers we've discussed, there
>>> are two distinct classes:
>>>
>>
>> Does anyone use SVNPath outside of a <Location> block?  The directive's
>> declaration permits that (includes RSRC_CONF), but we don't document it.
>> (See next hunk...)
>>
>>> * those which change how the path string is interpreted:  literal (the
>>> default), template, virtual
>>>
>>> * those which change how the interpreted path string is applied and
>>> used:  repository path, repos parent path
>>>
>>
>> Agreed with this distinction.  And to answer my own question above, it
>> would be sensible to use SVNPath at global scope and a modifier of the
>> "post-interpretation" kind in Location scope; for example, it would be
>> entirely reasonable to set any of
>>
>>    SVNPathOptions isparent=off
>>    SVNPathOptions anonymous_commits=off
>>    SVNPathOptions readonly=on
>>
>> in Location scope, without changing (or, indeed, knowing) the
>> SVNPath/SVNParentPath value.
>>
>>> Maybe the way to go here is to keep both SVNPath and SVNParentPath, take
>>> advantage of the need to escape whitespace in the pathspec, and simply
>>> add the optional modifier strings "template" or "virtual" to the ends of
>>> both directives:
>>>
>>>    SVNPath pathspec ["template"|"virtual"|...]
>>>    SVNParentPath pathspec ["template"|"virtual"|...]
>>>
>>> Thoughts?
>>
>> I'm sold on the distinction between "Knobs that affect the path string
>> interpretation" knobs and "post-interpretation" knobs.  As to whether we
>> keep SVNPath and SVNParentPath directives separate, versus have one
>> directive that contains the pathspec + interpretation options and
>> another directive that contains the post-interpretation options, I am
>> not sure.
>>
>> For the sake of concreteness, I suggest:
>>
>> 1. Add "template|virtual|..." to SVNPath (use "unescaped whitespace"
>>    as a mode selector, per earlier discussion) and deprecate SVNParentPath.
>>
>> 2. Add SVNPathOptions directive with knobs:
>>        isparent=[on|off]
>>        anonymous=[""|"r"|"rw"]
>>        readonly=[on|off]
>>
>> WDYT?
>>
>> Daniel
>>
>> P.S.  Should we move this to dev@?  I don't mind if this email is quoted
>> on a public list.
>>
>> P.P.S. In case I need to motivate the "anonymous=r" SVNPathOption
>> I suggested: the existing ways to make a repository read-only by
>> unauthenticated users involve either setting up an authz file or an
>> unwieldy <LimitExcept> block.  As a configurer I'd be very happy to have
>> a very clear statement: "This <Location> is read-only to unauthenticated
>> users."  mod_dav_svn can figure out the LimitExcept blacklist for me and
>> apply it.


Syntax for templated SVNPath

Posted by Daniel Shahaf <da...@elego.de>.
[ Just changing the subject.  The background is the branch cmpilato
started a couple of days ago; I'll let him introduce that. ]

Daniel Shahaf wrote on Fri, Aug 09, 2013 at 17:37:46 +0300:
> [ Forwarding to list, with permission ]
> 
> C. Michael Pilato wrote on Fri, Aug 09, 2013 at 10:06:25 -0400:
> > On 08/09/2013 03:06 AM, Daniel Shahaf wrote:
> > >> Ooh... option 3 is kind cool, and your use-case isn't particularly
> > >> far-fetched.  Though, I think if I were an administrator, I'd probably
> > >> want to smack someone for divorcing the "here's the data" and "here's
> > >> how you interpret the data" constructs like this.  Seems like it might
> > >> lead to some confusion.
> > > 
> > > That is true.  I realised that point, and I also came up with
> > > a counter-point for it: "do one thing and do it well".  SVNPath is the
> > > data and SVNPathOptions is the interpretation; there's no need to start
> > > guessing whether the argument is a path string, or a space-separated
> > > options string followed by a path string.  This is here and that is
> > > there.
> > 
> > Hrm...  In all realistic use-cases I can conceive, no one would tweak
> > the SVNPathOptions for a <Location> without also tweaking SVNPath
> > itself.  It's not as if you're going to have a single pathspec that's
> > sometimes a template, sometimes a literal path, sometimes a virtual
> > path, etc.  Furthermore, of the various modifiers we've discussed, there
> > are two distinct classes:
> > 
> 
> Does anyone use SVNPath outside of a <Location> block?  The directive's
> declaration permits that (includes RSRC_CONF), but we don't document it.
> (See next hunk...)
> 
> > * those which change how the path string is interpreted:  literal (the
> > default), template, virtual
> > 
> > * those which change how the interpreted path string is applied and
> > used:  repository path, repos parent path
> > 
> 
> Agreed with this distinction.  And to answer my own question above, it
> would be sensible to use SVNPath at global scope and a modifier of the
> "post-interpretation" kind in Location scope; for example, it would be
> entirely reasonable to set any of
> 
>    SVNPathOptions isparent=off
>    SVNPathOptions anonymous_commits=off
>    SVNPathOptions readonly=on
> 
> in Location scope, without changing (or, indeed, knowing) the
> SVNPath/SVNParentPath value.
> 
> > Maybe the way to go here is to keep both SVNPath and SVNParentPath, take
> > advantage of the need to escape whitespace in the pathspec, and simply
> > add the optional modifier strings "template" or "virtual" to the ends of
> > both directives:
> > 
> >    SVNPath pathspec ["template"|"virtual"|...]
> >    SVNParentPath pathspec ["template"|"virtual"|...]
> > 
> > Thoughts?
> 
> I'm sold on the distinction between "Knobs that affect the path string
> interpretation" knobs and "post-interpretation" knobs.  As to whether we
> keep SVNPath and SVNParentPath directives separate, versus have one
> directive that contains the pathspec + interpretation options and
> another directive that contains the post-interpretation options, I am
> not sure.
> 
> For the sake of concreteness, I suggest:
> 
> 1. Add "template|virtual|..." to SVNPath (use "unescaped whitespace"
>    as a mode selector, per earlier discussion) and deprecate SVNParentPath.
> 
> 2. Add SVNPathOptions directive with knobs:
>        isparent=[on|off]
>        anonymous=[""|"r"|"rw"]
>        readonly=[on|off]
> 
> WDYT?
> 
> Daniel
> 
> P.S.  Should we move this to dev@?  I don't mind if this email is quoted
> on a public list.
> 
> P.P.S. In case I need to motivate the "anonymous=r" SVNPathOption
> I suggested: the existing ways to make a repository read-only by
> unauthenticated users involve either setting up an authz file or an
> unwieldy <LimitExcept> block.  As a configurer I'd be very happy to have
> a very clear statement: "This <Location> is read-only to unauthenticated
> users."  mod_dav_svn can figure out the LimitExcept blacklist for me and
> apply it.