Quoting Julian Foad <julianfoad_at_btopenworld.com>:
> On Wed, 2008-11-26 at 03:59 +0100, Neels J Hofmeyr wrote:
>> sbutler_at_tigris.org wrote:
>> > Author: sbutler
>> > Date: Sat Nov 22 15:46:26 2008
>> > New Revision: 34345
>> >
>> > Log:
>> > Properly loggify the recording of tree conflict data during update and
>> > switch. The desired end result is the same as before; only the timing
>> > of the entry-writing has changed. It's now consistent with the other
>> > loggy commands.
[...]
> Now, the interesting and difficult stuff ...
>
>> > +
>> > + /* Re-adding an existing tree conflict victim is an error. */
>> > + if (svn_wc__tree_conflict_exists(loggy->tree_conflicts,
>> > + svn_path_basename(new_conflict->path,
>> > + loggy->pool),
>> > + loggy->pool))
>> > + return svn_error_create(SVN_ERR_WC_CORRUPT, NULL,
>> > + _("Attempt to add tree conflict that
>> already exists"));
>
> This is wrong. Loggy operations must be idempotent, so the meaning of
> the operation should not be strictly "add" but instead "set" the tree
> conflict description for a particular victim. It should silently do
> nothing if the conflict already exists.
OK.
>
> If a conflict description already exists for the child, but differs from
> the new conflict description for that same child, that is an error. We
> could detect that. In theory we will never try to raise such a conflict,
> but a check here would be helpful for bug-catching.
>
> There should be a check at a higher level for trying to add an identical
> conflict that already exists, as that would be useful for debugging too.
[...]
>> > static void
>> > start_handler(void *userData, const char *eltname, const char **atts)
>> > {
[...]
> Why is the array not initialized from the 'entry' here in
> run_log_from_memory() like it is in run_log()?
In run_log_from_memory() there's no rewriting of the entry, so there's
no point in collecting tree conflicts.
>
>> > + loggy->tree_conflicts = NULL;
>> > @@ -1720,6 +1773,7 @@ run_log(svn_wc_adm_access_t *adm_access,
[...]
>> > + /* Populate the tree conflict array with the existing tree
>> conflicts. */
>> > + SVN_ERR(svn_wc_entry(&entry,
>> svn_wc_adm_access_path(adm_access), adm_access,
>> > + TRUE, pool));
>> > + SVN_ERR(svn_wc__read_tree_conflicts(&(loggy->tree_conflicts),
>> > + entry->tree_conflict_data,
>> > + svn_wc_adm_access_path(adm_access),
>> > + pool));
>>
>> Nice, only one read of existing conflicts per parent dir.
>> But, wait a minute, that means that the conflicts are all written out only
>> *after* the rest of the loggy commands have been executed?
>
> Yes. This code runs a log file with tree conflict changes as follows:
>
> 1. Read the existing tree conflicts from the 'entry' in the adm_access
> cache, into a list in memory.
>
> 2. Execute all of the log commands; for the "add tree conflict"
> command, add the description to the list in memory.
>
> 3. Write the new list of tree conflicts to the 'entry' with
> svn_wc__entry_modify().
>
> 4. Some sort of re-synchronisation:
>> {
>> apr_hash_t *entries;
>> SVN_ERR(svn_wc_entries_read(&entries, adm_access, TRUE, pool));
>> SVN_ERR(svn_wc__entries_write(entries, adm_access, pool));
>> }
>
> If the "add tree conflict" command is idempotent, will this whole
> algorithm should be fine?
I think so. I'll just tweak step #2 to overwrite an existing TC silently.
>
>
>> Could that be a problem? I don't know enough about it, so I'm paranoid about
>> changing the order in any way... :P
>
> I don't understand the implications either, so I cannot say whether it
> is OK.
All of the entry updates (including tree conflict data) are written to
disk at the same time, in a single call to svn_wc__entries_write().
>
>> Does it make sense to write out the new state of tree_conflicts after each
>> consecutive series of tree-conflicts-add log commands?
>>
>> >
>> > /* Expat wants everything wrapped in a top-level form, so start with
>> > a ghost open tag. */
>> > @@ -1794,7 +1860,29 @@ run_log(svn_wc_adm_access_t *adm_access,
>> > goto rerun;
>> > #endif
>
> That "#endif" is the end of "#ifdef RERUN_LOG_FILES" testing loop, so
> that we can test for the ability to re-run the loggy operations without
> ill effect. I think the new block added below should be within that
> loop.
>
> However, I tried enabling "RERUN_LOG_FILES" in trunk_at_34498 without
> making any other change, and many of the regression tests fail, so it
> looks like the loggy code has bit-rotted.
Interesting! I'm traveling for a few hours tomorrow. I'll have a look.
Steve
--
Stephen Butler | Software Developer
elego Software Solutions GmbH
Gustav-Meyer-Allee 25 | 13355 Berlin | Germany
fon: +49 30 2345 8696 | mobile: +49 163 25 45 015
fax: +49 30 2345 8695 | http://www.elegosoft.com
Geschäftsführer: Olaf Wagner | Sitz der Gesellschaft: Berlin
Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-12-01 20:15:51 CET