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/21 23:58:58 UTC

svn commit: r1886774 - in /subversion/trunk/subversion: include/private/ libsvn_subr/ libsvn_wc/ tests/libsvn_subr/

Author: kotkov
Date: Sun Feb 21 23:58:57 2021
New Revision: 1886774

URL: http://svn.apache.org/viewvc?rev=1886774&view=rev
Log:
Rework the timestamp handling in the working file writer utility layer.

This change aims at solving two problems where using that writer could
have been causing differences between the timestamp values on the working
files and in the wc.db.

The first problem is that on Windows (NTFS on some older versions and other
file systems such as ReFS, FAT32, ...), the file systems may defer processing
of timestamps until the file handle is closed [1].  So when we peek and return
the current timestamp, we must also ensure that the timestamp does not change
after the handle to the working file is closed.

We haven't been taking that into the account before, and that is what has
been causing some of the timestamp discrepancies.

Luckily, there are two options that guarantee that the file will keep its
current timestamp after close.  We can either explicitly set a new timestamp,
or use a special option that instructs the file system to suspend updates for
timestamp values for all subsequent I/O operations. Both of these options
guarantee [2, 3] that no other operation will change the final timestamp.
And we use both of them, depending on whether we have to set a specific
timestamp, or not.

The second problem is that sometimes, e.g. when using the `use-commit-times`
option, the install stream would return a timestamp without re-fetching it
from the OS.  That would cause a timestamp discrepancy if the timestamp
precision of the filesystem is different from the microsecond precision
of the apr_time_t.

Technically, we solve both problems by introducing a private API that allows
"finalizing" an install stream, during which we set the final attributes and
the timestamps of the file, and optionally return them to the caller.
The timestamp values are always re-fetched from the OS.

This is a follow-up to r1886490.

[1: MS-FSA, 2.1.4.17, Note <42>]
File systems may choose to defer processing for a file that has been
modified to a later time, favoring performance over accuracy. The NTFS
file system on versions prior to Windows 10 v1809 operating system,
Windows Server v1809 operating system, and Windows Server 2019, and
non-NTFS file systems on all versions of Windows, defer this processing
until the Open gets closed.
https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-fsa/4e3695bd-7574-4f24-a223-b4679c065b63#Appendix_A_42

[2: MS-FSA 2.1.5.14.2]
If InputBuffer.LastWriteTime != 0:
  If InputBuffer.LastWriteTime != -2:
    The object store MUST set Open.UserSetModificationTime to TRUE.
https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-fsa/a36513b4-73c8-4888-ad29-8f3a196567e8

[3: MS-FSA 2.1.4.17]
If Open.UserSetModificationTime is FALSE, set Open.File.LastModificationTime
to the current system time.
https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-fsa/75cdaba1-4401-4c53-b09c-69ba6cd50ce6


* subversion/include/private/svn_io_private.h
  (svn_stream__install_get_info): Remove.
  (svn_stream__install_finalize): New.
  (svn_stream__install_stream): Tweak docstring.
  (SVN_IO__WIN_TIME_UNCHANGED,
   SVN_IO__WIN_TIME_SUSPEND_UPDATE): New constants.
  (svn_io__win_set_file_basic_info): Tweak docstring.

* subversion/libsvn_subr/io.c
  (svn_io__win_set_file_basic_info): Handle two new special values,
   SVN_IO__WIN_TIME_UNCHANGED and SVN_IO__WIN_TIME_SUSPEND_UPDATE,
   for the SET_MTIME argument.

* subversion/libsvn_subr/stream.c
  (svn_stream__install_finalize): Implement this new function.
   On Windows, either explicitly set the new timestamp, or suspend
   updates for timestamp values for all subsequent I/O operations.
   Always ask for the new timestamp value from the filesystem to
   properly handle a case where the filesystem has lower timestamp
   granularity than apr_time_t.
  (svn_stream__install_stream,
   svn_stream__install_delete): Handle a case where the file handle has
   been already closed.  That can now happen, for example, if the call to
   finalize() resulted in the fallback due to being unable to set file
   information by handle.

* subversion/libsvn_wc/working_file_writer.h
  (svn_wc__working_file_writer_get_info): Remove.
  (svn_wc__working_file_writer_finalize): New.
  (svn_wc__working_file_writer_stream): Tweak docstring.

