Re: [PATCH] Re: propedit on svn:mime-type

In message <3E6BBC52.2020205@xbc.nu>
          Branko Cibej <brane@xbc.nu> wrote:

> It would be a lot better if you created this stringbuf only if you
> actually know that you'll have to munge the value. Or at least, if the
> property name is one of those we recognise. You could move this into
> each of the "if (strcmp...)" bits. I know it means copying the
> svn_stringbuf_create_from_string call six times, but it also means you
> won't go copying values for those props that we don't care about.

Good point. Updated patch below, more tests to follow later.

Alex

* subversion/libsvn_wc/props.c
  (svn_wc_prop_set): Force svn:executable to a specific value, Strip all
  leading and trailing whitespace for svn:mime-type, svn:eol-style and
  svn:keywords. Make sure that the last line in the prop value ends in a
  newline for svn:ignore and svn:externals.

* subversion/include/svn_props.h
  (SVN_PROP_EXECUTABLE_VALUE): New value to force svn:executable to.

* subversion/tests/clients/cmdline/prop_tests.py
  (copy_should_use_copied_executable_and_mime_type_values): Take account of
  svn:executable being forced to "*".
  (strip_or_add_whitespace): New test for whitespace stripping.

* subversion/tests/clients/cmdline/schedule_tests.py
  (add_executable): Take account of svn:executable being forced to "*".

* doc/book/book/ch06.xml: Mention new behaviour of svn:executable.

Index: subversion/include/svn_props.h
===================================================================
--- subversion/include/svn_props.h (revision 5249)
+++ subversion/include/svn_props.h (working copy)
@@ -165,6 +165,9 @@
 /** Set to either TRUE or FALSE if we want a file to be executable or not. */
 #define SVN_PROP_EXECUTABLE SVN_PROP_PREFIX "executable"
 
+/** The value to force the executable property to when set */
+#define SVN_PROP_EXECUTABLE_VALUE "*"
+
 /** Describes external items to check out into this directory.
  *
  * Describes external items to check out into this directory.
Index: subversion/libsvn_wc/props.c
===================================================================
--- subversion/libsvn_wc/props.c (revision 5249)
+++ subversion/libsvn_wc/props.c (working copy)
@@ -1103,6 +1103,7 @@
   apr_hash_t *prophash;
   apr_file_t *fp = NULL;
   svn_subst_keywords_t *old_keywords;
+ svn_stringbuf_t *new_value = NULL;
   svn_node_kind_t kind;
   enum svn_prop_kind prop_kind = svn_property_kind (NULL, name);
 
@@ -1124,7 +1125,30 @@
     {
       SVN_ERR (validate_prop_against_node_kind (name, path, kind, pool));
       if (strcmp (name, SVN_PROP_EOL_STYLE) == 0)
- SVN_ERR (validate_eol_prop_against_file (path, adm_access, pool));
+ {
+ new_value = svn_stringbuf_create_from_string (value, pool);
+ svn_stringbuf_strip_whitespace (new_value);
+ SVN_ERR (validate_eol_prop_against_file (path, adm_access, pool));
+ }
+ else if (strcmp (name, SVN_PROP_MIME_TYPE) == 0)
+ {
+ new_value = svn_stringbuf_create_from_string (value, pool);
+ svn_stringbuf_strip_whitespace (new_value);
+ SVN_ERR (svn_mime_type_validate (new_value->data, pool));
+ }
+ else if (strcmp (name, SVN_PROP_IGNORE) == 0
+ || strcmp (name, SVN_PROP_EXTERNALS) == 0)
+ {
+ new_value = svn_stringbuf_create_from_string (value, pool);
+ /* Make sure that the last line ends in a newline */
+ if (new_value->data[new_value->len - 1] != '\n')
+ svn_stringbuf_appendbytes (new_value, "\n", sizeof ("\n"));
+ }
+ else if (strcmp (name, SVN_PROP_KEYWORDS) == 0)
+ {
+ new_value = svn_stringbuf_create_from_string (value, pool);
+ svn_stringbuf_strip_whitespace (new_value);
+ }
     }
 
   if (kind == svn_node_file && strcmp (name, SVN_PROP_EXECUTABLE) == 0)
