You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com> on 2008/09/14 15:56:14 UTC

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

2008-09-14 01:09:51 kfogel@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

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

Posted by Karl Fogel <kf...@red-bean.com>.
Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com> writes:
>> * 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.

Sorry, that was left over from a previous iteration of the patch; see
http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=142900
for why I undid it.

I've propedited the log message now.

Stefan, this renders the grammatical problem inoperative :-).

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org