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

Re: svn commit: r33063 - in trunk/subversion: libsvn_wc tests/cmdline

From: Arfrever Frehtes Taifersar Arahesis <arfrever.fta_at_gmail.com>
Date: Sun, 14 Sep 2008 17:56:14 +0200

2008-09-14 01:09:51 kfogel_at_tigris.org napisał(a):
> Author: kfogel
> Date: Sat Sep 13 16:09:50 2008
> New Revision: 33063
>
> Log:
> Fix issue #3282: replaced files should use the revert text-base rather
> than the regular text-base.
>
> * subversion/libsvn_wc/adm_ops.c
> (svn_wc_add3): If replacing, move the base text (if any) and base
> props to their 'revert' locations.
> (revert_admin_things): Always touch the 'reverted' boolean parameter.
> Look for a revert base if the entry is schedule-replace, without
> regard to entry->copied. (This undoes part of r26476, but r26476
> was not necessarily wrong; it's just that we've now fixed the bugs
> that that part of r26476 was compensating for.) Don't assume that
> there's a regular text-base at all: in a replace-without-history
> situation, there won't be. Correctly revert the working file when
> there is a revert text-base but no regular text-base. Update the
> timestamp based on whether text or props were reverted, not just
> text. Update doc string, both for above changes and to describe
> the 'use_commit_times' parameter, which was entirely undocumented.
>
> * subversion/libsvn_wc/adm_crawler.c
> (report_revisions_and_depths): If a file is scheduled for
> replacement, because the replaced file's base information is not
> useful in that case.

This change wasn't committed.