@@ -1133,14 +1157,18 @@
          If the svn:executable property was deleted (NULL value passed
          in), then chmod -x. */
       if (value == NULL)
- SVN_ERR (svn_io_set_file_executable (path, FALSE, TRUE, pool));
+ {
+ SVN_ERR (svn_io_set_file_executable (path, FALSE, TRUE, pool));
+ }
       else
- SVN_ERR (svn_io_set_file_executable (path, TRUE, TRUE, pool));
+ {
+ /* Since we only check if the property exists or not, force the
+ property value to a specific value */
+ new_value = svn_stringbuf_create_from_string (value, pool);
+ svn_stringbuf_set (new_value, SVN_PROP_EXECUTABLE_VALUE);
+ SVN_ERR (svn_io_set_file_executable (path, TRUE, TRUE, pool));
+ }
     }
- else if ((strcmp (name, SVN_PROP_MIME_TYPE) == 0) && value)
- {
- SVN_ERR (svn_mime_type_validate (value->data, pool));
- }
 
   err = svn_wc_prop_list (&prophash, path, adm_access, pool);
   if (err)
@@ -1162,6 +1190,8 @@
 
   /* Now we have all the properties in our hash. Simply merge the new
      property into it. */
+ if (new_value)
+ value = svn_string_create_from_buf (new_value, pool);
   apr_hash_set (prophash, name, APR_HASH_KEY_STRING, value);
   
   /* Open the propfile for writing. */
Index: subversion/tests/clients/cmdline/prop_tests.py
===================================================================
--- subversion/tests/clients/cmdline/prop_tests.py (revision 5249)
+++ subversion/tests/clients/cmdline/prop_tests.py (working copy)
@@ -699,12 +699,13 @@
     status = 1
 
   # Check the svn:executable value.
+ # The value of the svn:executable property is now always forced to '*'
   if os.name == 'posix':
     actual_stdout, actual_stderr = svntest.main.run_svn(None,
                                                         'pg',
                                                         'svn:executable',
                                                         new_path2)
- expected_stdout = ['on\n']
+ expected_stdout = ['*\n']
     if actual_stdout != expected_stdout:
       print "svn pg svn:executable output does not match expected."
       print "Expected standard output: ", expected_stdout, "\n"
@@ -737,7 +738,105 @@
 
   return 0
 
+#----------------------------------------------------------------------
 
