You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ne...@apache.org on 2010/03/10 21:37:14 UTC

svn commit: r921556 - in /subversion/trunk/subversion: libsvn_client/cat.c libsvn_client/export.c libsvn_wc/adm_crawler.c libsvn_wc/copy.c libsvn_wc/diff.c libsvn_wc/questions.c libsvn_wc/update_editor.c

Author: neels
Date: Wed Mar 10 20:37:13 2010
New Revision: 921556

URL: http://svn.apache.org/viewvc?rev=921556&view=rev
Log:
Fix all callers of svn_wc_get_pristine_contents2() and
svn_wc__get_pristine_contents(): make them able to cope with a returned NULL
stream pointer in *CONTENTS, as advertised by that function.

All current callers would have broken when svn_wc_get_pristine_contents2() was
returning NULL in *CONTENTS as described in the comment. That's probably
because the current implementation in effect assumes the text-base file to be
always present and never successfully returns NULL in *CONTENTS.
This patch adds sensitivity for a NULL returned in *CONTENTS to all callers,
to prepare for a patch that fixes svn_wc_get_pristine_contents2().

* subversion/libsvn_client/cat.c
  (cat_local_file):
    Handle a return value of *CONTENT == NULL by returning
    SVN_ERR_ILLEGAL_TARGET. (Previously failed with an error complaining about
    a non-existing text-base file.)

* subversion/libsvn_client/export.c
  (copy_one_versioned_file):
    Remove a condition that tried to avoid getting a non-existing base and
    instead use svn_wc_get_pristine_contents2()'s NULL return value to achieve
    the same. This also skips export for replaced files that have no base,
    where previously only added files were checked.  Add TODO comment to ask
    for proper treatment of replaced files (previously possibly failed with an
    error complaining about a non-existing text-base file).
  (copy_versioned_files):
    Fix a condition that avoids getting a non-existing base to also check
    replaced files (was checking only added files), and to not skip files that
    were copied-/moved-here. (This function can skip more stuff than
    copy_one_versioned_file(), so taking a different approach here.) Comment.

* subversion/libsvn_wc/adm_crawler.c
  (restore_file):
    Handle a return value of *CONTENT == NULL by skipping restoration for such
    nodes. (Previously possibly failed with an error complaining about a
    non-existing text-base file.)
  (svn_wc__internal_transmit_text_deltas):
    Handle a return value of *CONTENT == NULL by using an empty stream
    instead (in two places).

* subversion/libsvn_wc/copy.c
  (copy_file_administratively):
    Fix a condition that avoids getting a non-existing base to also check
    replaced files (was checking only added files), and to not skip files that
    were copied-/moved-here. Add two assertions to make sure we have covered
    all cases. Add TODO comment to ask for proper treatment of replaced files.

* subversion/libsvn_wc/diff.c
  (apply_textdelta):
    Handle a return value of *CONTENT == NULL by using an empty stream
    instead.

* subversion/libsvn_wc/questions.c
  (svn_wc__internal_text_modified_p):
    Handle a return value of *CONTENT == NULL by returning *MODIFIED_P = TRUE.
    (Previously possibly failed with an error complaining about a non-existing
    text-base file.)

* subversion/libsvn_wc/update_editor.c
  (add_file_with_history):
    Add assertion to verify that this code only hits nodes that have a base.
  (apply_textdelta):
    Handle a return value of *CONTENT == NULL by using an empty stream
    instead.

Modified:
    subversion/trunk/subversion/libsvn_client/cat.c
    subversion/trunk/subversion/libsvn_client/export.c
    subversion/trunk/subversion/libsvn_wc/adm_crawler.c
    subversion/trunk/subversion/libsvn_wc/copy.c
    subversion/trunk/subversion/libsvn_wc/diff.c
    subversion/trunk/subversion/libsvn_wc/questions.c
    subversion/trunk/subversion/libsvn_wc/update_editor.c

