You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Vincent Legoll <vi...@gmail.com> on 2009/12/04 14:35:02 UTC
[PATCH][RFC] allow local file:// URLs for svnadmin
Hello,
would such kind of patch be acceptable or is this useless ?
I think there's nothing preventing svnadmin to accept local
file:// URLs as repository location parameter. So why not ?
The attached patch, is probably wrong (you tell me) as my
C is a bit rusty, but should show the intended change...
Comments, reviews, flames, etc... All gladly accepted.
If this is deemed useful, I'll especially would like directions
on how to improve the patch to make it suitable for a real
submission...
--
Vincent Legoll
###################### Inline ########################
--- a/main.c
+++ b/main.c
@@ -133,6 +133,15 @@ parse_local_repos_path(apr_getopt_t *os,
}
else if (svn_path_is_url(*repos_path))
{
+ #define URL_LOCAL_FILE_PREFIX "file://"
+ if (strncmp(*repos_path, URL_LOCAL_FILE_PREFIX,
+ strlen(URL_LOCAL_FILE_PREFIX)))
+ {
+ *repos_path = svn_dirent_internal_style(*repos_path +
+ strlen(URL_LOCAL_FILE_PREFIX), pool);
+ return SVN_NO_ERROR;
+ }
+
return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
_("'%s' is an URL when it should be a path"),
*repos_path);
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2427043
Please start new threads on the <de...@subversion.apache.org> mailing list.
To subscribe to the new list, send an empty e-mail to <de...@subversion.apache.org>.
Re: [PATCH][RFC] allow local file:// URLs for svnadmin
Posted by Julian Foad <ju...@btopenworld.com>.
David Glasser wrote:
> Another difference between RA URLs and repos filesystem paths is that
> RA URLs can contain directories inside the repository.
>
> So you'd have to figure out the semantics there. If somebody
> specifies file:///path/to/repos/then/subdir, is it equivalent to
> path/to/repos? Or an error? Either way is likely to be confusing to
> somebody.
That's a good additional point. Thanks.
- Julian
> (I'm generally in the "not a good idea" camp here.)
>
> --dave
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2427302
Please start new threads on the <de...@subversion.apache.org> mailing list.
To subscribe to the new list, send an empty e-mail to <de...@subversion.apache.org>.
Re: [PATCH][RFC] allow local file:// URLs for svnadmin
Posted by David Glasser <gl...@davidglasser.net>.
Another difference between RA URLs and repos filesystem paths is that
RA URLs can contain directories inside the repository.
So you'd have to figure out the semantics there. If somebody
specifies file:///path/to/repos/then/subdir, is it equivalent to
path/to/repos? Or an error? Either way is likely to be confusing to
somebody.
(I'm generally in the "not a good idea" camp here.)
--dave
On Fri, Dec 4, 2009 at 6:35 AM, Vincent Legoll <vi...@gmail.com> wrote:
> Hello,
>
> would such kind of patch be acceptable or is this useless ?
>
> I think there's nothing preventing svnadmin to accept local
> file:// URLs as repository location parameter. So why not ?
>
> The attached patch, is probably wrong (you tell me) as my
> C is a bit rusty, but should show the intended change...
>
> Comments, reviews, flames, etc... All gladly accepted.
>
> If this is deemed useful, I'll especially would like directions
> on how to improve the patch to make it suitable for a real
> submission...
>
> --
> Vincent Legoll
>
> ###################### Inline ########################
>
> --- a/main.c
> +++ b/main.c
> @@ -133,6 +133,15 @@ parse_local_repos_path(apr_getopt_t *os,
> }
> else if (svn_path_is_url(*repos_path))
> {
> + #define URL_LOCAL_FILE_PREFIX "file://"
> + if (strncmp(*repos_path, URL_LOCAL_FILE_PREFIX,
> + strlen(URL_LOCAL_FILE_PREFIX)))
> + {
> + *repos_path = svn_dirent_internal_style(*repos_path +
> + strlen(URL_LOCAL_FILE_PREFIX), pool);
> + return SVN_NO_ERROR;
> + }
> +
> return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> _("'%s' is an URL when it should be a path"),
> *repos_path);
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2427043
>
> Please start new threads on the <de...@subversion.apache.org> mailing list.
> To subscribe to the new list, send an empty e-mail to <de...@subversion.apache.org>.
--
glasser@davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2427298
Please start new threads on the <de...@subversion.apache.org> mailing list.
To subscribe to the new list, send an empty e-mail to <de...@subversion.apache.org>.
Re: [PATCH][RFC] allow local file:// URLs for svnadmin
Posted by Julian Foad <ju...@wandisco.com>.
On Fri, 2009-12-04 at 16:05 +0100, Vincent Legoll wrote:
> On Fri, Dec 4, 2009 at 3:50 PM, Julian Foad <ju...@btopenworld.com> wrote:
> > Vincent Legoll wrote:
> >> I think there's nothing preventing svnadmin to accept local
> >> file:// URLs as repository location parameter. So why not ?
> >
> > Let me ask a different question: Why? (We don't add features just
> > because we can, we add them because they are useful enough to
> > be worth the maintenance.)
>
> I understand the maintenance burden, and that was the point I wanted
> to raise with that question about usefulness.
>
> > I can think of a reason: it could be handy to copying URLs between
> > "svn" and "svnlook" and "svnadmin" commands.
>
> That's what make me try to do the patch, I didn't cut'n'paste from
> another svn* command but from a tailor (SCM migration tool) config
> file, but that's the idea behind the patch yes...
>
> > Is that your reason? Is there any stronger reason?
>
> Yes, user-least-surprise or you-try-something-and-it-just-work,
> when I saw the error message about URL not being accepted,
> I told myself: "WTF, this is a damn local file:// URL"
OK. It sounds like a good idea to me.
> > Would you add it to svnlook as well?
>
> If the idea is acceptable and the work I'll put into is not lost, yes
> I'll do it.
Great. I would keep an eye out for the next day or two in case anyone
disagrees.
> > If you want to do this, remember that converting a URL to a local path
> > is not as simple as stripping off a prefix: there are at least two
> > different prefixes (file://localhost as well), and you have to un-escape
> > the URL escaping.
>
> Yes, I think the final patch will be somewhat bigger, I didn't actually
> look, but will, maybe URL handling code that could be borrowed is already
> there...
See svn_path_uri_decode().
> Thanks for the quick answer
A pleasure.
- Julian
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2427074
Please start new threads on the <de...@subversion.apache.org> mailing list.
To subscribe to the new list, send an empty e-mail to <de...@subversion.apache.org>.
Re: [PATCH][RFC] allow local file:// URLs for svnadmin
Posted by Vincent Legoll <vi...@gmail.com>.
On Fri, Dec 4, 2009 at 3:50 PM, Julian Foad <ju...@btopenworld.com> wrote:
> Vincent Legoll wrote:
>> I think there's nothing preventing svnadmin to accept local
>> file:// URLs as repository location parameter. So why not ?
>
> Let me ask a different question: Why? (We don't add features just
> because we can, we add them because they are useful enough to
> be worth the maintenance.)
I understand the maintenance burden, and that was the point I wanted
to raise with that question about usefulness.
> I can think of a reason: it could be handy to copying URLs between
> "svn" and "svnlook" and "svnadmin" commands.
That's what make me try to do the patch, I didn't cut'n'paste from
another svn* command but from a tailor (SCM migration tool) config
file, but that's the idea behind the patch yes...
> Is that your reason? Is there any stronger reason?
Yes, user-least-surprise or you-try-something-and-it-just-work,
when I saw the error message about URL not being accepted,
I told myself: "WTF, this is a damn local file:// URL"
> Would you add it to svnlook as well?
If the idea is acceptable and the work I'll put into is not lost, yes
I'll do it.
> If you want to do this, remember that converting a URL to a local path
> is not as simple as stripping off a prefix: there are at least two
> different prefixes (file://localhost as well), and you have to un-escape
> the URL escaping.
Yes, I think the final patch will be somewhat bigger, I didn't actually
look, but will, maybe URL handling code that could be borrowed is already
there...
Thanks for the quick answer
--
Vincent Legoll
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2427067
Please start new threads on the <de...@subversion.apache.org> mailing list.
To subscribe to the new list, send an empty e-mail to <de...@subversion.apache.org>.
Re: [PATCH][RFC] allow local file:// URLs for svnadmin
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Fri, 4 Dec 2009 at 18:16 -0000:
> Daniel Shahaf wrote:
> > Philip Martin wrote on Fri, 4 Dec 2009 at 15:51 -0000:
> > > Julian Foad <ju...@btopenworld.com> writes:
> > >
> > > > Vincent Legoll wrote:
> > > >> I think there's nothing preventing svnadmin to accept local
> > > >> file:// URLs as repository location parameter. So why not ?
> > >
> > > I think it's a bad idea because svnadmin doesn't use the RA layer.
> >
> > That's my initial reaction as well. Nowhere do we accept file:/// URLs
> > instead of paths: not in the API, not in svn (you can't say
> > "svn info file://path/to/wc"), etc.
>
> Spurious analogy. We're not talking about accepting URLs to WCs, only to
> repositories.
I was in an "accept a file:// URL as an alias for the path it points to" mood.
> The converse comparison is: nowhere do we not accept a URL
> to refer to a repository, except currently in svnadmin/svnlook. (Sure,
> there's the "not a general RA access" argument, etc. to consider, and
> I'm not fighting for accepting it, just responding to your statement.)
>
In general, 'svn' mirrors the client API and 'svnadmin' mirrors the
repos API. If we want svnadmin to accept file:// URLs, shouldn't
svn_repos_open() accept them too?
(I'm not saying it shouldn't; I'm saying that, if svnadmin is extended,
we may extend the repos API in the same manner.)
> - Julian
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2427139
>
> Please start new threads on the <de...@subversion.apache.org> mailing list.
> To subscribe to the new list, send an empty e-mail to <de...@subversion.apache.org>.
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2427144
Please start new threads on the <de...@subversion.apache.org> mailing list.
To subscribe to the new list, send an empty e-mail to <de...@subversion.apache.org>.
Re: [PATCH][RFC] allow local file:// URLs for svnadmin
Posted by Julian Foad <ju...@btopenworld.com>.
Daniel Shahaf wrote:
> Philip Martin wrote on Fri, 4 Dec 2009 at 15:51 -0000:
> > Julian Foad <ju...@btopenworld.com> writes:
> >
> > > Vincent Legoll wrote:
> > >> I think there's nothing preventing svnadmin to accept local
> > >> file:// URLs as repository location parameter. So why not ?
> >
> > I think it's a bad idea because svnadmin doesn't use the RA layer.
>
> That's my initial reaction as well. Nowhere do we accept file:/// URLs
> instead of paths: not in the API, not in svn (you can't say
> "svn info file://path/to/wc"), etc.
Spurious analogy. We're not talking about accepting URLs to WCs, only to
repositories. The converse comparison is: nowhere do we not accept a URL
to refer to a repository, except currently in svnadmin/svnlook. (Sure,
there's the "not a general RA access" argument, etc. to consider, and
I'm not fighting for accepting it, just responding to your statement.)
- Julian
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2427139
Please start new threads on the <de...@subversion.apache.org> mailing list.
To subscribe to the new list, send an empty e-mail to <de...@subversion.apache.org>.
Re: [PATCH][RFC] allow local file:// URLs for svnadmin
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Philip Martin wrote on Fri, 4 Dec 2009 at 15:51 -0000:
> Julian Foad <ju...@btopenworld.com> writes:
>
> > Vincent Legoll wrote:
> >> I think there's nothing preventing svnadmin to accept local
> >> file:// URLs as repository location parameter. So why not ?
>
> I think it's a bad idea because svnadmin doesn't use the RA layer.
That's my initial reaction as well. Nowhere do we accept file:/// URLs
instead of paths: not in the API, not in svn (you can't say
"svn info file://path/to/wc"), etc.
Daniel
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2427134
Please start new threads on the <de...@subversion.apache.org> mailing list.
To subscribe to the new list, send an empty e-mail to <de...@subversion.apache.org>.
Re: [PATCH][RFC] allow local file:// URLs for svnadmin
Posted by Philip Martin <ph...@wandisco.com>.
Julian Foad <ju...@btopenworld.com> writes:
> Vincent Legoll wrote:
>> I think there's nothing preventing svnadmin to accept local
>> file:// URLs as repository location parameter. So why not ?
I think it's a bad idea because svnadmin doesn't use the RA layer.
Adding this will create the false expectation that all RA protocols
should work.
> If you want to do this, remember that converting a URL to a local path
> is not as simple as stripping off a prefix: there are at least two
> different prefixes (file://localhost as well), and you have to un-escape
> the URL escaping.
The client allows one to use file://localhost:port as well (although
that might be a bug).
--
Philip
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2427085
Please start new threads on the <de...@subversion.apache.org> mailing list.
To subscribe to the new list, send an empty e-mail to <de...@subversion.apache.org>.
Re: [PATCH][RFC] allow local file:// URLs for svnadmin
Posted by Julian Foad <ju...@wandisco.com>.
C. Michael Pilato wrote:
> Hyrum K. Wright wrote:
> > My initial reaction is about consistency. I can't think of any place we
> > accept URLs as parameters to svnadmin, and I'm really hesitant to start
> > doing so. In the command-line client, when we say "URL" we mean *any*
> > well-formed URL, not just those with a specific access method. Changing
> > that meaning on any of the command-line tools is inconsistent, and should
> > be avoided.
>
> I'm a big fan of consistency. It's good for users. It's good for
> developers. It's great for parents. And drummers.
Me, too, as you well know, but it's not simply "consistency", it's
"consistency of <what?>". Your consistency is different from mine.
> I too, oppose this
> change, for the reasons Hyrum has so pleasantly stated.
I see your side too... and though there are merits in both sides, I
guess I'm coming around to the "leave it alone" point of view now.
Sorry, Vincent.
- Julian
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2427097
Please start new threads on the <de...@subversion.apache.org> mailing list.
To subscribe to the new list, send an empty e-mail to <de...@subversion.apache.org>.
Re: [PATCH][RFC] allow local file:// URLs for svnadmin
Posted by "C. Michael Pilato" <cm...@collab.net>.
Hyrum K. Wright wrote:
> On Dec 4, 2009, at 6:50 AM, Julian Foad wrote:
>
>> Vincent Legoll wrote:
>>> Hello,
>>>
>>> would such kind of patch be acceptable or is this useless ?
>>>
>>> I think there's nothing preventing svnadmin to accept local file://
>>> URLs as repository location parameter. So why not ?
>> Let me ask a different question: Why? (We don't add features just
>> because we can, we add them because they are useful enough to be worth
>> the maintenance.)
>>
>> I can think of a reason: it could be handy to copying URLs between
>> "svn" and "svnlook" and "svnadmin" commands. Is that your reason? Is
>> there any stronger reason? Would you add it to svnlook as well?
>>
>> If you want to do this, remember that converting a URL to a local path
>> is not as simple as stripping off a prefix: there are at least two
>> different prefixes (file://localhost as well), and you have to
>> un-escape the URL escaping.
>
> My initial reaction is about consistency. I can't think of any place we
> accept URLs as parameters to svnadmin, and I'm really hesitant to start
> doing so. In the command-line client, when we say "URL" we mean *any*
> well-formed URL, not just those with a specific access method. Changing
> that meaning on any of the command-line tools is inconsistent, and should
> be avoided.
I'm a big fan of consistency. It's good for users. It's good for
developers. It's great for parents. And drummers. I too, oppose this
change, for the reasons Hyrum has so pleasantly stated.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2427081
Please start new threads on the <de...@subversion.apache.org> mailing list.
To subscribe to the new list, send an empty e-mail to <de...@subversion.apache.org>.
Re: [PATCH][RFC] allow local file:// URLs for svnadmin
Posted by "Hyrum K. Wright" <hy...@hyrumwright.org>.
On Dec 4, 2009, at 6:50 AM, Julian Foad wrote:
> Vincent Legoll wrote:
>> Hello,
>>
>> would such kind of patch be acceptable or is this useless ?
>>
>> I think there's nothing preventing svnadmin to accept local
>> file:// URLs as repository location parameter. So why not ?
>
> Let me ask a different question: Why? (We don't add features just
> because we can, we add them because they are useful enough to be worth
> the maintenance.)
>
> I can think of a reason: it could be handy to copying URLs between "svn"
> and "svnlook" and "svnadmin" commands. Is that your reason? Is there any
> stronger reason? Would you add it to svnlook as well?
>
> If you want to do this, remember that converting a URL to a local path
> is not as simple as stripping off a prefix: there are at least two
> different prefixes (file://localhost as well), and you have to un-escape
> the URL escaping.
My initial reaction is about consistency. I can't think of any place we accept URLs as parameters to svnadmin, and I'm really hesitant to start doing so. In the command-line client, when we say "URL" we mean *any* well-formed URL, not just those with a specific access method. Changing that meaning on any of the command-line tools is inconsistent, and should be avoided.
-Hyrum
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2427073
Please start new threads on the <de...@subversion.apache.org> mailing list.
To subscribe to the new list, send an empty e-mail to <de...@subversion.apache.org>.
Re: [PATCH][RFC] allow local file:// URLs for svnadmin
Posted by Julian Foad <ju...@btopenworld.com>.
Vincent Legoll wrote:
> Hello,
>
> would such kind of patch be acceptable or is this useless ?
>
> I think there's nothing preventing svnadmin to accept local
> file:// URLs as repository location parameter. So why not ?
Let me ask a different question: Why? (We don't add features just
because we can, we add them because they are useful enough to be worth
the maintenance.)
I can think of a reason: it could be handy to copying URLs between "svn"
and "svnlook" and "svnadmin" commands. Is that your reason? Is there any
stronger reason? Would you add it to svnlook as well?
If you want to do this, remember that converting a URL to a local path
is not as simple as stripping off a prefix: there are at least two
different prefixes (file://localhost as well), and you have to un-escape
the URL escaping.
- Julian
> The attached patch, is probably wrong (you tell me) as my
> C is a bit rusty, but should show the intended change...
>
> Comments, reviews, flames, etc... All gladly accepted.
>
> If this is deemed useful, I'll especially would like directions
> on how to improve the patch to make it suitable for a real
> submission...
>
> --
> Vincent Legoll
>
> ###################### Inline ########################
>
> --- a/main.c
> +++ b/main.c
> @@ -133,6 +133,15 @@ parse_local_repos_path(apr_getopt_t *os,
> }
> else if (svn_path_is_url(*repos_path))
> {
> + #define URL_LOCAL_FILE_PREFIX "file://"
> + if (strncmp(*repos_path, URL_LOCAL_FILE_PREFIX,
> + strlen(URL_LOCAL_FILE_PREFIX)))
> + {
> + *repos_path = svn_dirent_internal_style(*repos_path +
> + strlen(URL_LOCAL_FILE_PREFIX), pool);
> + return SVN_NO_ERROR;
> + }
> +
> return svn_error_createf(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> _("'%s' is an URL when it should be a path"),
> *repos_path);
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2427054
Please start new threads on the <de...@subversion.apache.org> mailing list.
To subscribe to the new list, send an empty e-mail to <de...@subversion.apache.org>.