You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2013/07/19 22:46:22 UTC

svn commit: r1505006 - in /subversion/trunk: ./ subversion/include/svn_io.h subversion/libsvn_client/patch.c subversion/libsvn_fs_fs/fs_fs.c subversion/libsvn_subr/io.c

Author: stefan2
Date: Fri Jul 19 20:46:22 2013
New Revision: 1505006

URL: http://svn.apache.org/r1505006
Log:
Merge revisions r1433848,1438408,1445080 from branches/fsfs-format7.
These introduce svn_io_file_create_empty and svn_io_file_create_binary
as alternatives to svn_io_file_create.

A few conflicts had to be resolved.

Modified:
    subversion/trunk/   (props changed)
    subversion/trunk/subversion/include/svn_io.h
    subversion/trunk/subversion/libsvn_client/patch.c
    subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
    subversion/trunk/subversion/libsvn_subr/io.c

Propchange: subversion/trunk/
------------------------------------------------------------------------------
  Merged /subversion/branches/fsfs-format7:r1433848,1438408,1445080

Modified: subversion/trunk/subversion/include/svn_io.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_io.h?rev=1505006&r1=1505005&r2=1505006&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_io.h (original)
+++ subversion/trunk/subversion/include/svn_io.h Fri Jul 19 20:46:22 2013
@@ -691,6 +691,27 @@ svn_io_file_create(const char *file,
                    const char *contents,
                    apr_pool_t *pool);
 
+/** Create file at utf8-encoded @a file with binary contents @a contents
+ * of @a length bytes.  @a file must not already exist.
+ * Use @a pool for memory allocations.
+ *
+ * @since New in 1.9.
+ */
+svn_error_t *
+svn_io_file_create_binary(const char *file,
+                          const char *contents,
+                          apr_size_t length,
+                          apr_pool_t *pool);
+
+/** Create empty file at utf8-encoded @a file, which must not already exist.
+ * Use @a pool for memory allocations.
+ *
+ * @since New in 1.9.
+ */
+svn_error_t *
+svn_io_file_create_empty(const char *file,
+                         apr_pool_t *pool);
+
 /**
  * Lock file at @a lock_file. If @a exclusive is TRUE,
  * obtain exclusive lock, otherwise obtain shared lock.

Modified: subversion/trunk/subversion/libsvn_client/patch.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/patch.c?rev=1505006&r1=1505005&r2=1505006&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/patch.c (original)
+++ subversion/trunk/subversion/libsvn_client/patch.c Fri Jul 19 20:46:22 2013
@@ -2676,8 +2676,8 @@ install_patched_prop_targets(patch_targe
         {
           if (! dry_run)
             {
-              SVN_ERR(svn_io_file_create(target->local_abspath, "",
-                                         scratch_pool));
+              SVN_ERR(svn_io_file_create_empty(target->local_abspath,
+                                               scratch_pool));
               SVN_ERR(svn_wc_add_from_disk2(ctx->wc_ctx, target->local_abspath,
                                             NULL /*props*/,
                                             /* suppress notification */

Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1505006&r1=1505005&r2=1505006&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Fri Jul 19 20:46:22 2013
@@ -602,7 +602,7 @@ get_lock_on_filesystem(const char *lock_
       svn_error_clear(err);
       err = NULL;
 
-      SVN_ERR(svn_io_file_create(lock_filename, "", pool));
+      SVN_ERR(svn_io_file_create_empty(lock_filename, pool));
       SVN_ERR(svn_io_file_lock2(lock_filename, TRUE, FALSE, pool));
     }
 
