[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r930111 - in /subversion/trunk/subversion: libsvn_wc/update_editor.c tests/cmdline/trans_tests.py

From: Greg Stein <gstein_at_gmail.com>
Date: Fri, 2 Apr 2010 01:20:59 -0400

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

This is an archived mail posted to the Subversion Dev mailing list.