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/08/27 13:51:53 UTC

svn commit: r1892645 - /subversion/branches/pristines-on-demand/subversion/libsvn_wc/questions.c

Author: kotkov
Date: Fri Aug 27 13:51:53 2021
New Revision: 1892645

URL: http://svn.apache.org/viewvc?rev=1892645&view=rev
Log:
On the pristines-on-demand branch: Revert r1801448 ("Temporarily
revert r1759233 …").

This effectively reinstates the changes made in r1759233.

The r1759233 commit started comparing checksums in the implementation of
the svn_wc__internal_file_modified_p() function.  More specifically, it
changed the approach where we simultaneously read the pristine and
working files into calculating the checksum of the working file and
comparing it against the checksum recorded in wc.db.

We can't get too far away in the attempt to make the pristine contents
optional if we still need it to understand if the file has been modified.
So let's untangle that first by switching to a checksum comparison.

* subversion/libsvn_wc/questions.c
  (compare_and_verify): Replace PRISTINE_STREAM and PRISTINE_SIZE arguments
   with PRISTINE_CHECKSUM. Calculate SHA-1 checksum of detranslated
   (normalized) content and compare it with what's stored in wc.db. Open
   pristine file only if exact comparison is requested.
  (svn_wc__internal_file_modified_p): Do not open pristine file here, just
   pass pristine's checksum to compare_and_verify().

Modified:
    subversion/branches/pristines-on-demand/subversion/libsvn_wc/questions.c

Modified: subversion/branches/pristines-on-demand/subversion/libsvn_wc/questions.c
URL: http://svn.apache.org/viewvc/subversion/branches/pristines-on-demand/subversion/libsvn_wc/questions.c?rev=1892645&r1=1892644&r2=1892645&view=diff
==============================================================================
--- subversion/branches/pristines-on-demand/subversion/libsvn_wc/questions.c (original)
+++ subversion/branches/pristines-on-demand/subversion/libsvn_wc/questions.c Fri Aug 27 13:51:53 2021
@@ -79,15 +79,15 @@
 
 
 /* Set *MODIFIED_P to TRUE if (after translation) VERSIONED_FILE_ABSPATH
- * (of VERSIONED_FILE_SIZE bytes) differs from PRISTINE_STREAM (of
- * PRISTINE_SIZE bytes), else to FALSE if not.
+ * (of VERSIONED_FILE_SIZE bytes) differs from pristine file with checksum
+ * PRISTINE_CHECKSUM, else to FALSE if not.
  *
  * If EXACT_COMPARISON is FALSE, translate VERSIONED_FILE_ABSPATH's EOL
  * style and keywords to repository-normal form according to its properties,
- * and compare the result with PRISTINE_STREAM.  If EXACT_COMPARISON is
- * TRUE, translate PRISTINE_STREAM's EOL style and keywords to working-copy
- * form according to VERSIONED_FILE_ABSPATH's properties, and compare the
- * result with VERSIONED_FILE_ABSPATH.
+ * calculate checksum and compare the result with PRISTINE_STREAM.  If
+ * EXACT_COMPARISON is TRUE, open pristine, translate it's EOL style and
+ * keywords to working-copy form according to VERSIONED_FILE_ABSPATH's
+ * properties, and compare the result with VERSIONED_FILE_ABSPATH.
  *
  * HAS_PROPS should be TRUE if the file had properties when it was not
  * modified, otherwise FALSE.
@@ -95,8 +95,6 @@
  * PROPS_MOD should be TRUE if the file's properties have been changed,
  * otherwise FALSE.
  *
- * PRISTINE_STREAM will be closed before a successful return.
- *
  * DB is a wc_db; use SCRATCH_POOL for temporary allocation.
  */
 static svn_error_t *
@@ -104,20 +102,20 @@ compare_and_verify(svn_boolean_t *modifi
                    svn_wc__db_t *db,
                    const char *versioned_file_abspath,
                    svn_filesize_t versioned_file_size,
-                   svn_stream_t *pristine_stream,
-                   svn_filesize_t pristine_size,
+                   const svn_checksum_t *pristine_checksum,
                    svn_boolean_t has_props,
                    svn_boolean_t props_mod,
                    svn_boolean_t exact_comparison,
                    apr_pool_t *scratch_pool)
 {
-  svn_boolean_t same;
   svn_subst_eol_style_t eol_style;
   const char *eol_str;
   apr_hash_t *keywords;
   svn_boolean_t special = FALSE;
   svn_boolean_t need_translation;
   svn_stream_t *v_stream; /* versioned_file */
+  svn_checksum_t *v_checksum;
+  svn_error_t *err;
 
   SVN_ERR_ASSERT(svn_dirent_is_absolute(versioned_file_abspath));
 
@@ -140,13 +138,20 @@ compare_and_verify(svn_boolean_t *modifi
   else
     need_translation = FALSE;
 
-  if (! need_translation
-      && (versioned_file_size != pristine_size))
+  if (! need_translation)
     {
-      *modified_p = TRUE;
+      svn_filesize_t pristine_size;
+
+      SVN_ERR(svn_wc__db_pristine_read(NULL, &pristine_size, db,
+                                       versioned_file_abspath, pristine_checksum,
+                                       scratch_pool, scratch_pool));
+
+      if (versioned_file_size != pristine_size)
+        {
+          *modified_p = TRUE;
 
-      /* ### Why did we open the pristine? */
-      return svn_error_trace(svn_stream_close(pristine_stream));
+          return SVN_NO_ERROR;
+        }
     }
 
   /* ### Other checks possible? */
@@ -162,8 +167,13 @@ compare_and_verify(svn_boolean_t *modifi
       /* We don't use APR-level buffering because the comparison function
        * will do its own buffering. */
       apr_file_t *file;
-      SVN_ERR(svn_io_file_open(&file, versioned_file_abspath, APR_READ,
-                               APR_OS_DEFAULT, scratch_pool));
+      err = svn_io_file_open(&file, versioned_file_abspath, APR_READ,
+                             APR_OS_DEFAULT, scratch_pool);
+      /* Convert EACCESS on working copy path to WC specific error code. */
+      if (err && APR_STATUS_IS_EACCES(err->apr_err))
+        return svn_error_create(SVN_ERR_WC_PATH_ACCESS_DENIED, err, NULL);
+      else
+        SVN_ERR(err);
       v_stream = svn_stream_from_aprfile2(file, FALSE, scratch_pool);
 
       if (need_translation)
@@ -188,20 +198,38 @@ compare_and_verify(svn_boolean_t *modifi
             }
           else
             {
+              svn_boolean_t same;
+              svn_stream_t *pristine_stream;
+
+              SVN_ERR(svn_wc__db_pristine_read(&pristine_stream, NULL,
+                                               db, versioned_file_abspath,
+                                               pristine_checksum,
+                                               scratch_pool, scratch_pool));
               /* Wrap base stream to translate into working copy form, and
                * arrange to throw an error if its EOL style is inconsistent. */
               pristine_stream = svn_subst_stream_translated(pristine_stream,
                                                             eol_str, FALSE,
                                                             keywords, TRUE,
                                                             scratch_pool);
+              SVN_ERR(svn_stream_contents_same2(&same, pristine_stream, v_stream,
+                                                scratch_pool));
+              *modified_p = (! same);
+              return SVN_NO_ERROR;
             }
         }
     }
 
-  SVN_ERR(svn_stream_contents_same2(&same, pristine_stream, v_stream,
-                                    scratch_pool));
+  /* Get checksum of detranslated (normalized) content. */
+  err = svn_stream_contents_checksum(&v_checksum, v_stream,
+                                     pristine_checksum->kind,
+                                     scratch_pool, scratch_pool);
+  /* Convert EACCESS on working copy path to WC specific error code. */
+  if (err && APR_STATUS_IS_EACCES(err->apr_err))
+    return svn_error_create(SVN_ERR_WC_PATH_ACCESS_DENIED, err, NULL);
+  else
+    SVN_ERR(err);
 
-  *modified_p = (! same);
+  *modified_p = (! svn_checksum_match(v_checksum, pristine_checksum));
 
   return SVN_NO_ERROR;
 }
@@ -213,8 +241,6 @@ svn_wc__internal_file_modified_p(svn_boo
                                  svn_boolean_t exact_comparison,
                                  apr_pool_t *scratch_pool)
 {
-  svn_stream_t *pristine_stream;
-  svn_filesize_t pristine_size;
   svn_wc__db_status_t status;
   svn_node_kind_t kind;
   const svn_checksum_t *checksum;
@@ -302,27 +328,12 @@ svn_wc__internal_file_modified_p(svn_boo
     }
 
  compare_them:
-  SVN_ERR(svn_wc__db_pristine_read(&pristine_stream, &pristine_size,
-                                   db, local_abspath, checksum,
-                                   scratch_pool, scratch_pool));
-
   /* Check all bytes, and verify checksum if requested. */
-  {
-    svn_error_t *err;
-    err = compare_and_verify(modified_p, db,
+  SVN_ERR(compare_and_verify(modified_p, db,
                              local_abspath, dirent->filesize,
-                             pristine_stream, pristine_size,
-                             has_props, props_mod,
+                             checksum, has_props, props_mod,
                              exact_comparison,
-                             scratch_pool);
-
-    /* At this point we already opened the pristine file, so we know that
-       the access denied applies to the working copy path */
-    if (err && APR_STATUS_IS_EACCES(err->apr_err))
-      return svn_error_create(SVN_ERR_WC_PATH_ACCESS_DENIED, err, NULL);
-    else
-      SVN_ERR(err);
-  }
+                             scratch_pool));
 
   if (!*modified_p)
     {