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 07:21:34 CEST