On Wed, Jan 19, 2011 at 7:36 AM, Daniel Shahaf <d...@daniel.shahaf.name>wrote:

> Daniel Becroft wrote on Wed, Jan 19, 2011 at 07:27:15 +1000:
> > On Wed, Jan 19, 2011 at 3:08 AM, Daniel Shahaf <d...@daniel.shahaf.name
> >wrote:
> > > Daniel Becroft wrote on Tue, Jan 18, 2011 at 07:13:12 +1000:
> > > > 2) The problem also exists under the '--reintegrate' scenario, even
> > > > after a subsequent commit. Would it be better to include that case
> > > > here, or move the entire test to the merge_reintegrate.py suite?
> > > >
> > >
> > > I'm not sure we need a separate test for the --reintegrate case;
> > > it seems reasonable to assume it does (and will) share the 'set +x
> > > permission' logic with normal merges.
> > >
> >
> > I thought about that, but this situation fixes itself after the 'svn
> > merge/svn commit' combination. However, after using --reintegrate. and
> > committing, the executable bit is still not set, so I think there might
> be
> > something more going on there.
> >
>
> Ah, if there's a different in the results between merge and reintegrate
> merge, then two tests are justifiable.  (I missed that part of your
> original comment.)
>

Thanks, Daniel. I've attached a new version of the patch, with amended log
message below.

Blair: I've also looked at *why* this problem exists, and it's due to the
"Special case" merge of binary files (libsvn_client/merge.c:1454). If (right
== working), then do a simple file copy from the temporary file (which is
not marked as +x). This means that we don't run svn_wc_merge4.
Unfortunately, this means that a workqueue is not created, and the
sync_file_flags is never run. As to what the fix is, I have no idea.

Regards,
Daniel B.

[[[
Add regression test for issue #3686.

This issue involves the executable flag being lost during a merge of a
binary file with the 'svn:executable' property set.

* subversion/tests/cmdline/merge_tests.py
  (merge_change_to_file_with_executable): New test case.
]]]
Index: merge_tests.py
===================================================================
--- merge_tests.py	(revision 1060764)
+++ merge_tests.py	(working copy)
@@ -16316,6 +16316,85 @@
     "Subtree merge under working merge produced the wrong mergeinfo",
     '/A/C/nu:9', [], 'pg', SVN_PROP_MERGEINFO, nu_COPY_path)
 
+
+#----------------------------------------------------------------------
+# Test for issue #3686 'executable flag not correctly set on merge'
+# See http://subversion.tigris.org/issues/show_bug.cgi?id=3686
+def merge_change_to_file_with_executable(sbox):
+  "executable flag is maintained during binary merge"
+
+  # Scenario: When merging a change to a binary file with the 'svn:executable'
+  # property set, the file is not marked as 'executable'. After commit, the 
+  # executable bit is set correctly.
+  sbox.build()
+  wc_dir = sbox.wc_dir
+  trunk_url = sbox.repo_url + '/A/B/E'
+
+  alpha_path = os.path.join(wc_dir, "A", "B", "E", "alpha")
+  beta_path = os.path.join(wc_dir, "A", "B", "E", "beta")
+
+  # Force one of the files to be a binary type
+  svntest.actions.run_and_verify_svn(None, None, [],
+                                     'propset', 'svn:mime-type', 'application/octet-stream',
+                                     alpha_path)
+
+  # Set the 'svn:executable' property on both files
+  svntest.actions.run_and_verify_svn(None, None, [],
+                                     'propset', 'svn:executable', 'ON',
+                                     beta_path)
+
+  svntest.actions.run_and_verify_svn(None, None, [],
+                                     'propset', 'svn:executable', 'ON',
+                                     alpha_path)
+
+  # Verify the executable bit has been set before committing
+  if not os.access(alpha_path, os.X_OK):
+    raise svntest.Failure("alpha not marked as executable before commit")
+  if not os.access(beta_path, os.X_OK):
+    raise svntest.Failure("beta is not marked as executable before commit")
+
+  # Commit change (r2)
+  sbox.simple_commit()
+
+  # Create the branch
+  svntest.actions.run_and_verify_svn(None, None, [], 'cp',
+                                     trunk_url,
+                                     sbox.repo_url + '/branch',
+                                     '-m', "Creating the Branch")   
+
+  # Modify the files + commit (r3)
+  svntest.main.file_append(alpha_path, 'appended alpha text')
+  svntest.main.file_append(beta_path, 'appended beta text')
+  sbox.simple_commit()
+
+  # Switch the WC to the branch
+  svntest.actions.run_and_verify_svn(None, None, [], 'switch',
+                                     sbox.repo_url + '/branch',
+                                     wc_dir)
+  
+  # Recalculate the paths
+  alpha_path = os.path.join(wc_dir, "alpha")
+  beta_path = os.path.join(wc_dir, "beta")
+
+  # Merge the changes across
+  svntest.actions.run_and_verify_svn(None, None, [], 'merge',
+                                     trunk_url, wc_dir)
+
+  # Verify the executable bit has been set
+  if not os.access(alpha_path, os.X_OK):
+    raise svntest.Failure("alpha is not marked as executable after merge")
+  if not os.access(beta_path, os.X_OK):
+    raise svntest.Failure("beta is not marked as executable after merge")
+
+  # Commit (r4)
+  sbox.simple_commit()
+
+  # Verify the executable bit has been set
+  if not os.access(alpha_path, os.X_OK):
+    raise svntest.Failure("alpha is not marked as executable after commit")
+  if not os.access(beta_path, os.X_OK):
+    raise svntest.Failure("beta is not marked as executable after commit")
+
 ########################################################################
 # Run the tests
 
@@ -16507,6 +16586,7 @@
               merge_with_os_deleted_subtrees,
               no_self_referential_or_nonexistent_inherited_mergeinfo,
               XFail(subtree_merges_inherit_invalid_working_mergeinfo),
+              XFail(SkipUnless(merge_change_to_file_with_executable, svntest.main.is_posix_os)),
              ]
 
 if __name__ == '__main__':

Reply via email to