Fixed in r930175.
grrr......
On Fri, Apr 2, 2010 at 01:20, Greg Stein <gstein_at_gmail.com> wrote:
> This breaks: lock_tests 10, and switch_tests 17.
>
> Investigating...
>
> On Thu, Apr 1, 2010 at 18:09, <gstein_at_apache.org> wrote:
>> Author: gstein
>> Date: Thu Apr 1 22:09:09 2010
>> New Revision: 930111
>>
>> URL: http://svn.apache.org/viewvc?rev=930111&view=rev
>> Log:
>> Fix a leaking temporary file in the update editor, including a test to
>> verify the leakage (and its fix).
>>
>> Expand a bunch of comments, and relocate some work items for clarity.
>>
>> * subversion/libsvn_wc/update_editor.c:
>> (merge_file): remove the LEFT_VERSION and RIGHT_VERSION localvars. they
>> can be reintroduced if they ever get used. add a bunch of commentary
>> around the computation if IS_LOCALLY_MODIFIED. make sure that we
>> delete the TMPTEXT temporary file after we've used it. relocated the
>> deletion of the "copied text base" over to close_file().
>> (close_file): remove the copied text base once we're done with it.
>>
>> * subversion/tests/cmdline/trans_test.py:
>> (props_only_file_update): new test to ensure that a file is
>> retranslated on a props-only update.
>> (test_list): add new test
>>
>> Modified:
>> subversion/trunk/subversion/libsvn_wc/update_editor.c
>> subversion/trunk/subversion/tests/cmdline/trans_tests.py
>>
>> Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=930111&r1=930110&r2=930111&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
>> +++ subversion/trunk/subversion/libsvn_wc/update_editor.c Thu Apr 1 22:09:09 2010
>> @@ -4269,10 +4269,6 @@ merge_file(svn_stringbuf_t **log_accum,
>> svn_boolean_t is_replaced = FALSE;
>> svn_boolean_t magic_props_changed;
>> enum svn_wc_merge_outcome_t merge_outcome = svn_wc_merge_unchanged;
>> - svn_wc_conflict_version_t *left_version = NULL; /* ### Fill */
>> - svn_wc_conflict_version_t *right_version = NULL; /* ### Fill */
>> -
>> - /* Accumulated entry modifications. */
>> svn_wc_entry_t tmp_entry;
>> apr_uint64_t flags = 0;
>>
>> @@ -4285,6 +4281,7 @@ merge_file(svn_stringbuf_t **log_accum,
>>
>> - The .svn/entries file still reflects the old version of F.
>>
>> + ### there is no fb->old_text_base_path
>> - fb->old_text_base_path is the old pristine F.
>> (This is only set if there's a new text base).
>>
>> @@ -4324,16 +4321,34 @@ merge_file(svn_stringbuf_t **log_accum,
>> call reads the entries from the database the returned entry is
>> svn_wc_schedule_replace. 2 lines marked ### EBUG below. */
>> if (fb->copied_working_text)
>> - is_locally_modified = TRUE;
>> + {
>> + /* The file was copied here, and it came with both a (new) pristine
>> + and a working file. Presumably, the working file is modified
>> + relative to the new pristine.
>> +
>> + The new pristine is in NEW_TEXT_BASE_ABSPATH, which should also
>> + be FB->COPIED_TEXT_BASE. */
>> + is_locally_modified = TRUE;
>> + }
>> else if (entry && entry->file_external_path
>> && entry->schedule == svn_wc_schedule_replace) /* ###EBUG */
>> - is_locally_modified = FALSE;
>> + {
>> + is_locally_modified = FALSE;
>> + }
>> else if (! fb->obstruction_found)
>> - SVN_ERR(svn_wc__internal_text_modified_p(&is_locally_modified, eb->db,
>> - fb->local_abspath, FALSE, FALSE,
>> - pool));
>> + {
>> + /* The working file is not an obstruction. So: is the file modified,
>> + relative to its ORIGINAL pristine? */
>> + SVN_ERR(svn_wc__internal_text_modified_p(&is_locally_modified, eb->db,
>> + fb->local_abspath,
>> + FALSE /* force_comparison */,
>> + FALSE /* compare_textbases */,
>> + pool));
>> + }
>> else if (new_text_base_abspath)
>> {
>> + /* We have a new pristine to install. Is the file modified relative
>> + to this new pristine? */
>> SVN_ERR(svn_wc__internal_versioned_file_modcheck(&is_locally_modified,
>> eb->db,
>> fb->local_abspath,
>> @@ -4341,7 +4356,10 @@ merge_file(svn_stringbuf_t **log_accum,
>> FALSE, pool));
>> }
>> else
>> - is_locally_modified = FALSE;
>> + {
>> + /* No other potential changes, so the working file is NOT modified. */
>> + is_locally_modified = FALSE;
>> + }
>>
>> if (entry && entry->schedule == svn_wc_schedule_replace
>> && ! entry->file_external_path) /* ### EBUG */
>> @@ -4495,8 +4513,8 @@ merge_file(svn_stringbuf_t **log_accum,
>> SVN_ERR(svn_wc__internal_merge(
>> log_accum, &merge_outcome,
>> eb->db,
>> - merge_left, left_version,
>> - new_text_base_abspath, right_version,
>> + merge_left, NULL,
>> + new_text_base_abspath, NULL,
>> fb->local_abspath,
>> fb->copied_working_text,
>> oldrev_str, newrev_str, mine_str,
>> @@ -4504,6 +4522,9 @@ merge_file(svn_stringbuf_t **log_accum,
>> eb->conflict_func, eb->conflict_baton,
>> eb->cancel_func, eb->cancel_baton,
>> pool));
>> +
>> + /* ### now that we're past the internal_merge() (which might
>> + ### cause an exit), we can start writing work items. */
>> SVN_WC__FLUSH_LOG_ACCUM(eb->db, pb->local_abspath, *log_accum,
>> pool);
>>
>> @@ -4563,6 +4584,18 @@ merge_file(svn_stringbuf_t **log_accum,
>> SVN_ERR(svn_wc__loggy_copy(log_accum, pb->local_abspath,
>> tmptext, fb->local_abspath, pool, pool));
>> SVN_WC__FLUSH_LOG_ACCUM(eb->db, pb->local_abspath, *log_accum, pool);
>> +
>> + /* Done with the temporary file. Toss it. */
>> + {
>> + const svn_skel_t *work_item;
>> +
>> + SVN_ERR(svn_wc__wq_build_file_remove(&work_item,
>> + eb->db,
>> + tmptext,
>> + pool, pool));
>> + SVN_ERR(svn_wc__db_wq_add(eb->db, pb->local_abspath,
>> + work_item, pool));
>> + }
>> }
>>
>> if (lock_removed)
>> @@ -4628,17 +4661,6 @@ merge_file(svn_stringbuf_t **log_accum,
>> SVN_WC__FLUSH_LOG_ACCUM(eb->db, pb->local_abspath, *log_accum, pool);
>> }
>>
>> - /* Clean up add-with-history temp file. */
>> - if (fb->copied_text_base)
>> - {
>> - const svn_skel_t *work_item;
>> -
>> - SVN_ERR(svn_wc__wq_build_file_remove(&work_item,
>> - eb->db, fb->copied_text_base,
>> - pool, pool));
>> - SVN_ERR(svn_wc__db_wq_add(eb->db, pb->local_abspath, work_item, pool));
>> - }
>> -
>> /* Set the returned content state. */
>>
>> /* This is kind of interesting. Even if no new text was
>> @@ -4939,6 +4961,18 @@ close_file(void *file_baton,
>> SVN_WC__FLUSH_LOG_ACCUM(eb->db, fb->dir_baton->local_abspath,
>> delayed_log_accum, pool);
>>
>> + /* Clean up any temporary files. */
>> + if (fb->copied_text_base)
>> + {
>> + const svn_skel_t *work_item;
>> +
>> + SVN_ERR(svn_wc__wq_build_file_remove(&work_item,
>> + eb->db, fb->copied_text_base,
>> + pool, pool));
>> + SVN_ERR(svn_wc__db_wq_add(eb->db, fb->dir_baton->local_abspath,
>> + work_item, pool));
>> + }
>> +
>> /* We have one less referrer to the directory's bump information. */
>> SVN_ERR(maybe_bump_dir_info(eb, fb->bump_info, pool));
>>
>>
>> Modified: subversion/trunk/subversion/tests/cmdline/trans_tests.py
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/trans_tests.py?rev=930111&r1=930110&r2=930111&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/tests/cmdline/trans_tests.py (original)
>> +++ subversion/trunk/subversion/tests/cmdline/trans_tests.py Thu Apr 1 22:09:09 2010
>> @@ -790,6 +790,92 @@ def propset_revert_noerror(sbox):
>> svntest.actions.run_and_verify_status(wc_dir, expected_status)
>>
>>
>> +def props_only_file_update(sbox):
>> + "retranslation occurs on a props-only update"
>> +
>> + sbox.build()
>> + wc_dir = sbox.wc_dir
>> +
>> + iota_path = os.path.join(wc_dir, 'iota')
>> + content = ["This is the file 'iota'.\n",
>> + "$Author$\n",
>> + ]
>> + content_expanded = ["This is the file 'iota'.\n",
>> + "$Author: jrandom $\n",
>> + ]
>> +
>> + # Create r2 with iota's contents and svn:keywords modified
>> + open(iota_path, 'w').writelines(content)
>> + svntest.main.run_svn(None, 'propset', 'svn:keywords', 'Author', iota_path)
>> +
>> + expected_output = wc.State(wc_dir, {
>> + 'iota' : Item(verb='Sending'),
>> + })
>> +
>> + expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
>> + expected_status.tweak('iota', wc_rev=2)
>> +
>> + svntest.actions.run_and_verify_commit(wc_dir,
>> + expected_output,
>> + expected_status,
>> + None,
>> + wc_dir)
>> +
>> + # Create r3 that drops svn:keywords
>> +
>> + # put the content back to its untranslated form
>> + open(iota_path, 'w').writelines(content)
>> +
>> + svntest.main.run_svn(None, 'propset', 'svn:keywords', 'Id', iota_path)
>> +
>> +# expected_output = wc.State(wc_dir, {
>> +# 'iota' : Item(verb='Sending'),
>> +# })
>> +
>> + expected_status.tweak('iota', wc_rev=3)
>> +
>> + svntest.actions.run_and_verify_commit(wc_dir,
>> + expected_output,
>> + expected_status,
>> + None,
>> + wc_dir)
>> +
>> + # Now, go back to r2. iota should have the Author keyword expanded.
>> + expected_disk = svntest.main.greek_state.copy()
>> + expected_disk.tweak('iota', contents=''.join(content_expanded))
>> +
>> + expected_status = svntest.actions.get_virginal_state(wc_dir, 2)
>> +
>> + svntest.actions.run_and_verify_update(wc_dir,
>> + None, None, expected_status,
>> + None,
>> + None, None, None, None,
>> + False,
>> + wc_dir, '-r', '2')
>> +
>> + # Update to r3. this should retranslate iota, dropping the keyword expansion
>> + expected_disk = svntest.main.greek_state.copy()
>> + expected_disk.tweak('iota', contents=''.join(content))
>> +
>> + expected_status = svntest.actions.get_virginal_state(wc_dir, 3)
>> +
>> + svntest.actions.run_and_verify_update(wc_dir,
>> + None, expected_disk, expected_status,
>> + None,
>> + None, None, None, None,
>> + False,
>> + wc_dir)
>> +
>> + # We used to leave some temporary files around. Make sure that we don't.
>> + temps = os.listdir(os.path.join(wc_dir, '.svn', 'tmp'))
>> + temps.remove('prop-base')
>> + temps.remove('props')
>> + temps.remove('text-base')
>> + if temps:
>> + print('Temporary files leftover: %s' % (', '.join(temps),))
>> + raise svntest.Failure
>> +
>> +
>> ########################################################################
>> # Run the tests
>>
>> @@ -807,6 +893,7 @@ test_list = [ None,
>> copy_propset_commit,
>> propset_commit_checkout_nocrash,
>> propset_revert_noerror,
>> + props_only_file_update,
>> ]
>>
>> if __name__ == '__main__':
>>
>>
>>
>
Received on 2010-04-02 08:32:16 CEST