@@ -6633,16 +6633,16 @@ svn_fs_fs__create_txn(svn_fs_txn_t **txn
   SVN_ERR(create_new_txn_noderev_from_rev(fs, txn->id, root_id, pool));
 
   /* Create an empty rev file. */
-  SVN_ERR(svn_io_file_create(path_txn_proto_rev(fs, txn->id, pool), "",
-                             pool));
+  SVN_ERR(svn_io_file_create_empty(path_txn_proto_rev(fs, txn->id, pool),
+                                   pool));
 
   /* Create an empty rev-lock file. */
-  SVN_ERR(svn_io_file_create(path_txn_proto_rev_lock(fs, txn->id, pool), "",
-                             pool));
+  SVN_ERR(svn_io_file_create_empty(path_txn_proto_rev_lock(fs, txn->id, pool),
+                                   pool));
 
   /* Create an empty changes file. */
-  SVN_ERR(svn_io_file_create(path_txn_changes(fs, txn->id, pool), "",
-                             pool));
+  SVN_ERR(svn_io_file_create_empty(path_txn_changes(fs, txn->id, pool),
+                                   pool));
 
   /* Create the next-ids file. */
   return svn_io_file_create(path_txn_next_ids(fs, txn->id, pool), "0 0\n",
@@ -8867,7 +8867,7 @@ svn_fs_fs__create(svn_fs_t *fs,
                              (format >= SVN_FS_FS__MIN_NO_GLOBAL_IDS_FORMAT
                               ? "0\n" : "0 1 1\n"),
                              pool));
-  SVN_ERR(svn_io_file_create(path_lock(fs, pool), "", pool));
+  SVN_ERR(svn_io_file_create_empty(path_lock(fs, pool), pool));
   SVN_ERR(svn_fs_fs__set_uuid(fs, NULL, pool));
 
   SVN_ERR(write_revision_zero(fs));
@@ -8884,10 +8884,8 @@ svn_fs_fs__create(svn_fs_t *fs,
      the transaction sequence file. */
   if (format >= SVN_FS_FS__MIN_TXN_CURRENT_FORMAT)
     {
-      SVN_ERR(svn_io_file_create(path_txn_current(fs, pool),
-                                 "0\n", pool));
-      SVN_ERR(svn_io_file_create(path_txn_current_lock(fs, pool),
-                                 "", pool));
+      SVN_ERR(svn_io_file_create(path_txn_current(fs, pool), "0\n", pool));
+      SVN_ERR(svn_io_file_create_empty(path_txn_current_lock(fs, pool), pool));
     }
 
   /* This filesystem is ready.  Stamp it with a format number. */
@@ -11482,7 +11480,7 @@ hotcopy_create_empty_dest(svn_fs_t *src_
                              pool));
 
   /* Create lock file and UUID. */
-  SVN_ERR(svn_io_file_create(path_lock(dst_fs, pool), "", pool));
+  SVN_ERR(svn_io_file_create_empty(path_lock(dst_fs, pool), pool));
   SVN_ERR(svn_fs_fs__set_uuid(dst_fs, src_fs->uuid, pool));
 
   /* Create the min unpacked rev file. */
@@ -11495,8 +11493,8 @@ hotcopy_create_empty_dest(svn_fs_t *src_
     {
       SVN_ERR(svn_io_file_create(path_txn_current(dst_fs, pool),
                                  "0\n", pool));
-      SVN_ERR(svn_io_file_create(path_txn_current_lock(dst_fs, pool),
-                                 "", pool));
+      SVN_ERR(svn_io_file_create_empty(path_txn_current_lock(dst_fs, pool),
+                                       pool));
     }
 
   dst_ffd->youngest_rev_cache = 0;

Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1505006&r1=1505005&r2=1505006&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Fri Jul 19 20:46:22 2013
@@ -1166,9 +1166,10 @@ svn_io_make_dir_recursively(const char *
   return SVN_NO_ERROR;
 }
 
-svn_error_t *svn_io_file_create(const char *file,
-                                const char *contents,
-                                apr_pool_t *pool)
+svn_error_t *svn_io_file_create_binary(const char *file,
+                                       const char *contents,
+                                       apr_size_t length,
+                                       apr_pool_t *pool)
 {
   apr_file_t *f;
   apr_size_t written;
@@ -1178,16 +1179,28 @@ svn_error_t *svn_io_file_create(const ch
                            (APR_WRITE | APR_CREATE | APR_EXCL),
                            APR_OS_DEFAULT,
                            pool));
-  if (contents && *contents)
-    err = svn_io_file_write_full(f, contents, strlen(contents),
-                                 &written, pool);
-
+  if (length)
+    err = svn_io_file_write_full(f, contents, length, &written, pool);
 
   return svn_error_trace(
                         svn_error_compose_create(err,
                                                  svn_io_file_close(f, pool)));
 }
 
+svn_error_t *svn_io_file_create(const char *file,
+                                const char *contents,
+                                apr_pool_t *pool)
+{
+  return svn_error_trace(svn_io_file_create_binary(file, contents,
+                                                   strlen(contents), pool));
+}
+
+svn_error_t *svn_io_file_create_empty(const char *file,
+                                      apr_pool_t *pool)
+{
+  return svn_error_trace(svn_io_file_create_binary(file, "", 0, pool));
+}
+
 svn_error_t *svn_io_dir_file_copy(const char *src_path,
                                   const char *dest_path,
                                   const char *file,



Re: svn commit: r1505006 - in /subversion/trunk: ./ subversion/include/svn_io.h subversion/libsvn_client/patch.c subversion/libsvn_fs_fs/fs_fs.c subversion/libsvn_subr/io.c

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Sat, Jul 20, 2013 at 12:30 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:

> On Sat, Jul 20, 2013 at 12:46 AM,  <st...@apache.org> wrote:
> > Author: stefan2
> > Date: Fri Jul 19 20:46:22 2013
> > New Revision: 1505006
> >
> > URL: http://svn.apache.org/r1505006
> > Log:
> > Merge revisions r1433848,1438408,1445080 from branches/fsfs-format7.
> > These introduce svn_io_file_create_empty and svn_io_file_create_binary
> > as alternatives to svn_io_file_create.
> >
> [...]
>
> >
> > +svn_error_t *svn_io_file_create(const char *file,
> > +                                const char *contents,
> > +                                apr_pool_t *pool)
> > +{
> > +  return svn_error_trace(svn_io_file_create_binary(file, contents,
> > +                                                   strlen(contents),
> pool));
> > +}
> > +
> > +svn_error_t *svn_io_file_create_empty(const char *file,
> > +                                      apr_pool_t *pool)
> > +{
> > +  return svn_error_trace(svn_io_file_create_binary(file, "", 0, pool));
> > +}
> You may just pass NULL to svn_io_file_create_binary for zero length
> data.


True but I preferred the contents pointer to always be valid.


> Also I'm not sure that svn_io_file_create_empty() is really
> needed.
>

It is more convenient, more expressive and is being used
as such in a number of places.

-- Stefan^2.

Re: svn commit: r1505006 - in /subversion/trunk: ./ subversion/include/svn_io.h subversion/libsvn_client/patch.c subversion/libsvn_fs_fs/fs_fs.c subversion/libsvn_subr/io.c

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Sat, Jul 20, 2013 at 12:46 AM,  <st...@apache.org> wrote:
> Author: stefan2
> Date: Fri Jul 19 20:46:22 2013
> New Revision: 1505006
>
> URL: http://svn.apache.org/r1505006
> Log:
> Merge revisions r1433848,1438408,1445080 from branches/fsfs-format7.
> These introduce svn_io_file_create_empty and svn_io_file_create_binary
> as alternatives to svn_io_file_create.
>
[...]

>
> +svn_error_t *svn_io_file_create(const char *file,
> +                                const char *contents,
> +                                apr_pool_t *pool)
> +{
> +  return svn_error_trace(svn_io_file_create_binary(file, contents,
> +                                                   strlen(contents), pool));
> +}
> +
> +svn_error_t *svn_io_file_create_empty(const char *file,
> +                                      apr_pool_t *pool)
> +{
> +  return svn_error_trace(svn_io_file_create_binary(file, "", 0, pool));
> +}
You may just pass NULL to svn_io_file_create_binary for zero length
data. Also I'm not sure that svn_io_file_create_empty() is really
needed.

-- 
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com

Re: svn commit: r1505006 - in /subversion/trunk: ./ subversion/include/svn_io.h subversion/libsvn_client/patch.c subversion/libsvn_fs_fs/fs_fs.c subversion/libsvn_subr/io.c

Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Sat, Jul 20, 2013 at 2:05 AM, Bert Huijben <be...@qqmail.nl> wrote:
>> -----Original Message-----
>> From: stefan2@apache.org [mailto:stefan2@apache.org]
>> Sent: vrijdag 19 juli 2013 22:46
>> To: commits@subversion.apache.org
>> Subject: svn commit: r1505006 - in /subversion/trunk: ./
>> subversion/include/svn_io.h subversion/libsvn_client/patch.c
>> subversion/libsvn_fs_fs/fs_fs.c subversion/libsvn_subr/io.c
>>
>> Author: stefan2
>> Date: Fri Jul 19 20:46:22 2013
>> New Revision: 1505006
>>
>> URL: http://svn.apache.org/r1505006
>> Log:
>> Merge revisions r1433848,1438408,1445080 from branches/fsfs-format7.
>> These introduce svn_io_file_create_empty and svn_io_file_create_binary
>> as alternatives to svn_io_file_create.
>>
>> A few conflicts had to be resolved.
>
> Can you combine the original log messages to get a proper description here with the function
> additions. This makes it much easier to see when code was introduced when looking
> through the file history.
I agree with Bert that including original log message would be really
nice to review such merges properly. Actually I prefer to have full
log messages for merges describing all changes like they did to trunk
without committing to branch. Because I don't follow feature branches,
but want to review trunk commits.


-- 
Ivan Zhakov

RE: svn commit: r1505006 - in /subversion/trunk: ./ subversion/include/svn_io.h subversion/libsvn_client/patch.c subversion/libsvn_fs_fs/fs_fs.c subversion/libsvn_subr/io.c

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

> -----Original Message-----
> From: stefan2@apache.org [mailto:stefan2@apache.org]
> Sent: vrijdag 19 juli 2013 22:46
> To: commits@subversion.apache.org
> Subject: svn commit: r1505006 - in /subversion/trunk: ./
> subversion/include/svn_io.h subversion/libsvn_client/patch.c
> subversion/libsvn_fs_fs/fs_fs.c subversion/libsvn_subr/io.c
> 
> Author: stefan2
> Date: Fri Jul 19 20:46:22 2013
> New Revision: 1505006
> 
> URL: http://svn.apache.org/r1505006
> Log:
> Merge revisions r1433848,1438408,1445080 from branches/fsfs-format7.
> These introduce svn_io_file_create_empty and svn_io_file_create_binary
> as alternatives to svn_io_file_create.
> 
> A few conflicts had to be resolved.

Can you combine the original log messages to get a proper description here with the function additions. This makes it much easier to see when code was introduced when looking through the file history.

Looking at just this merge it is not clear why svn_io_file_create_binary() needs to be added, and what additional features these functions have over svn_io_file_create(), which already creates a text file with binary encoding.

With the diff it is clear, but the reason why belongs in the log message.


Looking at the diff, I understand the value of svn_io_file_create_binary(), but svn_io_file_create_empty() still looks like a convenience function for creating empty files, while you could just pass "", 0 to svn_io_file_create_binary().

And looking at the performance of the old working copy format on Windows, I'm certainly not recommending new code to create many empty files at once.
(File creations have very bad performance characteristics on Windows and network shares)

	Bert