svn-3665

Version:

1.6.11

Bug Link:

http://subversion.tigris.org/issues/show_bug.cgi?id=3665

http://svn.haxx.se/users/archive-2010-06/0197.shtml

Patch Link:

http://svn.apache.org/viewvc?diff_format=h&view=revision&revision=1030912

Symptom:

update a working copy with versioned file externals, files where the version is not equal to the last revision of the file, are double-updated on _every_ update.

In other words, the file externals come from the same repository as the working copy, AND we are not checking out the latest revision. When both these 2 conditions are met, SVN will update the file twice instead of only once.

The problem with this double updates for the reporter is that it changes the file timestamps everytime and thus triggers Makefile actions.

How to reproduce:

[peh003@soprano external]$ svn up

U    trunk/combined-src/file1.c /*update once*/


Fetching external item into 'trunk/combined-src/file1.c'
U    trunk/combined-src/file1.c /*update twice*/
Updated external to revision 4.



Fetching external item into 'trunk/combined-src/file2.c'
Updated external to revision 4.
/*no actual update on file.*/

Updated to revision 6.

Expected result

[peh003@soprano external]$ svn up

Updating '.' ...

Fetching external item into 'trunk/combined-src/file1.c'

External at revision 4.

Fetching external item into 'trunk/combined-src/file2.c'

External at revision 4.

At revision 6.

/**For later explaination**/

[peh003@soprano external]$ svn pget -v svn:externals -R .

Properties on 'trunk/combined-src':

  svn:externals

    ^/trunk/src1/file1.c@4 file1.c

    ^/trunk/src2/file2.c@4 file2.c

[peh003@soprano trunk]$ cd src1/

[peh003@soprano src1]$ svn info file1.c

...

Last Changed Rev: 5

...

[peh003@soprano src1]$ svn diff -r PREV:COMMITTED file1.c

Index: file1.c

===================================================================

--- file1.c        (revision 4)

+++ file1.c        (revision 5)

@@ -1 +1 @@

-ver 1

+ver 2

[peh003@soprano trunk]$ cd ../src2/

[peh003@soprano combined-src]$ svn info file2.c

...

Last Changed Rev: 4

...

Background:
External files/directories in svn

http://svnbook.red-bean.com/en/1.0/ch07s03.html

Root Cause:

Brief:

They did not handle the case where file externals are in the same repository as working copy.  

Should not report file externals as switched items when they are crawled as part
of a crawl of their parent directory(only report when the file externals are crawled explicitly), his should would cause the server to stop sending updates for them.

Detail:

As for why we should not mark the file externals in the condition that it is part of a crawling request of its parent,

Since the mechanism of updates with file externals happen in two passes:  

The first pass -- the update of the primary working copy -- actually changes "combined-src/file1.c" to it's r6 state.  (You can verify this by running 'svn up --ignore-externals').  Then the second pass comes along and undoes that very
update to "combined-src/file1.c", putting it back into its r4 state.

The difference between file1 and file2’s output is because of their LastChangedRev .

file2 has 4 as the LastChangedRev and the svn:externals has also rev 4.

file1 on the other hand has a LastChangedRev of 5, which *differs* from the rev 4 in svn:externals.
It is this difference that causes the problem.

/* The recursive crawler that describes a mixed-revision working
  copy to an RA layer.  Used to initiate updates.
*/
static svn_error_t *
report_revisions_and_depths(...)
{
 …
 /* Looping over current directory's BASE children: */
 for (i = 0; i < base_children->nelts; ++i)
 // base_children->nelts =3, when it comes to “trunk”
 {

      //in first iteration for “trunk”,  child = “combined-src”
     const char *child = APR_ARRAY_IDX(base_children, i, const char *);
     ...
-     svn_boolean_t this_switched;
+    svn_boolean_t this_switched = FALSE;
+    svn_boolean_t this_file_external = FALSE;


         const char *childname = svn_relpath_is_child(dir_repos_relpath,
                                                      this_repos_relpath,
                                                      NULL);

 /* Here, they did not even consider the case where file is external. The patch is to

    simply add the handling of external files. */

          // childname = “combined-src”, child = “combined-src”

         // and then a later recursive call come here, childname = NULL, child = “file1.c”
         if (childname == NULL || strcmp(childname, child) != 0)
-            this_switched = TRUE;
-          else
-            this_switched = FALSE;

+            {
+              const char *file_ext_str;
+              this_switched = TRUE;
+              /* This could be a file external!  We need to know
+                 that. */
+              SVN_ERR(svn_wc__db_temp_get_file_external(&file_ext_str, db,
+                                                        this_abspath,
+                                                        scratch_pool,
+                                                        scratch_pool));
+              if (file_ext_str)
+                this_file_external = TRUE;
// and then this will be set true
+            }

       }
       ...
+      /*** File Externals **/
+      if (this_file_external)
//we’ll do nothing for combined-src/file1.c
+        {
+          /* File externals are ... special.  We ignore them. */;
+        }

/*this_kind = svn_wc__db_kind_dir*/

-      if (this_kind == svn_wc__db_kind_file ||
-          this_kind == svn_wc__db_kind_symlink)
+      else if (this_kind == svn_wc__db_kind_file ||
+               this_kind == svn_wc__db_kind_symlink)

{
         if (report_everything)
           {
             /* Report the file unconditionally, one way or another. */
                …

}

}

     else if (this_kind == svn_wc__db_kind_dir
              && (depth > svn_depth_files
                  || depth == svn_depth_unknown))
       {

                …

          /* Finally, recurse if necessary and appropriate. */
         if (SVN_DEPTH_IS_RECURSIVE(depth))
           SVN_ERR(report_revisions_and_depths(...));

         }

     }
  return SVN_NO_ERROR;

}

Failure symptom category

Unnecessary computation.

Is there any log message?

No

Can ErrLog help?

No. They did not even check and handle the external file condition.