Modified: subversion/trunk/subversion/libsvn_client/cat.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/cat.c?rev=921556&r1=921555&r2=921556&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/cat.c (original)
+++ subversion/trunk/subversion/libsvn_client/cat.c Wed Mar 10 20:37:13 2010
@@ -86,6 +86,12 @@ cat_local_file(svn_wc_context_t *wc_ctx,
     {
       SVN_ERR(svn_wc_get_pristine_contents2(&input, wc_ctx, local_abspath,
                                             scratch_pool, scratch_pool));
+
+      if (input == NULL)
+        return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL,
+                 _("'%s' has no base revision until it is committed"),
+                 svn_dirent_local_style(local_abspath, scratch_pool));
+
       SVN_ERR(svn_wc_get_prop_diffs2(NULL, &props, wc_ctx, local_abspath,
                                      scratch_pool, scratch_pool));
     }

Modified: subversion/trunk/subversion/libsvn_client/export.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/export.c?rev=921556&r1=921555&r2=921556&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/export.c (original)
+++ subversion/trunk/subversion/libsvn_client/export.c Wed Mar 10 20:37:13 2010
@@ -128,23 +128,43 @@ copy_one_versioned_file(const char *from
   else if (err)
     return svn_error_return(err);
 
-  /* Only export 'added' files when the revision is WORKING.
-     Otherwise, skip the 'added' files, since they didn't exist
-     in the BASE revision and don't have an associated text-base.
+  /* Only export 'added' and 'replaced' files when the revision is WORKING, or
+     when they are part of a copy-/move-here and thus have a pristine base.
+     Otherwise, skip the 'added' files, since they didn't exist in the BASE
+     revision and don't have an associated text-base.
 
      Don't export 'deleted' files and directories unless it's a
      revision other than WORKING.  These files and directories
      don't really exist in WORKING. */
-  if ((revision->kind != svn_opt_revision_working &&
-       entry->schedule == svn_wc_schedule_add) ||
-      (revision->kind == svn_opt_revision_working &&
-       entry->schedule == svn_wc_schedule_delete))
+  if ((revision->kind != svn_opt_revision_working
+       && (entry->schedule == svn_wc_schedule_add
+           || entry->schedule == svn_wc_schedule_replace)
+       && !entry->copied)
+      || (revision->kind == svn_opt_revision_working &&
+          entry->schedule == svn_wc_schedule_delete))
     return SVN_NO_ERROR;
 
+  /* ### TODO: Handle replaced nodes properly.
+     ###       svn_opt_revision_base refers to the "new" 
+     ###       base of the node. That means, if a node is locally
+     ###       replaced, export skips this node, as if it was locally
+     ###       added, because svn_opt_revision_base refers to the base
+     ###       of the added node, not to the node that was deleted.
+     ###       In contrast, when the node is copied-here or moved-here,
+     ###       the copy/move source's content will be exported.
+     ###       It is currently not possible to export the revert-base
+     ###       when a node is locally replaced. We need a new
+     ###       svn_opt_revision_ enum value for proper distinction
+     ###       between revert-base and commit-base. */
+
   if (revision->kind != svn_opt_revision_working)
     {
       SVN_ERR(svn_wc_get_pristine_contents2(&source, wc_ctx, from_abspath,
                                             scratch_pool, scratch_pool));
+
+      /* Should have been caught by above add/replace condition. */
+      SVN_ERR_ASSERT(source != NULL);
+
       SVN_ERR(svn_wc_get_prop_diffs2(NULL, &props, wc_ctx, from_abspath,
                                      scratch_pool, scratch_pool));
     }
@@ -294,15 +314,23 @@ copy_versioned_files(const char *from,
                                       svn_node_unknown, FALSE, FALSE,
                                       pool, pool));
 