+def strip_or_add_whitespace(sbox):
+ "some svn: properties should have whitespace stripped or added"
+
+ # Bootstrap
+ if sbox.build():
+ return 1
+
+ wc_dir = sbox.wc_dir
+ A_path = os.path.join(wc_dir, 'A')
+ iota_path = os.path.join(wc_dir, 'iota')
+
+ # Leading and trailing whitespace should be stripped
+ svntest.main.run_svn(None, 'propset', 'svn:mime-type', ' text/html\n\n',
+ iota_path)
+
+ # Leading and trailing whitespace should be stripped
+ svntest.main.run_svn(None, 'propset', 'svn:eol-style', '\nnative\n',
+ iota_path)
+
+ # A trailing newline should be added
+ svntest.main.run_svn(None, 'propset', 'svn:ignore', '*.o\nfoo.c',
+ A_path)
+
+ # A trailing newline should be added
+ svntest.main.run_svn(None, 'propset', 'svn:externals',
+ 'foo http://foo.com/repos', A_path)
+
+ # Leading and trailing whitespace should be stripped, but not internal
+ # whitespace
+ svntest.main.run_svn(None, 'propset', 'svn:keywords', ' Rev Date \n',
+ iota_path)
+
+
+ # Check svn:mime-type
+ actual_stdout, actual_stderr = svntest.main.run_svn(None,
+ 'pg',
+ 'svn:mime-type',
+ iota_path)
+ expected_stdout = ['text/html\n']
+ if actual_stdout != expected_stdout:
+ print "svn pg svn:mime-type output does not match expected."
+ print "Expected standard output: ", expected_stdout, "\n"
+ print "Actual standard output: ", actual_stdout, "\n"
+ return 1
+
+ # Check svn:eol-style
+ actual_stdout, actual_stderr = svntest.main.run_svn(None,
+ 'pg',
+ 'svn:eol-style',
+ iota_path)
+ expected_stdout = ['native\n']
+ if actual_stdout != expected_stdout:
+ print "svn pg svn:eol-style output does not match expected."
+ print "Expected standard output: ", expected_stdout, "\n"
+ print "Actual standard output: ", actual_stdout, "\n"
+ return 1
+
+ # Check svn:ignore
+ actual_stdout, actual_stderr = svntest.main.run_svn(None,
+ 'pg',
+ 'svn:ignore',
+ A_path)
+ expected_stdout = ['*.o\n', 'foo.c\n', '\n']
+ if actual_stdout != expected_stdout:
+ print "svn pg svn:ignore output does not match expected."
+ print "Expected standard output: ", expected_stdout, "\n"
+ print "Actual standard output: ", actual_stdout, "\n"
+ return 1
+
+ # Check svn:externals
+ actual_stdout, actual_stderr = svntest.main.run_svn(None,
+ 'pg',
+ 'svn:externals',
+ A_path)
+ expected_stdout = ['foo http://foo.com/repos\n', '\n']
+ if actual_stdout != expected_stdout:
+ print "svn pg svn:externals output does not match expected."
+ print "Expected standard output: ", expected_stdout, "\n"
+ print "Actual standard output: ", actual_stdout, "\n"
+ return 1
+
+ # Check svn:keywords
+ actual_stdout, actual_stderr = svntest.main.run_svn(None,
+ 'pg',
+ 'svn:keywords',
+ iota_path)
+ expected_stdout = ['Rev Date\n']
+ if actual_stdout != expected_stdout:
+ print "svn pg svn:keywords output does not match expected."
+ print "Expected standard output: ", expected_stdout, "\n"
+ print "Actual standard output: ", actual_stdout, "\n"
+ return 1
+
+ return 0
+
+
+
 ########################################################################
 # Run the tests
 
@@ -757,6 +856,7 @@
               # If we learn how to write a pre-revprop-change hook for
               # non-Posix platforms, we won't have to skip here:
               Skip(revprop_change, (os.name != 'posix')),
+ strip_or_add_whitespace,
              ]
 
 if __name__ == '__main__':
Index: subversion/tests/clients/cmdline/schedule_tests.py
===================================================================
--- subversion/tests/clients/cmdline/schedule_tests.py (revision 5249)
+++ subversion/tests/clients/cmdline/schedule_tests.py (working copy)
@@ -177,7 +177,7 @@
   def runTest(wc_dir, fileName, perm, executable):
     fileName = os.path.join(wc_dir, fileName)
     if executable:
- expected = (["\n"], [])
+ expected = (["*\n"], [])
     else:
       expected = ([], [])
     f = open(fileName,"w")
Index: doc/book/book/ch06.xml
===================================================================
--- doc/book/book/ch06.xml (revision 5249)
+++ doc/book/book/ch06.xml (working copy)
@@ -931,16 +931,10 @@
               <literal>.COM</literal>) to denote executable
               files.</para>
           </footnote>
- Also, while it has no defined values, some people choose as
- a convention a value (such as <literal>on</literal>) to use
- when setting this property. Subversion doesn't
- care&mdash;it won't even read the property value. But be
- careful when choosing such conventions&mdash;inexperienced
- users might mistakenly believe that the way to disable this
- functionality is to change the property value to its
- idiomatic <!-- is that the right word? --> opposite (in this
- case, <literal>off</literal>). Finally, this property is
- valid only on files, not on directories.</para>
+ Also, although it has no defined values, Subversion will force
+ its value to <literal>*</literal> when setting this property.
+ Finally, this property is valid only on files, not on
+ directories.</para>
 
       </sect3>
 

-- 
Alex Waugh                  alex@alexwaugh.com
RISC OS Software from http://www.alexwaugh.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org