dionisos@tigris.org writes:
> Log:
> Make the 'working file has changed' heuristic more robust:
> check for the size of the file too.
>
Hi, Erik,
This is much appreciated!
>
> Modified: trunk/subversion/include/svn_wc.h
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/include/svn_wc.h?pathrev=23767&r1=23766&r2=23767
> ==============================================================================
> --- trunk/subversion/include/svn_wc.h (original)
> +++ trunk/subversion/include/svn_wc.h Sat Mar 10 03:22:25 2007
> @@ -1384,6 +1384,12 @@
> */
> const char *changelist;
>
> + /** Size of the file after being translated into local representation,
> + * or 0 if unknown.
> + * @since New in 1.5.
> + */
> + apr_off_t working_size;
This was already mentioned, but I'm still very uneasy about using a valid
value as "size unknown".
> +
> /** Whether a local copy of this entry should be kept in the working copy
> * after a deletion has been committed, Only valid for the this-dir entry
> * when it is scheduled for deletion.
>
> Modified: trunk/subversion/libsvn_wc/entries.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/entries.c?pathrev=23767&r1=23766&r2=23767
> ==============================================================================
> --- trunk/subversion/libsvn_wc/entries.c (original)
> +++ trunk/subversion/libsvn_wc/entries.c Sat Mar 10 03:22:25 2007
> @@ -2,7 +2,7 @@
> * entries.c : manipulating the administrative `entries' file.
> *
> * ====================================================================
> - * Copyright (c) 2000-2006 CollabNet. All rights reserved.
> + * Copyright (c) 2000-2007 CollabNet. All rights reserved.
> *
> * This software is licensed as described in the file COPYING, which
> * you should have received as part of this distribution. The terms
> @@ -454,6 +454,18 @@
> /* Keep entry in working copy after deletion? */
> SVN_ERR(read_bool(&entry->keep_local, SVN_WC__ENTRY_ATTR_KEEP_LOCAL,
> buf, end));
> + MAYBE_DONE;
> +
> + /* Translated size */
> + {
> + const char *val;
> +
> + SVN_ERR(read_str(&val, buf, end, pool));
> +
read_str should be read_val because the value cannot include escape sequences.
> + entry->working_size =
> + (apr_off_t)apr_strtoi64(val, NULL, 0);
> + }
> + MAYBE_DONE;
>
> done:
> *new_entry = entry;
> @@ -847,6 +859,27 @@
> entry->present_props = apr_pstrdup(pool, entry->present_props);
> }
>
> + /* Translated size */
> + {
> + const char *val
> + = apr_hash_get(atts,
> + SVN_WC__ENTRY_ATTR_WORKING_SIZE,
> + APR_HASH_KEY_STRING);
> + if (val)
> + {
> + if (! strcmp(val, SVN_WC__WORKING_SIZE_WC))
> + {
> + /* Special case (same as the timestamps); ignore here
> + these will be handled elsewhere */
> + }
> + else
> + /* Cast to off_t; it's safe: we put in an off_t to start with... */
> + entry->working_size = (apr_off_t)apr_strtoi64(val, NULL, 0);
> +
> + *modify_flags |= SVN_WC__ENTRY_MODIFY_WORKING_SIZE;
> + }
> + }
> +
> *new_entry = entry;
> return SVN_NO_ERROR;
> }
> @@ -1554,6 +1587,12 @@
> /* Keep in working copy flag. */
> write_bool(buf, SVN_WC__ENTRY_ATTR_KEEP_LOCAL, entry->keep_local);
>
> + /* Translated size */
> + write_str(buf,
> + entry->working_size
> + ? apr_off_t_toa(pool, entry->working_size) : "",
> + pool);
Should be write_val, mostly for symmetric reasons...
> Modified: trunk/subversion/libsvn_wc/log.h
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/log.h?pathrev=23767&r1=23766&r2=23767
> ==============================================================================
> --- trunk/subversion/libsvn_wc/log.h (original)
> +++ trunk/subversion/libsvn_wc/log.h Sat Mar 10 03:22:25 2007
> @@ -219,6 +219,20 @@
> const char *time_prop,
> apr_pool_t *pool);
>
> +
> +/* Extend **LOG_ACCUM with log instructions to set the timestamp of PATH
> + in the entry field with name TIME_PROP.
> +
> + Use SVN_WC__ENTRY_ATTR_* values for TIME_PROP.
> +*/
> +
> +svn_error_t *
> +svn_wc__loggy_set_entry_working_size_from_wc(svn_stringbuf_t **log_accum,
> + svn_wc_adm_access_t *adm_access,
> + const char *path,
> + apr_pool_t *pool);
Indentation got screwed up here. Also, docstrings talks about a parameter
TIME_PROP which is not present. Also, what does the log instructions set
the timestamp *to*?
> Modified: trunk/subversion/libsvn_wc/questions.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/questions.c?pathrev=23767&r1=23766&r2=23767
> ==============================================================================
> --- trunk/subversion/libsvn_wc/questions.c (original)
> +++ trunk/subversion/libsvn_wc/questions.c Sat Mar 10 03:22:25 2007
> @@ -163,6 +163,41 @@
> mark it for removal?
> */
>
> +
> +/*
> + */
Nice docstring, maybe a bit short, thoug:-)
> +static svn_error_t *
> +localfile_size_changed_p(svn_boolean_t *changed_p,
> + const char *path,
> + svn_wc_adm_access_t *adm_access,
> + apr_pool_t *pool)
> @@ -393,18 +428,30 @@
>
> if (! force_comparison)
> {
> + /* See if the local file's size is different from when we created it */
> +
> + /*### The number of stat() calls can be reduced by integrating
> + localfile_size_changed_p() and svn_wc__timestamps_equal_p() here. */
> +
I'd say that this is pretty important. This function is called for every
versioned file in svn status and we've worked quite hard to eliminate
unnecesary stat calls in the common codepath of that command. It would be
really sad if this work was partially undone by this change, and there's
actually no real reason to keep it this way, is there?
> Modified: trunk/subversion/libsvn_wc/update_editor.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/update_editor.c?pathrev=23767&r1=23766&r2=23767
> ==============================================================================
> --- trunk/subversion/libsvn_wc/update_editor.c (original)
> +++ trunk/subversion/libsvn_wc/update_editor.c Sat Mar 10 03:22:25 2007
> @@ -2052,13 +2052,15 @@
> apr_uint64_t modify_flags = SVN_WC__ENTRY_MODIFY_KIND
> | SVN_WC__ENTRY_MODIFY_REVISION
> | SVN_WC__ENTRY_MODIFY_DELETED
> - | SVN_WC__ENTRY_MODIFY_ABSENT;
> + | SVN_WC__ENTRY_MODIFY_ABSENT
> + | SVN_WC__ENTRY_MODIFY_WORKING_SIZE;
>
>
> tmp_entry.revision = new_revision;
> tmp_entry.kind = svn_node_file;
> tmp_entry.deleted = FALSE;
> tmp_entry.absent = FALSE;
> + tmp_entry.working_size = 0;
>
> /* Possibly install a *non*-inherited URL in the entry. */
> if (new_URL)
Maybe it doesn't hurt, but why do we modify the working size here, but not the
text timestamps? I'd imagine they should be treated quite similarly.
As I said before, the robustness improvement is appreciated.
Thanks,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Mar 13 09:43:20 2007