-  /* Only export 'added' files when the revision is WORKING.
-     Otherwise, skip the 'added' files, since they didn't exist
-     in the BASE revision and don't have an associated text-base.
+  /* Only export 'added' and 'replaced' files when the revision is WORKING;
+     when the revision is BASE (i.e. != WORKING), only export 'added' and
+     'replaced' files when they are part of a copy-/move-here. Otherwise, skip
+     them, since they don't have an associated text-base. This condition for
+     added/replaced simply is an optimization. Added and replaced files would
+     be handled similarly by svn_wc_get_pristine_contents2(), which would
+     return NULL if they have no base associated.
+     TODO: We may prefer not to duplicate this condition and rather use
+     svn_wc_get_pristine_contents2() or a dedicated new function instead.
 
      Don't export 'deleted' files and directories unless it's a
      revision other than WORKING.  These files and directories
      don't really exist in WORKING. */
-  if ((revision->kind != svn_opt_revision_working &&
-       entry->schedule == svn_wc_schedule_add) ||
+  if ((revision->kind != svn_opt_revision_working
+       && (entry->schedule == svn_wc_schedule_add
+           || entry->schedule == svn_wc_schedule_replace)
+       && !entry->copied) ||
       (revision->kind == svn_opt_revision_working &&
        entry->schedule == svn_wc_schedule_delete))
     return SVN_NO_ERROR;

Modified: subversion/trunk/subversion/libsvn_wc/adm_crawler.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_crawler.c?rev=921556&r1=921555&r2=921556&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/adm_crawler.c (original)
+++ subversion/trunk/subversion/libsvn_wc/adm_crawler.c Wed Mar 10 20:37:13 2010
@@ -74,6 +74,9 @@ restore_file(svn_wc__db_t *db,
 
   SVN_ERR(svn_wc__get_pristine_contents(&src_stream, db, local_abspath, pool,
                                         pool));
+  if (src_stream == NULL)
+    /* Nothing to restore. */
+    return SVN_NO_ERROR;
 
   SVN_ERR(svn_wc__get_special(&special, db, local_abspath, pool));
   if (special)
@@ -1175,6 +1178,8 @@ svn_wc__internal_transmit_text_deltas(co
       /* Compute delta against the pristine contents */
       SVN_ERR(svn_wc__get_pristine_contents(&base_stream, db, local_abspath,
                                             scratch_pool, scratch_pool));
+      if (base_stream == NULL)
+        base_stream = svn_stream_empty(scratch_pool);
 
       SVN_ERR(svn_wc__db_read_info(NULL, NULL, NULL,
                                    NULL, NULL, NULL,
@@ -1206,6 +1211,9 @@ svn_wc__internal_transmit_text_deltas(co
           /* ### we should ALREADY have the checksum for pristine. */
           SVN_ERR(svn_wc__get_pristine_contents(&p_stream, db, local_abspath,
                                                scratch_pool, scratch_pool));
+          if (p_stream == NULL)
+            p_stream = svn_stream_empty(scratch_pool);
+
           p_stream = svn_stream_checksummed2(p_stream, &p_checksum,
                                              NULL, svn_checksum_md5, TRUE,
                                              scratch_pool);

Modified: subversion/trunk/subversion/libsvn_wc/copy.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/copy.c?rev=921556&r1=921555&r2=921556&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/copy.c (original)
+++ subversion/trunk/subversion/libsvn_wc/copy.c Wed Mar 10 20:37:13 2010
@@ -402,9 +402,16 @@ copy_file_administratively(svn_wc_contex
                             scratch_pool, scratch_pool));
 
   /* Sanity check 2: You cannot make a copy of something that's not
-     in the repository unless it's a copy of an uncommitted copy. */
-  if ((src_entry->schedule == svn_wc_schedule_add && (! src_entry->copied))
-      || (! src_entry->url))
+     in the repository unless it's a copy of an uncommitted copy.
+     Added files don't have a base, but replaced files have a revert-base.
+     ### TODO: svn_opt_revision_base currently means "commit-base", which
+     ### technically is none for replaced files. We currently have no way to
+     ### get at the revert-base and need a new svn_opt_revision_X for that.
+   */
+  if (((src_entry->schedule == svn_wc_schedule_add
+        || src_entry->schedule == svn_wc_schedule_replace)
+       && (!src_entry->copied))
+      || (!src_entry->url))
     return svn_error_createf
       (SVN_ERR_UNSUPPORTED_FEATURE, NULL,
        _("Cannot copy or move '%s': it is not in the repository yet; "
@@ -484,6 +491,10 @@ copy_file_administratively(svn_wc_contex
                                         err, NULL);
               else if (err)
                 return svn_error_return(err);
+              
+              /* Above add/replace condition should have caught this already
+               * (-> error "Cannot copy..."). */
+              SVN_ERR_ASSERT(contents != NULL);
             }
           else if (err)
             return svn_error_return(err);
@@ -513,6 +524,9 @@ copy_file_administratively(svn_wc_contex
 
     SVN_ERR(svn_wc_get_pristine_contents2(&base_contents, wc_ctx, src_abspath,
                                           scratch_pool, scratch_pool));
+    /* Above add/replace condition should have caught this already
+     * (-> error "Cannot copy..."). */
+    SVN_ERR_ASSERT(base_contents != NULL);
 
     SVN_ERR(svn_wc_add_repos_file4(wc_ctx, dst_abspath,
                                    base_contents, contents,

Modified: subversion/trunk/subversion/libsvn_wc/diff.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/diff.c?rev=921556&r1=921555&r2=921556&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/diff.c (original)
+++ subversion/trunk/subversion/libsvn_wc/diff.c Wed Mar 10 20:37:13 2010
@@ -1501,6 +1501,8 @@ apply_textdelta(void *file_baton,
       /* The current text-base is the starting point if replacing */
       SVN_ERR(svn_wc__get_pristine_contents(&source, eb->db, fb->local_abspath,
                                             fb->pool, fb->pool));
+      if (source == NULL)
+        source = svn_stream_empty(fb->pool);
     }
 
   /* This is the file that will contain the pristine repository version. It

Modified: subversion/trunk/subversion/libsvn_wc/questions.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/questions.c?rev=921556&r1=921555&r2=921556&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/questions.c (original)
+++ subversion/trunk/subversion/libsvn_wc/questions.c Wed Mar 10 20:37:13 2010
@@ -369,6 +369,12 @@ svn_wc__internal_text_modified_p(svn_boo
 
   SVN_ERR(err);
 
+  if (pristine_stream == NULL)
+    {
+      *modified_p = TRUE;
+      return SVN_NO_ERROR;
+    }
+
   /* Check all bytes, and verify checksum if requested. */
   err = compare_and_verify(modified_p, db, local_abspath, pristine_stream,
                            compare_textbases, force_comparison,

Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=921556&r1=921555&r2=921556&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
+++ subversion/trunk/subversion/libsvn_wc/update_editor.c Wed Mar 10 20:37:13 2010
@@ -3834,6 +3834,11 @@ add_file_with_history(const char *path,
           SVN_ERR(svn_wc__get_pristine_contents(&source_text_base, db,
                                                 src_local_abspath,
                                                 subpool, subpool));
+
+          /* If this has no base, should we use an empty stream?
+           * This assert wants to verify that there are no such callers. */
+          SVN_ERR_ASSERT(source_text_base != NULL);
+          
           SVN_ERR(svn_wc__load_props(&base_props, &working_props, db,
                                      src_local_abspath, pool, subpool));
         }
@@ -4421,9 +4426,13 @@ apply_textdelta(void *file_baton,
                                             fb->local_abspath,
                                             handler_pool, handler_pool));
       else
-        SVN_ERR(svn_wc__get_pristine_contents(&source, fb->edit_baton->db,
-                                             fb->local_abspath,
-                                             handler_pool, handler_pool));
+        {
+          SVN_ERR(svn_wc__get_pristine_contents(&source, fb->edit_baton->db,
+                                                fb->local_abspath,
+                                                handler_pool, handler_pool));
+          if (source == NULL)
+            source = svn_stream_empty(handler_pool);
+        }
     }
   else
     {