Author: svn-role
Date: Tue Apr 21 19:02:38 2026
New Revision: 1933215
Log:
Merge r1931549 from trunk:
* r1931549
Use bytewise content comparison in the "is the file modified?" working copy
checks if we have the pristine file content.
Justification:
Avoid changing the file comparison characteristics for working copies with
pristine contents.
Notes:
Discussed in https://lists.apache.org/thread/128rgf7ozn72ljfzn7o18gfhb9l43y26
Votes:
+1: kotkov, rinrab, ivan
Modified:
subversion/branches/1.15.x/ (props changed)
subversion/branches/1.15.x/STATUS
subversion/branches/1.15.x/subversion/libsvn_wc/questions.c
Modified: subversion/branches/1.15.x/STATUS
==============================================================================
--- subversion/branches/1.15.x/STATUS Tue Apr 21 19:00:00 2026 (r1933214)
+++ subversion/branches/1.15.x/STATUS Tue Apr 21 19:02:38 2026 (r1933215)
@@ -57,14 +57,3 @@ Veto-blocked changes:
Approved changes:
=================
-
- * r1931549
- Use bytewise content comparison in the "is the file modified?" working copy
- checks if we have the pristine file content.
- Justification:
- Avoid changing the file comparison characteristics for working copies with
- pristine contents.
- Notes:
- Discussed in https://lists.apache.org/thread/128rgf7ozn72ljfzn7o18gfhb9l43y26
- Votes:
- +1: kotkov, rinrab, ivan
Modified: subversion/branches/1.15.x/subversion/libsvn_wc/questions.c
==============================================================================
--- subversion/branches/1.15.x/subversion/libsvn_wc/questions.c Tue Apr 21 19:00:00 2026 (r1933214)
+++ subversion/branches/1.15.x/subversion/libsvn_wc/questions.c Tue Apr 21 19:02:38 2026 (r1933215)
@@ -77,14 +77,99 @@
mark it for removal?
*/
+static svn_error_t *
+compare_exact(svn_boolean_t *modified_p,
+ svn_stream_t *stream,
+ const char *eol_str,
+ apr_hash_t *keywords,
+ const svn_checksum_t *pristine_checksum,
+ svn_stream_t *pristine_stream,
+ const char *pristine_eol_str,
+ apr_pool_t *scratch_pool)
+{
+ svn_boolean_t same;
+
+ /* For exact comparison, we check that contents remain the same when
+ retranslated according to file properties. */
+
+ if (pristine_stream)
+ {
+ /* We have pristine contents: wrap a stream to translate it into working
+ copy form, and check that contents remain the same. */
+
+ pristine_stream = svn_subst_stream_translated(pristine_stream,
+ eol_str, FALSE,
+ keywords, TRUE,
+ scratch_pool);
+
+ SVN_ERR(svn_stream_contents_same2(&same, pristine_stream, stream,
+ scratch_pool));
+ }
+ else
+ {
+ svn_checksum_t *working_checksum;
+ svn_checksum_t *detranslated_checksum;
+ svn_checksum_t *retranslated_checksum;
+
+ /* We don't have pristine contents. To make the comparison work without
+ it, let's check for two things:
+
+ 1) That the checksum of the detranslated contents matches the recorded
+ pristine checksum, as in the case of a non-exact comparison, ...
+
+ 2) ...and additionally, that the contents of the working file does not
+ change when retranslated according to its properties.
+
+ Technically we're going to do that with a single read of the file
+ contents, while checksumming it's original, detranslated and
+ retranslated versions.
+ */
+
+ stream = svn_stream_checksummed2(stream,
+ &working_checksum, NULL,
+ pristine_checksum->kind, TRUE,
+ scratch_pool);
+
+ stream = svn_subst_stream_translated(stream,
+ pristine_eol_str, TRUE,
+ keywords, FALSE,
+ scratch_pool);
+ stream = svn_stream_checksummed2(stream,
+ &detranslated_checksum, NULL,
+ pristine_checksum->kind, TRUE,
+ scratch_pool);
+
+ stream = svn_subst_stream_translated(stream, eol_str, FALSE,
+ keywords, TRUE,
+ scratch_pool);
+ stream = svn_stream_checksummed2(stream,
+ &retranslated_checksum, NULL,
+ pristine_checksum->kind, TRUE,
+ scratch_pool);
+
+ SVN_ERR(svn_stream_copy3(stream, svn_stream_empty(scratch_pool),
+ NULL, NULL, scratch_pool));
+
+ same = svn_checksum_match(detranslated_checksum, pristine_checksum) &&
+ svn_checksum_match(working_checksum, retranslated_checksum);
+ }
+
+ *modified_p = !same;
+ return SVN_NO_ERROR;
+}
/* Set *MODIFIED_P to TRUE if (after translation) VERSIONED_FILE_ABSPATH
- * (of VERSIONED_FILE_SIZE bytes) differs from pristine file with checksum
- * PRISTINE_CHECKSUM, else to FALSE if not.
+ * (of VERSIONED_FILE_SIZE bytes) differs from PRISTINE_STREAM (of
+ * PRISTINE_SIZE bytes with PRISTINE_CHECKSUM), else to FALSE if not.
+ *
+ * If PRISTINE_STREAM is NULL, perform checksum-based content comparison.
+ * If PRISTINE_STREAM is not NULL, perform bytewise content comparison
+ * against the provided stream. PRISTINE_STREAM will be closed before
+ * a successful return.
*
* If EXACT_COMPARISON is FALSE, translate VERSIONED_FILE_ABSPATH's EOL
* style and keywords to repository-normal form according to its properties,
- * calculate checksum and compare the result with PRISTINE_CHECKSUM.
+ * and compare the result with pristine contents.
* If EXACT_COMPARISON is TRUE, also check that VERSIONED_FILE_ABSPATH
* contents remains the same when retranslated according to its properties.
*
@@ -94,6 +179,7 @@
* PROPS_MOD should be TRUE if the file's properties have been changed,
* otherwise FALSE.
*
+ *
* DB is a wc_db; use SCRATCH_POOL for temporary allocation.
*/
static svn_error_t *
@@ -101,6 +187,8 @@ 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,
@@ -113,8 +201,7 @@ compare_and_verify(svn_boolean_t *modifi
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_boolean_t same;
SVN_ERR_ASSERT(svn_dirent_is_absolute(versioned_file_abspath));
@@ -140,20 +227,15 @@ compare_and_verify(svn_boolean_t *modifi
else
need_translation = FALSE;
- if (! need_translation)
+ /* Easy out check: different sizes with no translation mean the
+ * file was modified. */
+ if (!need_translation && versioned_file_size != pristine_size)
{
- 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;
+ if (pristine_stream)
+ SVN_ERR(svn_stream_close(pristine_stream));
- return SVN_NO_ERROR;
- }
+ *modified_p = TRUE;
+ return SVN_NO_ERROR;
}
/* ### Other checks possible? */
@@ -169,13 +251,8 @@ 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;
- 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);
+ SVN_ERR(svn_io_file_open(&file, versioned_file_abspath, APR_READ,
+ APR_OS_DEFAULT, scratch_pool));
v_stream = svn_stream_from_aprfile2(file, FALSE, scratch_pool);
if (need_translation)
@@ -188,79 +265,41 @@ compare_and_verify(svn_boolean_t *modifi
pristine_eol_str = eol_str;
if (exact_comparison)
- {
- svn_checksum_t *working_checksum;
- svn_checksum_t *detranslated_checksum;
- svn_checksum_t *retranslated_checksum;
-
- v_stream = svn_stream_checksummed2(v_stream,
- &working_checksum, NULL,
- pristine_checksum->kind, TRUE,
- scratch_pool);
-
- v_stream = svn_subst_stream_translated(v_stream,
- pristine_eol_str, TRUE,
- keywords, FALSE,
- scratch_pool);
- v_stream = svn_stream_checksummed2(v_stream,
- &detranslated_checksum, NULL,
- pristine_checksum->kind, TRUE,
- scratch_pool);
-
- v_stream = svn_subst_stream_translated(v_stream, eol_str, FALSE,
- keywords, TRUE,
- scratch_pool);
- v_stream = svn_stream_checksummed2(v_stream,
- &retranslated_checksum, NULL,
- pristine_checksum->kind, TRUE,
+ return svn_error_trace(compare_exact(modified_p,
+ v_stream, eol_str, keywords,
+ pristine_checksum,
+ pristine_stream,
+ pristine_eol_str,
+ scratch_pool));
+
+ /* Wrap file stream to detranslate into normal form,
+ * "repairing" the EOL style if it is inconsistent. */
+ v_stream = svn_subst_stream_translated(v_stream,
+ pristine_eol_str,
+ TRUE /* repair */,
+ keywords,
+ FALSE /* expand */,
scratch_pool);
-
- err = svn_stream_copy3(v_stream, svn_stream_empty(scratch_pool),
- NULL, NULL, 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);
-
- if (svn_checksum_match(detranslated_checksum, pristine_checksum) &&
- svn_checksum_match(working_checksum, retranslated_checksum))
- {
- *modified_p = FALSE;
- }
- else
- {
- *modified_p = TRUE;
- }
-
- return SVN_NO_ERROR;
- }
- else
- {
- /* Wrap file stream to detranslate into normal form,
- * "repairing" the EOL style if it is inconsistent. */
- v_stream = svn_subst_stream_translated(v_stream,
- pristine_eol_str,
- TRUE /* repair */,
- keywords,
- FALSE /* expand */,
- 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);
+ if (pristine_stream)
+ {
+ SVN_ERR(svn_stream_contents_same2(&same, pristine_stream, v_stream,
+ scratch_pool));
+ }
else
- SVN_ERR(err);
+ {
+ svn_checksum_t *v_checksum;
- *modified_p = (! svn_checksum_match(v_checksum, pristine_checksum));
+ SVN_ERR(svn_stream_contents_checksum(&v_checksum, v_stream,
+ pristine_checksum->kind,
+ scratch_pool,
+ scratch_pool));
+ same = svn_checksum_match(v_checksum, pristine_checksum);
+ }
+ *modified_p = !same;
return SVN_NO_ERROR;
}
@@ -279,6 +318,9 @@ svn_wc__internal_file_modified_p(svn_boo
svn_boolean_t has_props;
svn_boolean_t props_mod;
const svn_io_dirent2_t *dirent;
+ svn_stream_t *pristine_stream;
+ svn_filesize_t pristine_size;
+ svn_error_t *err;
/* Read the relevant info */
SVN_ERR(svn_wc__db_read_info(&status, &kind, NULL, NULL, NULL, NULL, NULL,
@@ -358,12 +400,23 @@ svn_wc__internal_file_modified_p(svn_boo
}
compare_them:
- /* Check all bytes, and verify checksum if requested. */
- SVN_ERR(compare_and_verify(modified_p, db,
- local_abspath, dirent->filesize,
- checksum, has_props, props_mod,
- exact_comparison,
- scratch_pool));
+ SVN_ERR(svn_wc__db_pristine_read(&pristine_stream, &pristine_size,
+ db, local_abspath, checksum,
+ scratch_pool, scratch_pool));
+
+ err = compare_and_verify(modified_p, db,
+ local_abspath, dirent->filesize,
+ pristine_stream, pristine_size,
+ 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);
if (!*modified_p)
{
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.