You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ko...@apache.org on 2021/02/23 17:01:26 UTC

svn commit: r1886843 - in /subversion/trunk/subversion: include/private/svn_wc_private.h libsvn_client/export.c libsvn_wc/update_editor.c libsvn_wc/wc_db.h libsvn_wc/working_file_writer.c libsvn_wc/working_file_writer.h libsvn_wc/workqueue.c

Author: kotkov
Date: Tue Feb 23 17:01:26 2021
New Revision: 1886843

URL: http://svn.apache.org/viewvc?rev=1886843&view=rev
Log:
During export, process files in a streamy fashion even when they need
translation.

Previously, translation of such files happened as a separate step in
close_file() and has been creating and using temporary files for that purpose.
For transports like HTTP having a separate step with nothing being read
from the connection puts an implicit limit on the size of the file that can
be exported without hitting a timeout.  Also, we have to create and re-read
those additional temporary files.

(For `svn export`, the scope of the problem is limited, compared to the
 problem with `svn checkout` or `svn update` described in r1886490,
 but I think it still exists.)

Technically, we begin using the working file writer utility in the
implementation of the export editor, like we did for the update editor.
This ensures that the intrinsic details of how the files are created on
disk will be the same for both export and checkout/update.  Also, doing
so enables various I/O optimizations provided by the install stream.


* subversion/libsvn_wc/working_file_writer.h:
  Delete the file.  Move declarations related to the working file writer ...

* subversion/include/private/svn_wc_private.h: ...here.  Include svn_subst.h.

* subversion/libsvn_wc/working_file_writer.c,
  subversion/libsvn_wc/wc_db.h,
  subversion/libsvn_wc/update_editor.c,
  subversion/libsvn_wc/workqueue.c: Adjust includes.

* subversion/libsvn_client/export.c
  (struct file_baton): Remove `tmppath`, `tmp_stream` fields. Add the new
   `file_writer` field.
  (open_working_file_writer): New helper function. Creates the writer for
   the file being exported based on the state in the file baton.
  (apply_textdelta): Open the working file writer for the file that is
   currently being exported.  Make svn_txdelta_apply() write data into the
   stream of the file writer.
  (close_file): Finalize the writer and use it to install the exported file.
  (struct handler_baton): Remove the `pool`, `tmppath` fields.
  (window_handler): No need to explicitly remove the temporary file: that is
   now performed automatically in the pool cleanup handler of the file writer.
  (export_file): Begin using the working file writer in this standalone function
   that drives the delta editor to export a single file.  Fetch the properties,
   open the working file writer, stream the file contents and complete the file
   installation.  We fetch properties and the contents in two separate calls
   to svn_ra_get_file(), like it's done in the current implementation of
   `svn cat`.  When fetching properties for HEAD revision, ask the RA layer
   to return the resolved revision number and use it in the subsequent call.

Removed:
    subversion/trunk/subversion/libsvn_wc/working_file_writer.h
Modified:
    subversion/trunk/subversion/include/private/svn_wc_private.h
    subversion/trunk/subversion/libsvn_client/export.c
    subversion/trunk/subversion/libsvn_wc/update_editor.c
    subversion/trunk/subversion/libsvn_wc/wc_db.h
    subversion/trunk/subversion/libsvn_wc/working_file_writer.c
    subversion/trunk/subversion/libsvn_wc/workqueue.c

Modified: subversion/trunk/subversion/include/private/svn_wc_private.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_wc_private.h?rev=1886843&r1=1886842&r2=1886843&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_wc_private.h (original)
+++ subversion/trunk/subversion/include/private/svn_wc_private.h Tue Feb 23 17:01:26 2021
@@ -37,6 +37,7 @@
 #ifndef SVN_WC_PRIVATE_H
 #define SVN_WC_PRIVATE_H
 
+#include "svn_subst.h"
 #include "svn_types.h"
 #include "svn_wc.h"
 #include "private/svn_diff_tree.h"
