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__':