Skip to content

Commit 73aeca5

Browse files
author
svn-role
committed
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 git-svn-id: https://svn.apache.org/repos/asf/subversion/branches/1.15.x@1933215 13f79535-47bb-0310-9956-ffa450edef68
1 parent cff04b9 commit 73aeca5

2 files changed

Lines changed: 149 additions & 107 deletions

File tree

‎STATUS‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,14 +57,3 @@ Veto-blocked changes:
5757

5858
Approved changes:
5959
=================
60-
61-
* r1931549
62-
Use bytewise content comparison in the "is the file modified?" working copy
63-
checks if we have the pristine file content.
64-
Justification:
65-
Avoid changing the file comparison characteristics for working copies with
66-
pristine contents.
67-
Notes:
68-
Discussed in https://lists.apache.org/thread/128rgf7ozn72ljfzn7o18gfhb9l43y26
69-
Votes:
70-
+1: kotkov, rinrab, ivan

‎subversion/libsvn_wc/questions.c‎

Lines changed: 149 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,99 @@
7777
mark it for removal?
7878
*/
7979

80+
static svn_error_t *
81+
compare_exact(svn_boolean_t *modified_p,
82+
svn_stream_t *stream,
83+
const char *eol_str,
84+
apr_hash_t *keywords,
85+
const svn_checksum_t *pristine_checksum,
86+
svn_stream_t *pristine_stream,
87+
const char *pristine_eol_str,
88+
apr_pool_t *scratch_pool)
89+
{
90+
svn_boolean_t same;
91+
92+
/* For exact comparison, we check that contents remain the same when
93+
retranslated according to file properties. */
94+
95+
if (pristine_stream)
96+
{
97+
/* We have pristine contents: wrap a stream to translate it into working
98+
copy form, and check that contents remain the same. */
99+
100+
pristine_stream = svn_subst_stream_translated(pristine_stream,
101+
eol_str, FALSE,
102+
keywords, TRUE,
103+
scratch_pool);
104+
105+
SVN_ERR(svn_stream_contents_same2(&same, pristine_stream, stream,
106+
scratch_pool));
107+
}
108+
else
109+
{
110+
svn_checksum_t *working_checksum;
111+
svn_checksum_t *detranslated_checksum;
112+
svn_checksum_t *retranslated_checksum;
113+
114+
/* We don't have pristine contents. To make the comparison work without
115+
it, let's check for two things:
116+
117+
1) That the checksum of the detranslated contents matches the recorded
118+
pristine checksum, as in the case of a non-exact comparison, ...
119+
120+
2) ...and additionally, that the contents of the working file does not
121+
change when retranslated according to its properties.
122+
123+
Technically we're going to do that with a single read of the file
124+
contents, while checksumming it's original, detranslated and
125+
retranslated versions.
126+
*/
127+
128+
stream = svn_stream_checksummed2(stream,
129+
&working_checksum, NULL,
130+
pristine_checksum->kind, TRUE,
131+
scratch_pool);
132+
133+
stream = svn_subst_stream_translated(stream,
134+
pristine_eol_str, TRUE,
135+
keywords, FALSE,
136+
scratch_pool);
137+
stream = svn_stream_checksummed2(stream,
138+
&detranslated_checksum, NULL,
139+
pristine_checksum->kind, TRUE,
140+
scratch_pool);
141+
142+
stream = svn_subst_stream_translated(stream, eol_str, FALSE,
143+
keywords, TRUE,
144+
scratch_pool);
145+
stream = svn_stream_checksummed2(stream,
146+
&retranslated_checksum, NULL,
147+
pristine_checksum->kind, TRUE,
148+
scratch_pool);
149+
150+
SVN_ERR(svn_stream_copy3(stream, svn_stream_empty(scratch_pool),
151+
NULL, NULL, scratch_pool));
152+
153+
same = svn_checksum_match(detranslated_checksum, pristine_checksum) &&
154+
svn_checksum_match(working_checksum, retranslated_checksum);
155+
}
156+
157+
*modified_p = !same;
158+
return SVN_NO_ERROR;
159+
}
80160