@@ -2128,6 +2129,58 @@ svn_wc__translated_stream(svn_stream_t *
                           apr_pool_t *result_pool,
                           apr_pool_t *scratch_pool);
 
+/* Context of the working file writer. */
+typedef struct svn_wc__working_file_writer_t svn_wc__working_file_writer_t;
+
+/* Create a write context for the (provisioned) working file with the specified
+   properties.  The file writer must be given data in the repository-normal
+   form and will handle its translation according to the specified properties.
+   Place the temporary file in the TMP_ABSPATH directory.  If FINAL_MTIME is
+   non-negative, it will be set as the last modification time on the installed
+   file. */
+svn_error_t *
+svn_wc__working_file_writer_open(svn_wc__working_file_writer_t **writer_p,
+                                 const char *tmp_abspath,
+                                 apr_time_t final_mtime,
+                                 svn_subst_eol_style_t eol_style,
+                                 const char *eol,
+                                 svn_boolean_t repair_eol,
+                                 apr_hash_t *keywords,
+                                 svn_boolean_t is_special,
+                                 svn_boolean_t is_executable,
+                                 svn_boolean_t needs_lock,
+                                 svn_boolean_t has_lock,
+                                 svn_boolean_t is_added,
+                                 apr_pool_t *result_pool,
+                                 apr_pool_t *scratch_pool);
+
+/* Get the writable stream for WRITER.  The returned stream supports reset
+   and is configured to be truncated on seek. */
+svn_stream_t *
+svn_wc__working_file_writer_get_stream(svn_wc__working_file_writer_t *writer);
+
+/* Finalize the content, attributes and the timestamps of the underlying
+   temporary file.  Return the properties of the finalized file in MTIME_P
+   and SIZE_P.  MTIME_P and SIZE_P both may be NULL. */
+svn_error_t *
+svn_wc__working_file_writer_finalize(apr_time_t *mtime_p,
+                                     apr_off_t *size_p,
+                                     svn_wc__working_file_writer_t *writer,
+                                     apr_pool_t *scratch_pool);
+
+/* Atomically install the contents of WRITER to TARGET_ABSPATH.
+   If the writer has not been previously finalized with a call to
+   svn_wc__working_file_writer_finalize(), the behavior is undefined. */
+svn_error_t *
+svn_wc__working_file_writer_install(svn_wc__working_file_writer_t *writer,
+                                    const char *target_abspath,
+                                    apr_pool_t *scratch_pool);
+
+/* Cleanup WRITER by closing and removing the underlying file. */
+svn_error_t *
+svn_wc__working_file_writer_close(svn_wc__working_file_writer_t *writer);
+
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */

Modified: subversion/trunk/subversion/libsvn_client/export.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/export.c?rev=1886843&r1=1886842&r2=1886843&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/export.c (original)
+++ subversion/trunk/subversion/libsvn_client/export.c Tue Feb 23 17:01:26 2021
@@ -526,11 +526,8 @@ struct file_baton
   struct edit_baton *edit_baton;
 
   const char *path;
-  const char *tmppath;
-
-  /* We need to keep this around so we can explicitly close it in close_file,
-     thus flushing its output to disk so we can copy and translate it. */
-  svn_stream_t *tmp_stream;
+  /* The writer for the file being exported. */
+  svn_wc__working_file_writer_t *file_writer;
 
   /* The MD5 digest of the file's fulltext.  This is all zeros until
      the last textdelta window handler call returns. */
@@ -558,8 +555,6 @@ struct handler_baton
 {
   svn_txdelta_window_handler_t apply_handler;
   void *apply_baton;
-  apr_pool_t *pool;
-  const char *tmppath;
 };
 
 
