[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 02:31:40 -0400

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

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