81161
/* Set *MODIFIED_P to TRUE if (after translation) VERSIONED_FILE_ABSPATH
82-
* (of VERSIONED_FILE_SIZE bytes) differs from pristine file with checksum
83-
* PRISTINE_CHECKSUM, else to FALSE if not.
162+
* (of VERSIONED_FILE_SIZE bytes) differs from PRISTINE_STREAM (of
163+
* PRISTINE_SIZE bytes with PRISTINE_CHECKSUM), else to FALSE if not.
164+
*
165+
* If PRISTINE_STREAM is NULL, perform checksum-based content comparison.
166+
* If PRISTINE_STREAM is not NULL, perform bytewise content comparison
167+
* against the provided stream. PRISTINE_STREAM will be closed before
168+
* a successful return.
84169
*
85170
* If EXACT_COMPARISON is FALSE, translate VERSIONED_FILE_ABSPATH's EOL
86171
* style and keywords to repository-normal form according to its properties,
87-
* calculate checksum and compare the result with PRISTINE_CHECKSUM.
172+
* and compare the result with pristine contents.
88173
* If EXACT_COMPARISON is TRUE, also check that VERSIONED_FILE_ABSPATH
89174
* contents remains the same when retranslated according to its properties.
90175
*
@@ -94,13 +179,16 @@
94179
* PROPS_MOD should be TRUE if the file's properties have been changed,
95180
* otherwise FALSE.
96181
*
182+
*
97183
* DB is a wc_db; use SCRATCH_POOL for temporary allocation.
98184
*/
99185
static svn_error_t *
100186
compare_and_verify(svn_boolean_t *modified_p,
101187
svn_wc__db_t *db,
102188
const char *versioned_file_abspath,
103189
svn_filesize_t versioned_file_size,
190+
svn_stream_t *pristine_stream,
191+
svn_filesize_t pristine_size,
104192
const svn_checksum_t *pristine_checksum,
105193
svn_boolean_t has_props,
106194
svn_boolean_t props_mod,
@@ -113,8 +201,7 @@ compare_and_verify(svn_boolean_t *modified_p,
113201
svn_boolean_t special = FALSE;
114202
svn_boolean_t need_translation;
115203
svn_stream_t *v_stream; /* versioned_file */
116-
svn_checksum_t *v_checksum;
117-
svn_error_t *err;
204+
svn_boolean_t same;
118205

119206
SVN_ERR_ASSERT(svn_dirent_is_absolute(versioned_file_abspath));
120207

@@ -140,20 +227,15 @@ compare_and_verify(svn_boolean_t *modified_p,
140227
else
141228
need_translation = FALSE;
142229

143-
if (! need_translation)
230+
/* Easy out check: different sizes with no translation mean the
231+
* file was modified. */
232+
if (!need_translation && versioned_file_size != pristine_size)
144233
{
145-
svn_filesize_t pristine_size;
234+
if (pristine_stream)
235+
SVN_ERR(svn_stream_close(pristine_stream));
146236

147-
SVN_ERR(svn_wc__db_pristine_read(NULL, &pristine_size, db,
148-
versioned_file_abspath, pristine_checksum,
149-
scratch_pool, scratch_pool));
150-
151-
if (versioned_file_size != pristine_size)
152-
{
153-
*modified_p = TRUE;
154-
155-
return SVN_NO_ERROR;
156-
}
237+
*modified_p = TRUE;
238+
return SVN_NO_ERROR;
157239
}
158240

159241
/* ### Other checks possible? */
@@ -169,13 +251,8 @@ compare_and_verify(svn_boolean_t *modified_p,
169251
/* We don't use APR-level buffering because the comparison function
170252
* will do its own buffering. */
171253
apr_file_t *file;
172-
err = svn_io_file_open(&file, versioned_file_abspath, APR_READ,
173-
APR_OS_DEFAULT, scratch_pool);
174-
/* Convert EACCESS on working copy path to WC specific error code. */
175-
if (err && APR_STATUS_IS_EACCES(err->apr_err))
176-
return svn_error_create(SVN_ERR_WC_PATH_ACCESS_DENIED, err, NULL);
177-
else
178-
SVN_ERR(err);
254+
SVN_ERR(svn_io_file_open(&file, versioned_file_abspath, APR_READ,
255+
APR_OS_DEFAULT, scratch_pool));
179256
v_stream = svn_stream_from_aprfile2(file, FALSE, scratch_pool);
180257

181258
if (need_translation)
@@ -188,79 +265,41 @@ compare_and_verify(svn_boolean_t *modified_p,
188265
pristine_eol_str = eol_str;
189266

190267
if (exact_comparison)
191-
{
192-
svn_checksum_t *working_checksum;
193-
svn_checksum_t *detranslated_checksum;
194-
svn_checksum_t *retranslated_checksum;
195-
196-
v_stream = svn_stream_checksummed2(v_stream,
197-
&working_checksum, NULL,
198-
pristine_checksum->kind, TRUE,
199-
scratch_pool);
200-
201-
v_stream = svn_subst_stream_translated(v_stream,
202-
pristine_eol_str, TRUE,
203-
keywords, FALSE,
204-
scratch_pool);
205-
v_stream = svn_stream_checksummed2(v_stream,
206-
&detranslated_checksum, NULL,
207-
pristine_checksum->kind, TRUE,
208-
scratch_pool);
209-
210-
v_stream = svn_subst_stream_translated(v_stream, eol_str, FALSE,
211-
keywords, TRUE,
212-
scratch_pool);
213-
v_stream = svn_stream_checksummed2(v_stream,
214-
&retranslated_checksum, NULL,
215-
pristine_checksum->kind, TRUE,
268+
return svn_error_trace(compare_exact(modified_p,
269+
v_stream, eol_str, keywords,
270+
pristine_checksum,
271+
pristine_stream,
272+
pristine_eol_str,
273+
scratch_pool));
274+
275+
/* Wrap file stream to detranslate into normal form,
276+
* "repairing" the EOL style if it is inconsistent. */
277+
v_stream = svn_subst_stream_translated(v_stream,
278+
pristine_eol_str,
279+
TRUE /* repair */,
280+
keywords,
281+
FALSE /* expand */,
216282
scratch_pool);
217-
218-
err = svn_stream_copy3(v_stream, svn_stream_empty(scratch_pool),
219-
NULL, NULL, scratch_pool);
220-
/* Convert EACCESS on working copy path to WC specific error code. */
221-
if (err && APR_STATUS_IS_EACCES(err->apr_err))
222-
return svn_error_create(SVN_ERR_WC_PATH_ACCESS_DENIED, err, NULL);
223-
else
224-
SVN_ERR(err);
225-
226-
if (svn_checksum_match(detranslated_checksum, pristine_checksum) &&
227-
svn_checksum_match(working_checksum, retranslated_checksum))
228-
{
229-
*modified_p = FALSE;
230-
}
231-
else
232-
{
233-
*modified_p = TRUE;
234-
}
235-
236-
return SVN_NO_ERROR;
237-
}
238-
else
239-
{
240-
/* Wrap file stream to detranslate into normal form,
241-
* "repairing" the EOL style if it is inconsistent. */
242-
v_stream = svn_subst_stream_translated(v_stream,
243-
pristine_eol_str,
244-
TRUE /* repair */,
245-
keywords,
246-
FALSE /* expand */,
247-
scratch_pool);
248-
}
249283
}
250284
}
251285

252-
/* Get checksum of detranslated (normalized) content. */
253-
err = svn_stream_contents_checksum(&v_checksum, v_stream,
254-
pristine_checksum->kind,
255-
scratch_pool, scratch_pool);
256-
/* Convert EACCESS on working copy path to WC specific error code. */
257-
if (err && APR_STATUS_IS_EACCES(err->apr_err))
258-
return svn_error_create(SVN_ERR_WC_PATH_ACCESS_DENIED, err, NULL);
286+
if (pristine_stream)
287+
{
288+
SVN_ERR(svn_stream_contents_same2(&same, pristine_stream, v_stream,
289+
scratch_pool));
290+
}
259291
else
260-
SVN_ERR(err);
292+
{
293+
svn_checksum_t *v_checksum;
261294

262-
*modified_p = (! svn_checksum_match(v_checksum, pristine_checksum));
295+
SVN_ERR(svn_stream_contents_checksum(&v_checksum, v_stream,
296+
pristine_checksum->kind,
297+
scratch_pool,
298+
scratch_pool));
299+
same = svn_checksum_match(v_checksum, pristine_checksum);
300+
}
263301

302+
*modified_p = !same;
264303
return SVN_NO_ERROR;
265304
}
266305

@@ -279,6 +318,9 @@ svn_wc__internal_file_modified_p(svn_boolean_t *modified_p,
279318
svn_boolean_t has_props;
280319
svn_boolean_t props_mod;
281320
const svn_io_dirent2_t *dirent;
321+
svn_stream_t *pristine_stream;
322+
svn_filesize_t pristine_size;
323+
svn_error_t *err;
282324

283325
/* Read the relevant info */
284326
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_boolean_t *modified_p,
358400
}
359401

360402
compare_them:
361-
/* Check all bytes, and verify checksum if requested. */
362-
SVN_ERR(compare_and_verify(modified_p, db,
363-
local_abspath, dirent->filesize,
364-
checksum, has_props, props_mod,
365-
exact_comparison,
366-
scratch_pool));
403+
SVN_ERR(svn_wc__db_pristine_read(&pristine_stream, &pristine_size,
404+
db, local_abspath, checksum,
405+
scratch_pool, scratch_pool));
406+
407+
err = compare_and_verify(modified_p, db,
408+
local_abspath, dirent->filesize,
409+
pristine_stream, pristine_size,
410+
checksum, has_props, props_mod,
411+
exact_comparison,
412+
scratch_pool);
413+
414+
/* At this point we already opened the pristine file, so we know that
415+
the access denied applies to the working copy path. */
416+
if (err && APR_STATUS_IS_EACCES(err->apr_err))
417+
return svn_error_create(SVN_ERR_WC_PATH_ACCESS_DENIED, err, NULL);
418+
else
419+
SVN_ERR(err);
367420

368421
if (!*modified_p)
369422
{

0 commit comments

Comments
 (0)