@@ -679,23 +674,76 @@ static svn_error_t *
 window_handler(svn_txdelta_window_t *window, void *baton)
 {
   struct handler_baton *hb = baton;
-  svn_error_t *err;
 
-  err = hb->apply_handler(window, hb->apply_baton);
-  if (err)
+  SVN_ERR(hb->apply_handler(window, hb->apply_baton));
+
+  return SVN_NO_ERROR;
+}
+
+
+/* Create the writer for the file being exported based on the
+   state in the file baton FB. */
+static svn_error_t *
+open_working_file_writer(svn_wc__working_file_writer_t **writer_p,
+                         struct file_baton *fb,
+                         apr_pool_t *result_pool,
+                         apr_pool_t *scratch_pool)
+{
+  const char *eol_style_val;
+  svn_subst_eol_style_t eol_style;
+  const char *eol;
+  apr_hash_t *keywords;
+  apr_time_t final_mtime;
+  const char *tmp_path;
+  const char *tmp_abspath;
+
+  if (fb->eol_style_val)
+    eol_style_val = fb->eol_style_val->data;
+  else
+    eol_style_val = NULL;
+
+  SVN_ERR(get_eol_style(&eol_style, &eol, eol_style_val,
+                        fb->edit_baton->native_eol));
+
+  if (fb->keywords_val)
+    {
+      SVN_ERR(svn_subst_build_keywords3(&keywords, fb->keywords_val->data,
+                                        fb->revision, fb->url,
+                                        fb->repos_root_url, fb->date,
+                                        fb->author, scratch_pool));
+    }
+  else
     {
-      /* We failed to apply the patch; clean up the temporary file.  */
-      err = svn_error_compose_create(
-                    err,
-                    svn_io_remove_file2(hb->tmppath, TRUE, hb->pool));
+      keywords = NULL;
     }
 
-  return svn_error_trace(err);
-}
+  if (fb->date)
+    final_mtime = fb->date;
+  else
+    final_mtime = -1;
 
+  /* Create a temporary file in the same directory as the file. */
+  tmp_path = svn_dirent_dirname(fb->path, scratch_pool);
+  SVN_ERR(svn_dirent_get_absolute(&tmp_abspath, tmp_path, scratch_pool));
+  SVN_ERR(svn_wc__working_file_writer_open(writer_p,
+                                           tmp_abspath,
+                                           final_mtime,
+                                           eol_style,
+                                           eol,
+                                           TRUE /* repair_eol */,
+                                           keywords,
+                                           fb->special,
+                                           fb->executable_val != NULL,
+                                           FALSE,
+                                           FALSE,
+                                           FALSE,
+                                           result_pool,
+                                           scratch_pool));
 
+  return SVN_NO_ERROR;
+}
 