* subversion/libsvn_wc/working_file_writer.c
  (svn_wc__working_file_writer_get_info): Remove.
  (svn_wc__working_file_writer_finalize): Implement this new function.
   Forward to the finalize() function for the install stream.

* subversion/libsvn_wc/wc_db.c
  (install_working_file): Finalize the working file and get its
   timestamp and size before installing it.

* subversion/libsvn_wc/wc_db_pristine.c
  (pristine_install_txn): Finalize the working file and get its size
   before installing it.

* subversion/libsvn_wc/workqueue.c
  (run_file_install): Finalize the working file before installing it.

* subversion/tests/libsvn_subr/io-test.c
  (test_install_stream_to_longpath,
   test_install_stream_over_readonly_file,
   test_install_stream_set_read_only,
   test_install_stream_set_affected_time,
   test_install_stream): Finalize the working file before installing it.
  (test_install_stream_delete,
   test_install_stream_delete_after_finalize): New tests.
  (test_funcs): Run the new tests.

Modified:
    subversion/trunk/subversion/include/private/svn_io_private.h
    subversion/trunk/subversion/libsvn_subr/io.c
    subversion/trunk/subversion/libsvn_subr/stream.c
    subversion/trunk/subversion/libsvn_wc/wc_db.c
    subversion/trunk/subversion/libsvn_wc/wc_db_pristine.c
    subversion/trunk/subversion/libsvn_wc/working_file_writer.c
    subversion/trunk/subversion/libsvn_wc/working_file_writer.h
    subversion/trunk/subversion/libsvn_wc/workqueue.c
    subversion/trunk/subversion/tests/libsvn_subr/io-test.c

Modified: subversion/trunk/subversion/include/private/svn_io_private.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_io_private.h?rev=1886774&r1=1886773&r2=1886774&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_io_private.h (original)
+++ subversion/trunk/subversion/include/private/svn_io_private.h Sun Feb 21 23:58:57 2021
@@ -120,8 +120,20 @@ void
 svn_stream__install_set_affected_time(svn_stream_t *install_stream,
                                       apr_time_t mtime);
 
+/* 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. The returned properties are guaranteed to be preserved
+   after the stream is installed. MTIME_P and SIZE_P both may be NULL.*/
+svn_error_t *
+svn_stream__install_finalize(apr_time_t *mtime_p,
+                             apr_off_t *size_p,
+                             svn_stream_t *install_stream,
+                             apr_pool_t *scratch_pool);
+
 /* Installs a stream created with svn_stream__create_for_install in its final
    location FINAL_ABSPATH, potentially using platform specific optimizations.
+   If the stream has not been finalized with svn_stream__install_finalize(),
+   the behavior is undefined.
 
    If MAKE_PARENTS is TRUE, this function will create missing parent
    directories if needed.
@@ -137,14 +149,6 @@ svn_error_t *
 svn_stream__install_delete(svn_stream_t *install_stream,
                            apr_pool_t *scratch_pool);
 
-/* Retrieves file information for the specified install stream.
-   MTIME_P and SIZE_P both may be NULL to allow for partial queries. */
-svn_error_t *
-svn_stream__install_get_info(apr_time_t *mtime_p,
-                             apr_off_t *size_p,
-                             svn_stream_t *install_stream,
-                             apr_pool_t *scratch_pool);
-
 /* Internal version of svn_stream_from_aprfile2() supporting the
    additional TRUNCATE_ON_SEEK argument. */
 svn_stream_t *
@@ -190,11 +194,29 @@ svn_io__win_rename_open_file(apr_file_t
                              const char *to_path,
                              apr_pool_t *pool);
 
