You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Senthil Kumaran S <se...@collab.net> on 2009/03/12 09:09:18 UTC
[PATCH] Fix issue #2068
Hi,
The attached patch gives a solution to fix issue #2068, though this is not
backward compatible with earlier versions of subversion.
[[[
Fix issue #2068.
NOTE: This is not backward compatible.
* subversion/libsvn_subr/config_file.c
(svn_config_ensure): Change the delimiter between properties to '|'
from ';' in default config file.
* subversion/libsvn_client/add.c
(auto_props_enumerator): Change delimiter to '|' from ';'
* subversion/tests/cmdline/autoprop_tests.py
(create_config): Reflect above change.
]]]
Thank You.
--
Senthil Kumaran S
http://www.stylesen.org/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1311793
Re: [PATCH] Fix issue #2068
Posted by Senthil Kumaran S <se...@collab.net>.
Senthil Kumaran S wrote:
> The attached patch gives a solution to fix issue #2068, though this is not
> backward compatible with earlier versions of subversion.
If we have the following line in ~/.subversion/config
<snip>
[auto-props]
*.txt = svn:mime-type=text/plain;charset=UTF-8|svn:eol-style=native
</snip>
An 'svn add' will produce the following with this patch:
<snip>
$ svn di
Index: file.txt
===================================================================
--- file.txt (revision 0)
+++ file.txt (revision 0)
@@ -0,0 +1 @@
+some file.
Property changes on: file.txt
___________________________________________________________________
Added: svn:mime-type
+ text/plain;charset=UTF-8
Added: svn:eol-style
+ native
</snip>
OTOH, a config line like the following:
<snip>
[auto-props]
*.txt = svn:mime-type=text/plain;charset=UTF-8;svn:eol-style=native
</snip>
without this patch 'svn add' will give the following:
<snip>
$ svn di
Index: file.txt
===================================================================
--- file.txt (revision 0)
+++ file.txt (revision 0)
@@ -0,0 +1 @@
+some file.
Property changes on: file.txt
___________________________________________________________________
Added: svn:mime-type
+ text/plain
Added: charset
+ UTF-8
Added: svn:eol-style
+ native
</snip>
Thank You.
--
Senthil Kumaran S
http://www.stylesen.org/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1311814
Re: [PATCH] Fix issue #2068
Posted by "C. Michael Pilato" <cm...@collab.net>.
Senthil Kumaran S wrote:
> Senthil Kumaran S wrote:
>> [[[
>> Fix issue #2068.
>>
>> * subversion/libsvn_client/add.c
>> (split_props): New function to split individual properties, which escapes
>> double semi-colons ie., ";;"
>> (auto_props_enumerator): Use split_props to collect individual properties
>> and then store each one of them.
>>
>> Suggested by: rhuijben
>> ]]]
>
> Is there any comments on this patch? If not I shall commit it tomorrow.
goforit
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1402627
Re: [PATCH] Fix issue #2068
Posted by Senthil Kumaran S <se...@collab.net>.
Senthil Kumaran S wrote:
> [[[
> Fix issue #2068.
>
> * subversion/libsvn_client/add.c
> (split_props): New function to split individual properties, which escapes
> double semi-colons ie., ";;"
> (auto_props_enumerator): Use split_props to collect individual properties
> and then store each one of them.
>
> Suggested by: rhuijben
> ]]]
Is there any comments on this patch? If not I shall commit it tomorrow.
Thank You.
--
Senthil Kumaran S
http://www.stylesen.org/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1398966
Re: [PATCH] Fix issue #2068
Posted by Senthil Kumaran S <se...@collab.net>.
Hi,
Bert Huijben wrote:
> * It doesn't handle multiple escaped semicolons as
> "myprop=string;;;;continued;nextprop=q" and
> "myprop=string;;and;;string;nextprop=q"
>
> I'm not sure if these issues are all fixable using apr_strtok, as that
> function replaces the ';' with a '\0'.
Here is my latest/updated patch attached :) This does not break backward
compatibility.
[[[
Fix issue #2068.
* subversion/libsvn_client/add.c
(split_props): New function to split individual properties, which escapes
double semi-colons ie., ";;"
(auto_props_enumerator): Use split_props to collect individual properties
and then store each one of them.
Suggested by: rhuijben
]]]
Thank You.
--
Senthil Kumaran S
http://www.stylesen.org/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1370101
Re: [PATCH] Fix issue #2068
Posted by Senthil Kumaran S <se...@collab.net>.
Bert Huijben wrote:
> * It doesn't handle multiple escaped semicolons as
> "myprop=string;;;;continued;nextprop=q" and
> "myprop=string;;and;;string;nextprop=q"
Lately after sending the patch I realized it :(
--
Senthil Kumaran S
http://www.stylesen.org/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1340978
RE: [PATCH] Fix issue #2068
Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: Senthil Kumaran S [mailto:senthil@collab.net]
> Sent: Tuesday, March 17, 2009 12:32 PM
> To: David Glasser
> Cc: Bert Huijben; dev@subversion.tigris.org; C. Michael Pilato
> Subject: Re: [PATCH] Fix issue #2068
>
> David Glasser wrote:
> > On Thu, Mar 12, 2009 at 3:03 AM, Bert Huijben <rh...@sharpsvn.net>
> wrote:
> >> Maybe we should use a double semicolon as escape sequence for
> passing a
> >> semicolon. I don't like the idea of breaking all old config files,
> registry
> >> settings, etc..
> >>
> >> This fix only removes the semicolon problem, but it breaks putting |
> >> characters in properties.
> >
> > That's a great suggestion, since in the previous format, any double
> > semicolon was completely redundant.
>
> As per above suggestion, I am sending this updated patch. Let me know
> if it
> needs any change, if not I shall commit it tomorrow.
>
> [[[
> Fix issue #2068.
>
> * subversion/libsvn_client/add.c
> (auto_props_enumerator): Add logic to escape double semicolons.
>
> Suggested by: rhuijben
> ]]]
>
> Thank You.
Looking at the patch,
Index: subversion/libsvn_client/add.c
===================================================================
--- subversion/libsvn_client/add.c (revision 36615)
+++ subversion/libsvn_client/add.c (working copy)
@@ -113,8 +113,17 @@
{
int len;
const char *this_value;
- char *equal_sign = strchr(property, '=');
+ char *equal_sign;
+ char *more_prop;
+ if (property[(strlen(property)+1)] == ';')
+ {
+ more_prop = apr_strtok(NULL, ";", &last_token);
+ property = apr_pstrcat(autoprops->pool, property, ";",
+ more_prop, NULL);
+ }
+ equal_sign = strchr(property, '=');
+
if (equal_sign)
{
*equal_sign = '\0';
--------------------
I see a few issues
* It misses an end of the string test (property[(strlen(property)+1)] could
be outside the allocated buffer)
* It doesn't handle multiple escaped semicolons as
"myprop=string;;;;continued;nextprop=q" and
"myprop=string;;and;;string;nextprop=q"
I'm not sure if these issues are all fixable using apr_strtok, as that
function replaces the ';' with a '\0'.
(The first case could be checked by comparing *last_token to '\0', but that
is based on knowledge of the current implementation of apr_strtok())
Bert
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1340969
Re: [PATCH] Fix issue #2068
Posted by Senthil Kumaran S <se...@collab.net>.
David Glasser wrote:
> On Thu, Mar 12, 2009 at 3:03 AM, Bert Huijben <rh...@sharpsvn.net> wrote:
>> Maybe we should use a double semicolon as escape sequence for passing a
>> semicolon. I don't like the idea of breaking all old config files, registry
>> settings, etc..
>>
>> This fix only removes the semicolon problem, but it breaks putting |
>> characters in properties.
>
> That's a great suggestion, since in the previous format, any double
> semicolon was completely redundant.
As per above suggestion, I am sending this updated patch. Let me know if it
needs any change, if not I shall commit it tomorrow.
[[[
Fix issue #2068.
* subversion/libsvn_client/add.c
(auto_props_enumerator): Add logic to escape double semicolons.
Suggested by: rhuijben
]]]
Thank You.
--
Senthil Kumaran S
http://www.stylesen.org/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1340243
Re: [PATCH] Fix issue #2068
Posted by David Glasser <gl...@davidglasser.net>.
On Thu, Mar 12, 2009 at 3:03 AM, Bert Huijben <rh...@sharpsvn.net> wrote:
>> -----Original Message-----
>> From: Senthil Kumaran S [mailto:senthil@collab.net]
>> Sent: donderdag 12 maart 2009 10:09
>> To: dev@subversion.tigris.org
>> Subject: [PATCH] Fix issue #2068
>>
>> Hi,
>>
>> The attached patch gives a solution to fix issue #2068, though this is not
>> backward compatible with earlier versions of subversion.
>>
>> [[[
>> Fix issue #2068.
>>
>> NOTE: This is not backward compatible.
>>
>> * subversion/libsvn_subr/config_file.c
>> (svn_config_ensure): Change the delimiter between properties to '|'
>> from ';' in default config file.
>
> Maybe we should use a double semicolon as escape sequence for passing a
> semicolon. I don't like the idea of breaking all old config files, registry
> settings, etc..
>
> This fix only removes the semicolon problem, but it breaks putting |
> characters in properties.
That's a great suggestion, since in the previous format, any double
semicolon was completely redundant.
--dave
--
glasser@davidglasser.net | langtonlabs.org | flickr.com/photos/glasser/
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1313694
Re: [PATCH] Fix issue #2068
Posted by Daniel Rall <dl...@finemaltcoding.com>.
On Thu, Mar 12, 2009 at 7:43 AM, C. Michael Pilato <cm...@collab.net> wrote:
> Bert Huijben wrote:
>> It is very hard to introduce an escape when you previously accepted
>> everything as valid :(
>
> Not if you rename the configuration section to [auto-props2].
>
> /me ducks.
Ewww.
This could be fixed by adding a "format-number" field (e.g. to the
"[global]" group), and doing a second pass of parsing on the raw
config file data to handle parsing escape characters (I favor
doubling-up the semi-colons as suggested by Bert, a la quotes in SQL).
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1313257
Re: [PATCH] Fix issue #2068
Posted by "C. Michael Pilato" <cm...@collab.net>.
Bert Huijben wrote:
> It is very hard to introduce an escape when you previously accepted
> everything as valid :(
Not if you rename the configuration section to [auto-props2].
/me ducks.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1313183
RE: [PATCH] Fix issue #2068
Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: C. Michael Pilato [mailto:cmpilato@collab.net]
> Sent: donderdag 12 maart 2009 14:49
> To: Bert Huijben
> Cc: 'Senthil Kumaran S'; dev@subversion.tigris.org
> Subject: Re: [PATCH] Fix issue #2068
>
> Bert Huijben wrote:
> >> -----Original Message-----
> >> From: Senthil Kumaran S [mailto:senthil@collab.net]
> >> Sent: donderdag 12 maart 2009 10:09
> >> To: dev@subversion.tigris.org
> >> Subject: [PATCH] Fix issue #2068
> >>
> >> Hi,
> >>
> >> The attached patch gives a solution to fix issue #2068, though this is not
> >> backward compatible with earlier versions of subversion.
> >>
> >> [[[
> >> Fix issue #2068.
> >>
> >> NOTE: This is not backward compatible.
> >>
> >> * subversion/libsvn_subr/config_file.c
> >> (svn_config_ensure): Change the delimiter between properties to '|'
> >> from ';' in default config file.
> >
> > Maybe we should use a double semicolon as escape sequence for passing a
> > semicolon. I don't like the idea of breaking all old config files, registry
> > settings, etc..
>
> <span value="$0.02">I would much more prefer that we stick with some more
> commonly used escape character, such as the backslash (\), in any places
> where that character can't be reasonably interpreted as a Windows directory
> separator.</span>
I agree that the '\' is a very common escape, but in this case we used to parse everything except the semicolon as value, so I think the semicolon is the only character we can use 100% fully backwards compatible. But maybe \; is so uncommon we could use that too, but I'm not sure if using \ as escape for every character would be wise.
It is very hard to introduce an escape when you previously accepted everything as valid :(
Bert
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1313139
Re: [PATCH] Fix issue #2068
Posted by "C. Michael Pilato" <cm...@collab.net>.
Bert Huijben wrote:
>> -----Original Message-----
>> From: Senthil Kumaran S [mailto:senthil@collab.net]
>> Sent: donderdag 12 maart 2009 10:09
>> To: dev@subversion.tigris.org
>> Subject: [PATCH] Fix issue #2068
>>
>> Hi,
>>
>> The attached patch gives a solution to fix issue #2068, though this is not
>> backward compatible with earlier versions of subversion.
>>
>> [[[
>> Fix issue #2068.
>>
>> NOTE: This is not backward compatible.
>>
>> * subversion/libsvn_subr/config_file.c
>> (svn_config_ensure): Change the delimiter between properties to '|'
>> from ';' in default config file.
>
> Maybe we should use a double semicolon as escape sequence for passing a
> semicolon. I don't like the idea of breaking all old config files, registry
> settings, etc..
<span value="$0.02">I would much more prefer that we stick with some more
commonly used escape character, such as the backslash (\), in any places
where that character can't be reasonably interpreted as a Windows directory
separator.</span>
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1312963
RE: [PATCH] Fix issue #2068
Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: Senthil Kumaran S [mailto:senthil@collab.net]
> Sent: donderdag 12 maart 2009 10:09
> To: dev@subversion.tigris.org
> Subject: [PATCH] Fix issue #2068
>
> Hi,
>
> The attached patch gives a solution to fix issue #2068, though this is not
> backward compatible with earlier versions of subversion.
>
> [[[
> Fix issue #2068.
>
> NOTE: This is not backward compatible.
>
> * subversion/libsvn_subr/config_file.c
> (svn_config_ensure): Change the delimiter between properties to '|'
> from ';' in default config file.
Maybe we should use a double semicolon as escape sequence for passing a
semicolon. I don't like the idea of breaking all old config files, registry
settings, etc..
This fix only removes the semicolon problem, but it breaks putting |
characters in properties.
Bert
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1312094