-/* Write incoming data into the tmpfile stream */
+/* Write incoming data into the file writer */
 static svn_error_t *
 apply_textdelta(void *file_baton,
                 const char *base_checksum,
@@ -706,21 +754,10 @@ apply_textdelta(void *file_baton,
   struct file_baton *fb = file_baton;
   struct handler_baton *hb = apr_palloc(pool, sizeof(*hb));
 
-  /* Create a temporary file in the same directory as the file. We're going
-     to rename the thing into place when we're done. */
-  SVN_ERR(svn_stream_open_unique(&fb->tmp_stream, &fb->tmppath,
-                                 svn_dirent_dirname(fb->path, pool),
-                                 svn_io_file_del_none, fb->pool, fb->pool));
-
-  hb->pool = pool;
-  hb->tmppath = fb->tmppath;
-
-  /* svn_txdelta_apply() closes the stream, but we want to close it in the
-     close_file() function, so disown it here. */
-  /* ### contrast to when we call svn_ra_get_file() which does NOT close the
-     ### tmp_stream. we *should* be much more consistent! */
+  SVN_ERR(open_working_file_writer(&fb->file_writer, fb, fb->pool, pool));
+
   svn_txdelta_apply(svn_stream_empty(pool),
-                    svn_stream_disown(fb->tmp_stream, pool),
+                    svn_wc__working_file_writer_get_stream(fb->file_writer),
                     fb->text_digest, NULL, pool,
                     &hb->apply_handler, &hb->apply_baton);
 
@@ -785,23 +822,21 @@ change_dir_prop(void *dir_baton,
 }
 
 
-/* Move the tmpfile to file, and send feedback. */
+/* Install the file, and send feedback. */
 static svn_error_t *
 close_file(void *file_baton,
            const char *text_digest,
            apr_pool_t *pool)
 {
   struct file_baton *fb = file_baton;
-  struct edit_baton *eb = fb->edit_baton;
   svn_checksum_t *text_checksum;
   svn_checksum_t *actual_checksum;
+  const char *target_abspath;
 
   /* Was a txdelta even sent? */
-  if (! fb->tmppath)
+  if (! fb->file_writer)
     return SVN_NO_ERROR;
 
-  SVN_ERR(svn_stream_close(fb->tmp_stream));
-
   SVN_ERR(svn_checksum_parse_hex(&text_checksum, svn_checksum_md5, text_digest,
                                  pool));
   actual_checksum = svn_checksum__from_digest_md5(fb->text_digest, pool);
@@ -814,45 +849,12 @@ close_file(void *file_baton,
                                      _("Checksum mismatch for '%s'"),
                                      svn_dirent_local_style(fb->path, pool));
 
-  if ((! fb->eol_style_val) && (! fb->keywords_val) && (! fb->special))
-    {
-      SVN_ERR(svn_io_file_rename2(fb->tmppath, fb->path, FALSE, pool));
-    }
-  else
-    {
-      svn_subst_eol_style_t style;
-      const char *eol = NULL;
-      svn_boolean_t repair = FALSE;
-      apr_hash_t *final_kw = NULL;
-
-      if (fb->eol_style_val)
-        {
-          SVN_ERR(get_eol_style(&style, &eol, fb->eol_style_val->data,
-                                eb->native_eol));
-          repair = TRUE;
-        }
-
-      if (fb->keywords_val)
-        SVN_ERR(svn_subst_build_keywords3(&final_kw, fb->keywords_val->data,
-                                          fb->revision, fb->url,
-                                          fb->repos_root_url, fb->date,
-                                          fb->author, pool));
-
-      SVN_ERR(svn_subst_copy_and_translate4(fb->tmppath, fb->path,
-                                            eol, repair, final_kw,
-                                            TRUE, /* expand */
-                                            fb->special,
-                                            eb->cancel_func, eb->cancel_baton,
-                                            pool));
+  SVN_ERR(svn_dirent_get_absolute(&target_abspath, fb->path, pool));
 
-      SVN_ERR(svn_io_remove_file2(fb->tmppath, FALSE, pool));
-    }
-
-  if (fb->executable_val)
-    SVN_ERR(svn_io_set_file_executable(fb->path, TRUE, FALSE, pool));
-
-  if (fb->date && (! fb->special))
-    SVN_ERR(svn_io_set_file_affected_time(fb->date, fb->path, pool));
+  SVN_ERR(svn_wc__working_file_writer_finalize(NULL, NULL, fb->file_writer,
+                                               pool));
+  SVN_ERR(svn_wc__working_file_writer_install(fb->file_writer, target_abspath,
+                                              pool));
 
   if (fb->edit_baton->notify_func)
     {
@@ -1212,6 +1214,8 @@ export_file(const char *from_url,
   apr_hash_index_t *hi;
   struct file_baton *fb = apr_pcalloc(scratch_pool, sizeof(*fb));
   svn_node_kind_t to_kind;
+  svn_revnum_t target_rev;
+  svn_stream_t *stream;
 
   SVN_ERR_ASSERT(svn_path_is_url(from_url));
 
@@ -1251,18 +1255,20 @@ export_file(const char *from_url,
   fb->pool = scratch_pool;
   fb->repos_root_url = eb->repos_root_url;
 
-  /* Copied from apply_textdelta(). */
-  SVN_ERR(svn_stream_open_unique(&fb->tmp_stream, &fb->tmppath,
-                                 svn_dirent_dirname(fb->path, scratch_pool),
-                                 svn_io_file_del_none,
-                                 fb->pool, fb->pool));
-
-  /* Step outside the editor-likeness for a moment, to actually talk
-   * to the repository. */
-  /* ### note: the stream will not be closed */
-  SVN_ERR(svn_ra_get_file(ra_session, "", loc->rev,
-                          fb->tmp_stream,
-                          NULL, &props, scratch_pool));
+  /* Grab some properties we need to know in order to figure out if anything
+     special needs to be done with this file. */
+  target_rev = loc->rev;
+  if (SVN_IS_VALID_REVNUM(target_rev))
+    {
+      SVN_ERR(svn_ra_get_file(ra_session, "", target_rev, NULL, NULL,
+                              &props, scratch_pool));
+    }
+  else
+    {
+      /* For HEAD, fetch the actual revision and use in subsequent calls. */
+      SVN_ERR(svn_ra_get_file(ra_session, "", SVN_INVALID_REVNUM, NULL,
+                              &target_rev, &props, scratch_pool));
+    }
 
   /* Push the props into change_file_prop(), to update the file_baton
    * with information. */
@@ -1274,8 +1280,16 @@ export_file(const char *from_url,
       SVN_ERR(change_file_prop(fb, propname, propval, scratch_pool));
     }
 
-  /* And now just use close_file() to do all the keyword and EOL
-   * work, and put the file into place. */
+  /* Step outside the editor-likeness for a moment, to open the file writer
+   * and to actually talk to the repository. */
+  SVN_ERR(open_working_file_writer(&fb->file_writer, fb, fb->pool,
+                                   scratch_pool));
+  stream = svn_wc__working_file_writer_get_stream(fb->file_writer);
+  SVN_ERR(svn_ra_get_file(ra_session, "", target_rev, stream, NULL, NULL,
+                          scratch_pool));
+  SVN_ERR(svn_stream_close(stream));
+
+  /* And now just use close_file() to put the file into place. */
   SVN_ERR(close_file(fb, NULL, scratch_pool));
 
   return SVN_NO_ERROR;

Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=1886843&r1=1886842&r2=1886843&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
+++ subversion/trunk/subversion/libsvn_wc/update_editor.c Tue Feb 23 17:01:26 2021
@@ -48,7 +48,6 @@
 #include "conflicts.h"
 #include "translate.h"
 #include "workqueue.h"
-#include "working_file_writer.h"
 
 #include "private/svn_subr_private.h"
 #include "private/svn_wc_private.h"

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.h?rev=1886843&r1=1886842&r2=1886843&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.h (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.h Tue Feb 23 17:01:26 2021
@@ -49,8 +49,6 @@
 
 #include "svn_private_config.h"
 
-#include "working_file_writer.h"
-
 #ifdef __cplusplus
 extern "C" {
 #endif /* __cplusplus */

Modified: subversion/trunk/subversion/libsvn_wc/working_file_writer.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/working_file_writer.c?rev=1886843&r1=1886842&r2=1886843&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/working_file_writer.c (original)
+++ subversion/trunk/subversion/libsvn_wc/working_file_writer.c Tue Feb 23 17:01:26 2021
@@ -27,8 +27,7 @@
 #include "svn_path.h"
 
 #include "private/svn_io_private.h"
-
-#include "working_file_writer.h"
+#include "private/svn_wc_private.h"
 
 struct svn_wc__working_file_writer_t
 {

Modified: subversion/trunk/subversion/libsvn_wc/workqueue.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/workqueue.c?rev=1886843&r1=1886842&r2=1886843&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/workqueue.c (original)
+++ subversion/trunk/subversion/libsvn_wc/workqueue.c Tue Feb 23 17:01:26 2021
@@ -38,9 +38,9 @@
 #include "adm_files.h"
 #include "conflicts.h"
 #include "translate.h"
-#include "working_file_writer.h"
 
 #include "private/svn_io_private.h"
+#include "private/svn_wc_private.h"
 #include "private/svn_skel.h"