This breaks: lock_tests 10, and switch_tests 17. Investigating...
On Thu, Apr 1, 2010 at 18:09, <gst...@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__': > > >