+/* Special value that indicates that the file system should not update
+   timestamp values such as LastAccessTime, LastWriteTime, and ChangeTime
+   during this I/O operation.
+   Corresponds to the predefined value of "0" in:
+   https://docs.microsoft.com/windows-hardware/drivers/ddi/wdm/ns-wdm-_file_basic_information#remarks
+ */
+#define SVN_IO__WIN_TIME_UNCHANGED APR_INT64_MIN + 0
+
+/* Special value that indicates that the file system should suspend updates
+   for timestamp values such as LastAccessTime, LastWriteTime, and ChangeTime
+   during subsequent I/O operations on the file handle.
+   Corresponds to the predefined value of "-1" in:
+   https://docs.microsoft.com/windows-hardware/drivers/ddi/wdm/ns-wdm-_file_basic_information#remarks
+ */
+#define SVN_IO__WIN_TIME_SUSPEND_UPDATE APR_INT64_MIN + 1
+
 /* This Windows-specific function sets the basic file information using an
-   existing file handle. If SET_MTIME is non-negative, it will be set as
-   the new LastWriteTime value. If SET_READ_ONLY is non-zero, the file
-   attributes will be updated to include FILE_ATTRIBUTE_READONLY.
-   Return SVN_ERR_UNSUPPORTED_FEATURE if not supported by OS. */
+   existing file handle. The SET_MTIME will be set as the new LastWriteTime
+   value, unless it is equal to one of the predefined special values such
+   as SVN_IO__WIN_TIME_UNCHANGED or SVN_IO__WIN_TIME_SUSPEND_UPDATE.
+   If SET_READ_ONLY is non-zero, the file attributes will be updated to
+   include FILE_ATTRIBUTE_READONLY. Return SVN_ERR_UNSUPPORTED_FEATURE
+   if not supported by OS. */
 svn_error_t *
 svn_io__win_set_file_basic_info(apr_file_t *file,
                                 const char *path,

Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1886774&r1=1886773&r2=1886774&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Sun Feb 21 23:58:57 2021
@@ -2365,10 +2365,31 @@ svn_io__win_set_file_basic_info(apr_file
   info.LastAccessTime.QuadPart = 0;
   info.ChangeTime.QuadPart = 0;
 
-  if (set_mtime >= 0)
-    info.LastWriteTime.QuadPart = (set_mtime + APR_DELTA_EPOCH_IN_USEC) * 10;
+  if (set_mtime == SVN_IO__WIN_TIME_UNCHANGED)
+    {
+      /* If you specify a value of zero for any of the XxxTime members of the
+         FILE_BASIC_INFORMATION structure, the ZwSetInformationFile function
+         keeps a file's current setting for that time.
+         https://docs.microsoft.com/windows-hardware/drivers/ddi/wdm/ns-wdm-_file_basic_information#remarks
+       */
+      info.LastWriteTime.QuadPart = 0;
+    }
+  else if (set_mtime == SVN_IO__WIN_TIME_SUSPEND_UPDATE)
+    {
+      /* File system updates the values of the LastAccessTime, LastWriteTime,
+         and ChangeTime members as appropriate after an I/O operation is
+         performed on a file. A driver or application can request that the
+         file system not update one or more of these members for I/O operations
+         that are performed on the caller's file handle by setting the
+         appropriate members to -1.
+         https://docs.microsoft.com/windows-hardware/drivers/ddi/wdm/ns-wdm-_file_basic_information#remarks
+       */
+      info.LastWriteTime.QuadPart = -1;
+    }
   else
-    info.LastWriteTime.QuadPart = 0;
+    {
+      info.LastWriteTime.QuadPart = (set_mtime + APR_DELTA_EPOCH_IN_USEC) * 10;
+    }
 
   if (set_read_only)
     info.FileAttributes |= FILE_ATTRIBUTE_READONLY;

Modified: subversion/trunk/subversion/libsvn_subr/stream.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/stream.c?rev=1886774&r1=1886773&r2=1886774&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/stream.c (original)
+++ subversion/trunk/subversion/libsvn_subr/stream.c Sun Feb 21 23:58:57 2021
@@ -2354,6 +2354,137 @@ svn_stream__install_set_affected_time(sv
 }
 
 svn_error_t *
+svn_stream__install_finalize(apr_time_t *mtime_p,
+                             apr_off_t *size_p,
+                             svn_stream_t *install_stream,
+                             apr_pool_t *scratch_pool)
+{
+  struct install_baton_t *ib = install_stream->baton;
+  svn_boolean_t finalized = FALSE;
+  apr_finfo_t finfo;
+  apr_int32_t wanted;
+
+#ifdef WIN32
+  /* If the caller asked us for the timestamp with a non-null MTIME_P,
+     ensure that subsequent I/O operations won't change it; see below.
+   */
+  if (ib->set_mtime >= 0 || ib->set_read_only || mtime_p)
+    {
+      apr_time_t set_mtime;
+      svn_error_t *err;
+
+      /* On Windows, the file systems may defer processing of timestamps until
+         the file handle is closed, as specified in [1].  Since we peek and
+         return the current timestamp, we MUST ensure that the timestamp does
+         not change after the call to finalize().
+
+         Luckily, there are two options that guarantee that the file will keep
+         its current timestamp after close.  We can either explicitly set a new
+         timestamp, or use a special option that instructs the file system to
+         suspend updates for timestamp values for all subsequent I/O operations.
+         Both of these options guarantee [2, 3] that no other operation will
+         change the final timestamp.  So we use both of them, depending on
+         whether the caller wants us to set a specific timestamp, or not.
+
+         [1: MS-FSA, 2.1.4.17, Note <42>]
+         File systems may choose to defer processing for a file that has been
+         modified to a later time, favoring performance over accuracy. The NTFS
+         file system on versions prior to Windows 10 v1809 operating system,
+         Windows Server v1809 operating system, and Windows Server 2019, and
+         non-NTFS file systems on all versions of Windows, defer this processing
+         until the Open gets closed.
+         https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-fsa/4e3695bd-7574-4f24-a223-b4679c065b63#Appendix_A_42
+
+         [2: MS-FSA 2.1.5.14.2]
+         If InputBuffer.LastWriteTime != 0:
+           If InputBuffer.LastWriteTime != -2:
+             The object store MUST set Open.UserSetModificationTime to TRUE.
+         https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-fsa/a36513b4-73c8-4888-ad29-8f3a196567e8
+
+         [3: MS-FSA 2.1.4.17]
+         If Open.UserSetModificationTime is FALSE, set Open.File.LastModificationTime
+         to the current system time.
+         https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-fsa/75cdaba1-4401-4c53-b09c-69ba6cd50ce6
+       */
+      if (ib->set_mtime >= 0)
+        set_mtime = ib->set_mtime;
+      else
+        set_mtime = SVN_IO__WIN_TIME_SUSPEND_UPDATE;
+
+      err = svn_io__win_set_file_basic_info(ib->baton_apr.file, ib->tmp_path,
+                                            set_mtime, ib->set_read_only,
+                                            scratch_pool);
+
+      if (err && err->apr_err == SVN_ERR_UNSUPPORTED_FEATURE)
+        {
+          /* Setting information by handle is not supported on this platform:
+             fallback to setting it by path. */
+          svn_error_clear(err);
+        }
+      else if (err)
+        {
+          return svn_error_trace(err);
+        }
+      else
+        {
+          finalized = TRUE;
+        }
+    }
+  else
+    {
+      finalized = TRUE;
+    }
+#endif
+
+  if (!finalized)
+    {
+      SVN_ERR(svn_io_file_close(ib->baton_apr.file, scratch_pool));
+      ib->baton_apr.file = NULL;
+
+      if (ib->set_read_only)
+        SVN_ERR(svn_io_set_file_read_only(ib->tmp_path, FALSE,
+                                          scratch_pool));
+      if (ib->set_executable)
+        SVN_ERR(svn_io_set_file_executable(ib->tmp_path, TRUE, FALSE,
+                                           scratch_pool));
+      if (ib->set_mtime >= 0)
+        SVN_ERR(svn_io_set_file_affected_time(ib->set_mtime, ib->tmp_path,
+                                              scratch_pool));
+
+      finalized = TRUE;
+    }
+
+  wanted = 0;
+  if (mtime_p)
+    wanted |= APR_FINFO_MTIME;
+  if (size_p)
+    wanted |= APR_FINFO_SIZE;
+
+  /* Note that we always fetch the values such as MTIME_P from the filesystem,
+     because it might have a lower timestamp granularity than apr_time_t.
+   */
+  if (wanted)
+    {
+      apr_status_t status;
+
+      if (ib->baton_apr.file)
+        status = apr_file_info_get(&finfo, wanted, ib->baton_apr.file);
+      else
+        status = apr_stat(&finfo, ib->tmp_path, wanted, scratch_pool);
+
+      if (status)
+        return svn_error_wrap_apr(status, NULL);
+    }
+
+  if (mtime_p)
+    *mtime_p = finfo.mtime;
+  if (size_p)
+    *size_p = finfo.size;
+
+  return SVN_NO_ERROR;
+}
+
+svn_error_t *
 svn_stream__install_stream(svn_stream_t *install_stream,
                            const char *final_abspath,
                            svn_boolean_t make_parents,
@@ -2363,21 +2494,10 @@ svn_stream__install_stream(svn_stream_t
   svn_error_t *err;
 
   SVN_ERR_ASSERT(svn_dirent_is_absolute(final_abspath));
-#ifdef WIN32
-  err = SVN_NO_ERROR;
 
-  /* Windows doesn't have an executable bit. */
-  if (ib->set_mtime >= 0 || ib->set_read_only)
-    {
-      err = svn_io__win_set_file_basic_info(ib->baton_apr.file,
-                                            ib->tmp_path,
-                                            ib->set_mtime,
-                                            ib->set_read_only,
-                                            scratch_pool);
-    }
-
-  if (!err)
+  if (ib->baton_apr.file)
     {
+#ifdef WIN32
       err = svn_io__win_rename_open_file(ib->baton_apr.file, ib->tmp_path,
                                          final_abspath, scratch_pool);
       if (make_parents && err && APR_STATUS_IS_ENOENT(err->apr_err))
@@ -2396,36 +2516,28 @@ svn_stream__install_stream(svn_stream_t
           err = svn_io__win_rename_open_file(ib->baton_apr.file, ib->tmp_path,
                                              final_abspath, scratch_pool);
         }
-    }
 
-  /* ### rhuijben: I wouldn't be surprised if we later find out that we
-                   have to fall back to close+rename on some specific
-                   error values here, to support some non standard NAS
-                   and filesystem scenarios. */
-  if (err && err->apr_err == SVN_ERR_UNSUPPORTED_FEATURE)
-    {
-      /* Rename open files is not supported on this platform: fallback to
-         svn_io_file_rename2(). */
-      svn_error_clear(err);
-      err = SVN_NO_ERROR;
-    }
-  else
-    {
-      return svn_error_compose_create(err,
-                                      svn_io_file_close(ib->baton_apr.file,
-                                                        scratch_pool));
-    }
+        /* ### rhuijben: I wouldn't be surprised if we later find out that we
+                         have to fall back to close+rename on some specific
+                         error values here, to support some non standard NAS
+                         and filesystem scenarios. */
+        if (err && err->apr_err == SVN_ERR_UNSUPPORTED_FEATURE)
+          {
+            /* Rename open files is not supported on this platform: fallback to
+               svn_io_file_rename2(). */
+            svn_error_clear(err);
+            err = SVN_NO_ERROR;
+          }
+        else
+          {
+            return svn_error_compose_create(err,
+                                            svn_io_file_close(ib->baton_apr.file,
+                                                              scratch_pool));
+          }
 #endif
 
-  /* Close temporary file. */
-  SVN_ERR(svn_io_file_close(ib->baton_apr.file, scratch_pool));
-
-  if (ib->set_read_only)
-    SVN_ERR(svn_io_set_file_read_only(ib->tmp_path, FALSE, scratch_pool));
-  if (ib->set_executable)
-    SVN_ERR(svn_io_set_file_executable(ib->tmp_path, TRUE, FALSE, scratch_pool));
-  if (ib->set_mtime >= 0)
-    SVN_ERR(svn_io_set_file_affected_time(ib->set_mtime, ib->tmp_path, scratch_pool));
+        SVN_ERR(svn_io_file_close(ib->baton_apr.file, scratch_pool));
+    }
 
   err = svn_io_file_rename2(ib->tmp_path, final_abspath, FALSE, scratch_pool);
 
@@ -2454,67 +2566,33 @@ svn_stream__install_stream(svn_stream_t
 }
 
 svn_error_t *
-svn_stream__install_get_info(apr_time_t *mtime_p,
-                             apr_off_t *size_p,
-                             svn_stream_t *install_stream,
-                             apr_pool_t *scratch_pool)
-{
-  struct install_baton_t *ib = install_stream->baton;
-  apr_status_t status;
-  apr_int32_t wanted;
-  apr_finfo_t finfo;
-
-  /* If we are instructed to set a specific timestamp, return it.
-     For everything else, stat the file to get the information. */
-  wanted = 0;
-  if (mtime_p && ib->set_mtime < 0)
-    wanted |= APR_FINFO_MTIME;
-  if (size_p)
-    wanted |= APR_FINFO_SIZE;
-
-  if (wanted)
-    {
-      status = apr_file_info_get(&finfo, wanted, ib->baton_apr.file);
-      if (status)
-        return svn_error_wrap_apr(status, NULL);
-    }
-
-  if (mtime_p && ib->set_mtime >= 0)
-    *mtime_p = ib->set_mtime;
-  else if (mtime_p)
-    *mtime_p = finfo.mtime;
-
-  if (size_p)
-    *size_p = finfo.size;
-
-  return SVN_NO_ERROR;
-}
-
-svn_error_t *
 svn_stream__install_delete(svn_stream_t *install_stream,
                            apr_pool_t *scratch_pool)
 {
   struct install_baton_t *ib = install_stream->baton;
 
+  if (ib->baton_apr.file)
+    {
 #ifdef WIN32
-  svn_error_t *err;
+      svn_error_t *err;
 
-  /* Mark the file as delete on close to avoid having to reopen
-     the file as part of the delete handling. */
-  err = svn_io__win_delete_file_on_close(ib->baton_apr.file,  ib->tmp_path,
-                                         scratch_pool);
-  if (err == SVN_NO_ERROR)
-    {
-      SVN_ERR(svn_io_file_close(ib->baton_apr.file, scratch_pool));
-      return SVN_NO_ERROR; /* File is already gone */
-    }
+      /* Mark the file as delete on close to avoid having to reopen
+         the file as part of the delete handling. */
+      err = svn_io__win_delete_file_on_close(ib->baton_apr.file, ib->tmp_path,
+                                             scratch_pool);
+      if (err == SVN_NO_ERROR)
+        {
+          SVN_ERR(svn_io_file_close(ib->baton_apr.file, scratch_pool));
+          return SVN_NO_ERROR; /* File is already gone */
+        }
 
-  /* Deleting file on close may be unsupported, so ignore errors and
-     fallback to svn_io_remove_file2(). */
-  svn_error_clear(err);
+      /* Deleting file on close may be unsupported, so ignore errors and
+         fallback to svn_io_remove_file2(). */
+      svn_error_clear(err);
 #endif
 
-  SVN_ERR(svn_io_file_close(ib->baton_apr.file, scratch_pool));
+      SVN_ERR(svn_io_file_close(ib->baton_apr.file, scratch_pool));
+    }
 
   return svn_error_trace(svn_io_remove_file2(ib->tmp_path, FALSE,
                                              scratch_pool));

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1886774&r1=1886773&r2=1886774&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Sun Feb 21 23:58:57 2021
@@ -1791,11 +1791,16 @@ install_working_file(svn_wc__db_wcroot_t
       apr_time_t mtime;
       apr_off_t size;
 
-      SVN_ERR(svn_wc__working_file_writer_get_info(&mtime, &size, file_writer,
+      SVN_ERR(svn_wc__working_file_writer_finalize(&mtime, &size, file_writer,
                                                    scratch_pool));
       SVN_ERR(db_record_fileinfo(wcroot, local_relpath, size, mtime,
                                  scratch_pool));
     }
+  else
+    {
+      SVN_ERR(svn_wc__working_file_writer_finalize(NULL, NULL, file_writer,
+                                                   scratch_pool));
+    }
 
   local_abspath = svn_dirent_join(wcroot->abspath, local_relpath, scratch_pool);
   SVN_ERR(svn_wc__working_file_writer_install(file_writer, local_abspath,

Modified: subversion/trunk/subversion/libsvn_wc/wc_db_pristine.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db_pristine.c?rev=1886774&r1=1886773&r2=1886774&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db_pristine.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db_pristine.c Sun Feb 21 23:58:57 2021
@@ -308,7 +308,7 @@ pristine_install_txn(svn_sqlite__db_t *s
         apr_finfo_t finfo;
         apr_off_t size;
 
-        SVN_ERR(svn_stream__install_get_info(NULL, &size, install_stream,
+        SVN_ERR(svn_stream__install_finalize(NULL, &size, install_stream,
                                              scratch_pool));
 
         SVN_ERR(svn_io_stat(&finfo, pristine_abspath, APR_FINFO_SIZE,
@@ -335,7 +335,7 @@ pristine_install_txn(svn_sqlite__db_t *s
   {
     apr_off_t size;
 
-    SVN_ERR(svn_stream__install_get_info(NULL, &size, install_stream,
+    SVN_ERR(svn_stream__install_finalize(NULL, &size, install_stream,
                                          scratch_pool));
     SVN_ERR(svn_stream__install_stream(install_stream, pristine_abspath,
                                        TRUE, scratch_pool));

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=1886774&r1=1886773&r2=1886774&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/working_file_writer.c (original)
+++ subversion/trunk/subversion/libsvn_wc/working_file_writer.c Sun Feb 21 23:58:57 2021
@@ -150,12 +150,12 @@ svn_wc__working_file_writer_get_stream(s
 }
 
 svn_error_t *
-svn_wc__working_file_writer_get_info(apr_time_t *mtime_p,
+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)
 {
-  SVN_ERR(svn_stream__install_get_info(mtime_p, size_p, writer->install_stream,
+  SVN_ERR(svn_stream__install_finalize(mtime_p, size_p, writer->install_stream,
                                        scratch_pool));
 
   return SVN_NO_ERROR;

Modified: subversion/trunk/subversion/libsvn_wc/working_file_writer.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/working_file_writer.h?rev=1886774&r1=1886773&r2=1886774&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/working_file_writer.h (original)
+++ subversion/trunk/subversion/libsvn_wc/working_file_writer.h Sun Feb 21 23:58:57 2021
@@ -61,15 +61,18 @@ svn_wc__working_file_writer_open(svn_wc_
 svn_stream_t *
 svn_wc__working_file_writer_get_stream(svn_wc__working_file_writer_t *writer);
 
-/* Retrieve file information for WRITER.
-   MTIME_P and SIZE_P both may be NULL to allow for partial queries. */
+/* 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_get_info(apr_time_t *mtime_p,
+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. */
+/* 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,

Modified: subversion/trunk/subversion/libsvn_wc/workqueue.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/workqueue.c?rev=1886774&r1=1886773&r2=1886774&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/workqueue.c (original)
+++ subversion/trunk/subversion/libsvn_wc/workqueue.c Sun Feb 21 23:58:57 2021
@@ -574,6 +574,8 @@ run_file_install(work_item_baton_t *wqb,
                            cancel_func, cancel_baton,
                            scratch_pool));
 
+  SVN_ERR(svn_wc__working_file_writer_finalize(NULL, NULL, file_writer,
+                                               scratch_pool));
   SVN_ERR(svn_wc__working_file_writer_install(file_writer, local_abspath,
                                               scratch_pool));
 

Modified: subversion/trunk/subversion/tests/libsvn_subr/io-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/io-test.c?rev=1886774&r1=1886773&r2=1886774&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_subr/io-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_subr/io-test.c Sun Feb 21 23:58:57 2021
@@ -997,7 +997,7 @@ test_install_stream_to_longpath(apr_pool
   SVN_ERR(svn_stream__create_for_install(&stream, deep_dir, pool, pool));
   SVN_ERR(svn_stream_puts(stream, "stream1 content"));
   SVN_ERR(svn_stream_close(stream));
-  SVN_ERR(svn_stream__install_get_info(&mtime, &size, stream, pool));
+  SVN_ERR(svn_stream__install_finalize(&mtime, &size, stream, pool));
   /* Ensure that we will notice a timestamp change, if it happens. */
   svn_io_sleep_for_timestamps(NULL, pool);
   SVN_ERR(svn_stream__install_stream(stream,
@@ -1048,7 +1048,7 @@ test_install_stream_over_readonly_file(a
   SVN_ERR(svn_stream__create_for_install(&stream, tmp_dir, pool, pool));
   SVN_ERR(svn_stream_puts(stream, "stream1 content"));
   SVN_ERR(svn_stream_close(stream));
-  SVN_ERR(svn_stream__install_get_info(&mtime, &size, stream, pool));
+  SVN_ERR(svn_stream__install_finalize(&mtime, &size, stream, pool));
   /* Ensure that we will notice a timestamp change, if it happens. */
   svn_io_sleep_for_timestamps(NULL, pool);
   SVN_ERR(svn_stream__install_stream(stream,
@@ -1096,7 +1096,7 @@ test_install_stream_set_read_only(apr_po
   SVN_ERR(svn_stream_puts(stream, "stream1 content"));
   SVN_ERR(svn_stream_close(stream));
   svn_stream__install_set_read_only(stream, TRUE);
-  SVN_ERR(svn_stream__install_get_info(&mtime, &size, stream, pool));
+  SVN_ERR(svn_stream__install_finalize(&mtime, &size, stream, pool));
   /* Ensure that we will notice a timestamp change, if it happens. */
   svn_io_sleep_for_timestamps(NULL, pool);
   SVN_ERR(svn_stream__install_stream(stream,
@@ -1150,7 +1150,7 @@ test_install_stream_set_affected_time(ap
                                 pool));
   svn_stream__install_set_affected_time(stream, expected_timestamp);
 
-  SVN_ERR(svn_stream__install_get_info(&mtime, &size, stream, pool));
+  SVN_ERR(svn_stream__install_finalize(&mtime, &size, stream, pool));
   /* Ensure that we will notice a timestamp change, if it happens. */
   svn_io_sleep_for_timestamps(NULL, pool);
   SVN_ERR(svn_stream__install_stream(stream,
@@ -1200,7 +1200,7 @@ test_install_stream(apr_pool_t *pool)
   SVN_ERR(svn_stream__create_for_install(&stream, tmp_dir, pool, pool));
   SVN_ERR(svn_stream_puts(stream, "stream1 content"));
   SVN_ERR(svn_stream_close(stream));
-  SVN_ERR(svn_stream__install_get_info(&mtime, &size, stream, pool));
+  SVN_ERR(svn_stream__install_finalize(&mtime, &size, stream, pool));
   /* Ensure that we will notice a timestamp change, if it happens. */
   svn_io_sleep_for_timestamps(NULL, pool);
   SVN_ERR(svn_stream__install_stream(stream,
@@ -1225,6 +1225,65 @@ test_install_stream(apr_pool_t *pool)
 }
 
 static svn_error_t *
+test_install_stream_delete(apr_pool_t *pool)
+{
+  const char *tmp_dir;
+  const char *final_abspath;
+  apr_pool_t *subpool;
+  svn_stream_t *stream;
+  apr_hash_t *dirents;
+
+  /* Create an empty directory. */
+  SVN_ERR(svn_test_make_sandbox_dir(&tmp_dir,
+                                    "test_install_stream_delete",
+                                    pool));
+
+  final_abspath = svn_dirent_join(tmp_dir, "stream1", pool);
+
+  subpool = svn_pool_create(pool);
+  SVN_ERR(svn_stream__create_for_install(&stream, tmp_dir, subpool, subpool));
+  SVN_ERR(svn_stream_puts(stream, "stream1 content"));
+  SVN_ERR(svn_stream_close(stream));
+  SVN_ERR(svn_stream__install_delete(stream, subpool));
+  svn_pool_destroy(subpool);
+
+  SVN_ERR(svn_io_get_dirents3(&dirents, tmp_dir, TRUE, pool, pool));
+  SVN_TEST_INT_ASSERT(apr_hash_count(dirents), 0);
+
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+test_install_stream_delete_after_finalize(apr_pool_t *pool)
+{
+  const char *tmp_dir;
+  const char *final_abspath;
+  apr_pool_t *subpool;
+  svn_stream_t *stream;
+  apr_hash_t *dirents;
+
+  /* Create an empty directory. */
+  SVN_ERR(svn_test_make_sandbox_dir(&tmp_dir,
+                                    "test_install_stream_delete_after_finalize",
+                                    pool));
+
+  final_abspath = svn_dirent_join(tmp_dir, "stream1", pool);
+
+  subpool = svn_pool_create(pool);
+  SVN_ERR(svn_stream__create_for_install(&stream, tmp_dir, subpool, subpool));
+  SVN_ERR(svn_stream_puts(stream, "stream1 content"));
+  SVN_ERR(svn_stream_close(stream));
+  SVN_ERR(svn_stream__install_finalize(NULL, NULL, stream, subpool));
+  SVN_ERR(svn_stream__install_delete(stream, subpool));
+  svn_pool_destroy(subpool);
+
+  SVN_ERR(svn_io_get_dirents3(&dirents, tmp_dir, TRUE, pool, pool));
+  SVN_TEST_INT_ASSERT(apr_hash_count(dirents), 0);
+
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
 test_file_size_get(apr_pool_t *pool)
 {
   const char *tmp_dir, *path;
@@ -1442,6 +1501,10 @@ static struct svn_test_descriptor_t test
                    "test svn_stream__install_set_affected_time"),
     SVN_TEST_PASS2(test_install_stream,
                    "test svn_stream__install_stream"),
+    SVN_TEST_PASS2(test_install_stream_delete,
+                   "test svn_stream__install_delete"),
+    SVN_TEST_PASS2(test_install_stream_delete_after_finalize,
+                   "test svn_stream__install_delete after finalize"),
     SVN_TEST_PASS2(test_file_size_get,
                    "test svn_io_file_size_get"),
     SVN_TEST_PASS2(test_file_rename2,