Daniel Shahaf <d...@daniel.shahaf.name> writes:

> Noorul Islam K M wrote on Tue, May 24, 2011 at 15:39:10 +0530:
>
>> Philip Martin <philip.mar...@wandisco.com> writes:
>> 
>> > Noorul Islam K M <noo...@collab.net> writes:
>> >
>> >> Index: subversion/libsvn_client/export.c
>> >> ===================================================================
>> >> --- subversion/libsvn_client/export.c     (revision 1071880)
>> >> +++ subversion/libsvn_client/export.c     (working copy)
>> >> @@ -524,7 +524,26 @@
>> >>      }
>> >>    else if (from_kind == svn_node_file)
>> >>      {
>> >> +      svn_node_kind_t kind;
>> >> +      svn_error_t *err;
>> >> +
>> >>        SVN_ERR(append_basename_if_dir(&to_abspath, from_abspath, FALSE, 
>> >> pool));
>> >> +      svn_error_clear(svn_io_check_path(to_abspath, &kind, pool));
>> >> +
>> >> +      if ((kind == svn_node_file) && ! force)
>> >> +        {
>> >> +          return svn_error_createf(SVN_ERR_FS_ALREADY_EXISTS, NULL,
>> >> +                                   _("Destination file '%s' exists, "
>> >> +                                     "and will not be overwritten unless 
>> >> "
>> >> +                                     "forced"),
>> >> +                                   svn_dirent_local_style(to_abspath, 
>> >> pool));
>> >
>> > That's the wrong error, SVN_ERR_FS_ is for the Subversion filesystem in
>> > the repository.
>> >
>> 
>> I could see usage of the same at several places in libsvn_client.
>> 
>
> To indicate that a path doesn't exist in the repository, which isn't the
> case here.

As discussed on IRC updated patch to use SVN_ERR_ILLEGAL_TARGET.

Thanks and Regards
Noorul

Index: subversion/tests/cmdline/externals_tests.py
===================================================================
--- subversion/tests/cmdline/externals_tests.py (revision 1126953)
+++ subversion/tests/cmdline/externals_tests.py (working copy)
@@ -752,7 +752,8 @@
                                      repo_url, wc_dir)
   # Export the working copy.
   svntest.actions.run_and_verify_svn(None, None, [],
-                                     'export', wc_dir, export_target)
+                                     'export', '--force',
+                                     wc_dir, export_target)
 
   ### We should be able to check exactly the paths that externals_test_setup()
   ### set up; however, --ignore-externals fails to ignore 'A/B/gamma' so this
Index: subversion/tests/cmdline/export_tests.py
===================================================================
--- subversion/tests/cmdline/export_tests.py    (revision 1126953)
+++ subversion/tests/cmdline/export_tests.py    (working copy)
@@ -589,19 +589,19 @@
                                         '.', expected_output,
                                         expected_disk)
 
-@XFail()
 @Issue(3799)
 def export_file_overwrite_fails(sbox):
   "exporting a file refuses to silently overwrite"
   sbox.build(create_wc = True, read_only = True)
 
   iota_path = os.path.abspath(os.path.join(sbox.wc_dir, 'iota'))
+  iota_url = sbox.repo_url + '/iota'
   not_iota_contents = "This obstructs 'iota'.\n"
 
   tmpdir = sbox.get_tempname('file-overwrites')
   os.mkdir(tmpdir)
 
-  # Run it
+  # Run it for source local
   open(os.path.join(tmpdir, 'iota'), 'w').write(not_iota_contents)
   svntest.actions.run_and_verify_svn(None, [], '.*exist.*',
                                      'export', iota_path, tmpdir)
@@ -612,6 +612,17 @@
       })
   svntest.actions.verify_disk(tmpdir, expected_disk)
 