>
> * subversion/libsvn_wc/update_editor.c
> (choose_base_paths): A file is replaced if it is schedule-replace,
> regardless of whether or not it has copyfrom information. WTF.
> (add_file): Make the code match what the comment already said --
> allow the addition if the entry is scheduled for addition without
> history. Extend the comment to make it clear that "addition"
> includes replacement.
>
> * subversion/libsvn_wc/props.c
> (svn_wc__load_props): Don't examine entry->copied when determining
> whether to load revert props or not.
>
> * subversion/libsvn_wc/diff.c
> (file_diff): Diff against the revert base iff the regular base is
> not available.
>
> * subversion/tests/cmdline/prop_tests.py
> (test_list): Expect same_replacement_props to pass now.
>
> Modified:
> trunk/subversion/libsvn_wc/adm_ops.c
> trunk/subversion/libsvn_wc/diff.c
> trunk/subversion/libsvn_wc/props.c
> trunk/subversion/libsvn_wc/update_editor.c
> trunk/subversion/tests/cmdline/prop_tests.py
>
> Modified: trunk/subversion/libsvn_wc/adm_ops.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/adm_ops.c?pathrev=33063&r1=33062&r2=33063
> ==============================================================================
> --- trunk/subversion/libsvn_wc/adm_ops.c Sat Sep 13 12:27:32 2008 (r33062)
> +++ trunk/subversion/libsvn_wc/adm_ops.c Sat Sep 13 16:09:50 2008 (r33063)
> @@ -1521,6 +1521,34 @@ svn_wc_add3(const char *path,
> SVN_ERR(svn_wc__props_delete(path, svn_wc__props_working,
> adm_access, pool));
>
> + if (is_replace)
> + {
> + /* We don't want the old base text (if any) and base props to be
> + mistakenly used as the bases for the new, replacment object.

s/replacment/replacement/

> + So, move them out of the way. */
> +
> + /* ### TODO: In an ideal world, this whole function would be loggy.
> + * ### But the directory recursion code below is already tangled
> + * ### enough, and re-doing the code above would require setting
> + * ### up more of tmp_entry. It's more than a SMOP. For now,
> + * ### I'm leaving it be, though we set up the revert base(s)
> + * ### loggily because that's Just How It's Done.
> + */
> + svn_stringbuf_t *log_accum = svn_stringbuf_create("", pool);
> +
> + if (orig_entry->kind == svn_node_file)
> + {
> + const char *textb = svn_wc__text_base_path(path, FALSE, pool);
> + const char *rtextb = svn_wc__text_revert_path(path, FALSE, pool);
> + SVN_ERR(svn_wc__loggy_move(&log_accum, NULL, adm_access,
> + textb, rtextb, FALSE, pool));
> + }
> + SVN_ERR(svn_wc__loggy_revert_props_create(&log_accum, path,
> + adm_access, TRUE, pool));
> + SVN_ERR(svn_wc__write_log(adm_access, 0, log_accum, pool));
> + SVN_ERR(svn_wc__run_log(adm_access, NULL, pool));
> + }
> +
> if (kind == svn_node_dir) /* scheduling a directory for addition */
> {
>
> @@ -1713,8 +1741,14 @@ svn_wc_add(const char *path,
>
> */
>
> -/* Revert ENTRY for NAME in directory represented by ADM_ACCESS. Sets
> - *REVERTED to TRUE if something actually is reverted.
> +/* Revert ENTRY for NAME in directory represented by ADM_ACCESS.
> +
> + Set *REVERTED to TRUE if something (text or props or both) is
> + reverted, FALSE otherwise.
> +
> + If something is reverted and USE_COMMIT_TIMES is true, then update
> + the entry's timestamp to the last-committed-time; otherwise don't
> + do that.
>
> Use SVN_WC_ENTRY_THIS_DIR as NAME for reverting ADM_ACCESS directory
> itself.
> @@ -1736,6 +1770,9 @@ revert_admin_things(svn_wc_adm_access_t
> apr_hash_t *baseprops = NULL;
> svn_boolean_t revert_base = FALSE;
>
> + /* By default, assume no action; we'll see what happens later. */
> + *reverted = FALSE;
> +
> /* Build the full path of the thing we're reverting. */
> fullpath = svn_wc_adm_access_path(adm_access);
> if (strcmp(name, SVN_WC_ENTRY_THIS_DIR) != 0)
> @@ -1744,7 +1781,7 @@ revert_admin_things(svn_wc_adm_access_t
> /* Deal with properties. */
> if (entry->schedule == svn_wc_schedule_replace)
> {
> - revert_base = entry->copied;
> + revert_base = entry->schedule == svn_wc_schedule_replace;
> /* Use the revertpath as the new propsbase if it exists. */
>
> baseprops = apr_hash_make(pool);
> @@ -1799,54 +1836,108 @@ revert_admin_things(svn_wc_adm_access_t
>
> if (entry->kind == svn_node_file)
> {
> - svn_node_kind_t base_kind;
> - const char *base_thing;
> - svn_boolean_t tgt_modified;
> + svn_node_kind_t kind;
> +
> + const char *regular_base_path
> + = svn_wc__text_base_path(fullpath, FALSE, pool);
> +
> + /* This becomes NULL if there is no revert-base. */
> + const char *revert_base_path
> + = svn_wc__text_revert_path(fullpath, FALSE, pool);
>
> - /* If the working file is missing, we need to reinstall it. */
> if (! reinstall_working)
> {
> - svn_node_kind_t kind;
> + /* If the working file is missing, we need to reinstall it. */
> SVN_ERR(svn_io_check_path(fullpath, &kind, pool));
> if (kind == svn_node_none)
> reinstall_working = TRUE;
> }
>
> - base_thing = svn_wc__text_base_path(fullpath, FALSE, pool);
> -
> - /* Check for text base presence. */
> - SVN_ERR(svn_io_check_path(base_thing, &base_kind, pool));
> + /* Whether or not the working file was missing, if there is a
> + revert text-base, we'll need to put it back to the regular
> + text-base and then reinstall the working file. (Strictly
> + speaking, the working file could be unmodified w.r.t. the
> + revert-base, but discovering that would be costly and chances
> + are it's not the case anyway. So we just assume. */
> + SVN_ERR(svn_io_check_path(revert_base_path, &kind, pool));
> + if (kind == svn_node_file)
> + {
> + reinstall_working = TRUE;
> + }
> + else if (kind == svn_node_none)
> + {
> + SVN_ERR(svn_io_check_path(regular_base_path, &kind, pool));
> + if (kind != svn_node_file)
> + {
> + /* A real file must have either a regular or a revert
> + text-base. If it has neither, we could be looking at
> + the situation described in issue #2101, in which
> + case all we can do is deliver the expected error. */
> + return svn_error_createf(APR_ENOENT, NULL,
> + _("Error restoring text for '%s'"),
> + svn_path_local_style(fullpath, pool));
> + }
> + else
> + {
> + revert_base_path = NULL;
> + }
> + }
> + else
> + {
> + return svn_error_createf
> + (SVN_ERR_NODE_UNKNOWN_KIND, NULL,
> + _("unexpected kind for revert-base '%s'"),
> + svn_path_local_style(revert_base_path, pool));
> + }
> +
> + /* You'd think we could just write out one log command to move
> + the revert base (if any) to the regular base, then another to
> + copy-and-translate the regular base to the working file.
> +
> + Unfortunately, svn_wc__loggy_copy() doesn't actually write
> + out a copy instruction if the src file for the copy isn't
> + present *at the time the log is being composed*. See that
> + function's documentation for details.
> +
> + So instead, we write out a log command to copy-and-translate
> + the revert text-base to the working file, then another log
> + command to move the revert text-base to the regular
> + text-base. */
> +
> + if (revert_base_path)
> + {
> + SVN_ERR(svn_wc__loggy_copy
> + (&log_accum, NULL, adm_access, svn_wc__copy_translate,
> + revert_base_path, fullpath, FALSE, pool));
> + SVN_ERR(svn_wc__loggy_move
> + (&log_accum, NULL, adm_access,
> + revert_base_path, regular_base_path, FALSE, pool));
> + *reverted = TRUE;
> + }
> + else
> + {
> + /* No revert-base -- so don't assume reinstall_working either. */
>
> - if (base_kind != svn_node_file)
> - return svn_error_createf(APR_ENOENT, NULL,
> - _("Error restoring text for '%s'"),
> - svn_path_local_style(fullpath, pool));
> -
> - /* Look for a revert base file. If it exists use it for the
> - text base for the file. If it doesn't use the normal text base. */
> - SVN_ERR(svn_wc__loggy_move
> - (&log_accum, &tgt_modified, adm_access,
> - svn_wc__text_revert_path(fullpath, FALSE, pool), base_thing,
> - FALSE, pool));
> - reinstall_working = reinstall_working || tgt_modified;
> + if (! reinstall_working)
> + {
> + SVN_ERR(svn_wc__text_modified_internal_p
> + (&reinstall_working, fullpath, FALSE,
> + adm_access, FALSE, pool));
> + }
>
> - /* A shortcut: since we will translate when reinstall_working,
> - we don't need to check if the working file is modified. */
> - if (! reinstall_working)
> - SVN_ERR(svn_wc__text_modified_internal_p(&reinstall_working,
> - fullpath, FALSE, adm_access,
> - FALSE, pool));
> + if (reinstall_working)
> + {
> + SVN_ERR(svn_wc__loggy_copy
> + (&log_accum, NULL, adm_access, svn_wc__copy_translate,
> + regular_base_path, fullpath, FALSE, pool));
> + *reverted = TRUE;
> + }
> + }
>
> + /* If we reinstalled the working file, then maybe update the
> + text timestamp in the entries file. */
> if (reinstall_working)
> {
> - /* If there are textual mods (or if the working file is
> - missing altogether), copy the text-base out into
> - the working copy, and update the timestamp in the entries
> - file. */
> - SVN_ERR(svn_wc__loggy_copy(&log_accum, NULL, adm_access,
> - svn_wc__copy_translate,
> - base_thing, fullpath, FALSE, pool));
> -
> /* Possibly set the timestamp to last-commit-time, rather
> than the 'now' time that already exists. */
> if (use_commit_times && entry->cmt_date)
> @@ -1860,8 +1951,6 @@ revert_admin_things(svn_wc_adm_access_t
> fullpath, SVN_WC__ENTRY_ATTR_TEXT_TIME, pool));
> SVN_ERR(svn_wc__loggy_set_entry_working_size_from_wc
> (&log_accum, adm_access, fullpath, pool));
> -
> - *reverted = TRUE;
> }
> }
>
> @@ -1980,10 +2069,14 @@ revert_entry(svn_depth_t *depth,
> apr_pool_t *pool)
> {
> const char *bname;
> - svn_boolean_t reverted = FALSE;
> svn_boolean_t is_wc_root = FALSE;
> svn_wc_adm_access_t *dir_access;
>
> + /* Initialize this even though revert_admin_things() is guaranteed
> + to set it, because we don't know that revert_admin_things() will
> + be called. */
> + svn_boolean_t reverted = FALSE;
> +
> /* Fetch the access baton for this path. */
> SVN_ERR(svn_wc_adm_probe_retrieve(&dir_access, parent_access, path, pool));
>

This change isn't described in the log message.

-- 
Arfrever Frehtes Taifersar Arahesis

Received on 2008-09-14 18:01:12 CEST

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