+  # Run it for source URL
+  open(os.path.join(tmpdir, 'iota'), 'w').write(not_iota_contents)
+  svntest.actions.run_and_verify_svn(None, [], '.*exist.*',
+                                     'export', iota_url, tmpdir)
+
+  # Verify it failed
+  expected_disk = svntest.wc.State('', {
+      'iota': Item(contents=not_iota_contents),
+      })
+  svntest.actions.verify_disk(tmpdir, expected_disk)
+
 def export_ignoring_keyword_translation(sbox):
   "export ignoring keyword translation"
   sbox.build()
@@ -867,6 +878,29 @@
 
   os.chdir(orig_dir)
 
+def export_file_overwrite_with_force(sbox):
+  "exporting a file with force option"
+  sbox.build(create_wc = True, read_only = True)
+
+  iota_path = os.path.abspath(os.path.join(sbox.wc_dir, 'iota'))
+  not_iota_contents = "This obstructs 'iota'.\n"
+  iota_contents = "This is the file 'iota'.\n"
+
+  tmpdir = sbox.get_tempname('file-overwrites')
+  os.mkdir(tmpdir)
+
+  expected_disk = svntest.wc.State('', {
+      'iota': Item(contents=iota_contents),
+      })
+
+  # Run it
+  open(os.path.join(tmpdir, 'iota'), 'w').write(not_iota_contents)
+  svntest.actions.run_and_verify_svn(None, svntest.verify.AnyOutput,
+                                     [], 'export', '--force',
+                                     iota_path, tmpdir)
+
+  svntest.actions.verify_disk(tmpdir, expected_disk)
+
 ########################################################################
 # Run the tests
 
@@ -899,6 +933,7 @@
               export_working_copy_with_depths,
               export_externals_with_native_eol,
               export_to_current_dir,
+              export_file_overwrite_with_force,
              ]
 
 if __name__ == '__main__':
Index: subversion/libsvn_client/export.c
===================================================================
--- subversion/libsvn_client/export.c   (revision 1126953)
+++ subversion/libsvn_client/export.c   (working copy)
@@ -569,6 +569,25 @@
     }
   else if (from_kind == svn_node_file)
     {
+      svn_node_kind_t kind;
+
+      svn_error_clear(svn_io_check_path(to_abspath, &kind, pool));
+
+      if ((kind == svn_node_file || kind == svn_node_unknown) && ! force)
+        {
+          return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL,
+                                   _("Destination file '%s' exists, "
+                                     "and will not be overwritten unless "
+                                     "forced"),
+                                   svn_dirent_local_style(to_abspath, pool));
+        }
+      else if (kind == svn_node_dir)
+        return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL,
+                                 _("Destination %s exists. Cannot overwrite "
+                                   "directory with non-directory"),
+                                 svn_dirent_local_style(to_abspath, pool));
+
+
       SVN_ERR(copy_one_versioned_file(from_abspath, to_abspath, ctx,
                                       revision, native_eol, ignore_keywords,
                                       pool));
@@ -1064,6 +1083,7 @@
           apr_hash_t *props;
           apr_hash_index_t *hi;
           struct file_baton *fb = apr_pcalloc(pool, sizeof(*fb));
+          svn_node_kind_t to_kind;
 
           if (svn_path_is_empty(to_path))
             {
@@ -1080,7 +1100,23 @@
               eb->root_path = to_path;
             }
 
+          svn_error_clear(svn_io_check_path(to_path, &to_kind, pool));
 
+          if ((to_kind == svn_node_file || to_kind == svn_node_unknown) &&
+              ! overwrite)
+            {
+              return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL,
+                                       _("Destination file '%s' exists, "
+                                         "and will not be overwritten unless "
+                                         "forced"),
+                                       svn_dirent_local_style(to_path, pool));
+            }
+          else if (to_kind == svn_node_dir)
+            return svn_error_createf(SVN_ERR_ILLEGAL_TARGET, NULL,
+                                     _("Destination %s exists. Cannot 
overwrite "
+                                       "directory with non-directory"),
+                                     svn_dirent_local_style(to_path, pool));
+
           /* Since you cannot actually root an editor at a file, we
            * manually drive a few functions of our editor. */
 

Reply via email to