You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by cm...@apache.org on 2011/02/09 18:07:19 UTC

svn commit: r1068977 - in /subversion/trunk/subversion: svn/cat-cmd.c tests/cmdline/cat_tests.py

Author: cmpilato
Date: Wed Feb  9 17:07:19 2011
New Revision: 1068977

URL: http://svn.apache.org/viewvc?rev=1068977&view=rev
Log:
Fix issue #3713 ("svn cat for non-existing file returns incorrect
exit-status").  Make 'svn cat' return a non-zero errorcode when one or
more targets fails.  Also print an error message in those situations.

* subversion/svn/cat-cmd.c
  (svn_cl__cat): Pass SVN_ERR_FS_NOT_FOUND to svn_cl__try in order to
    catch this error when processing non existent URL targets, print
    warning and proceed with other targets. Print error message at the
    end if operation fails on any one of the targets and return 1.

* subversion/tests/cmdline/cat_tests.py
  (cat_local_directory, cat_nonexistent_file, cat_skip_uncattable,
   cat_unversioned_file, cat_url_special_characters):
    Update return code and use regular expressions to match output.
  (cat_non_existing_remote_file): New test.
  (test_list): Add reference to new test.

Patch by: Noorul Islam K M <no...@collab.net>
          (Tweaked by me.)

Modified:
    subversion/trunk/subversion/svn/cat-cmd.c
    subversion/trunk/subversion/tests/cmdline/cat_tests.py

Modified: subversion/trunk/subversion/svn/cat-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/cat-cmd.c?rev=1068977&r1=1068976&r2=1068977&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/cat-cmd.c (original)
+++ subversion/trunk/subversion/svn/cat-cmd.c Wed Feb  9 17:07:19 2011
@@ -47,6 +47,7 @@ svn_cl__cat(apr_getopt_t *os,
   int i;
   svn_stream_t *out;
   apr_pool_t *subpool = svn_pool_create(pool);
+  svn_boolean_t saw_a_problem = FALSE;
 
   SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
                                                       opt_state->targets,
@@ -63,6 +64,7 @@ svn_cl__cat(apr_getopt_t *os,
       const char *target = APR_ARRAY_IDX(targets, i, const char *);
       const char *truepath;
       svn_opt_revision_t peg_revision;
+      svn_boolean_t success;
 
       svn_pool_clear(subpool);
       SVN_ERR(svn_cl__check_cancel(ctx->cancel_baton));
@@ -74,13 +76,19 @@ svn_cl__cat(apr_getopt_t *os,
       SVN_ERR(svn_cl__try(svn_client_cat2(out, truepath, &peg_revision,
                                           &(opt_state->start_revision),
                                           ctx, subpool),
-                           NULL, opt_state->quiet,
+                           &success, opt_state->quiet,
                            SVN_ERR_UNVERSIONED_RESOURCE,
                            SVN_ERR_ENTRY_NOT_FOUND,
                            SVN_ERR_CLIENT_IS_DIRECTORY,
+                           SVN_ERR_FS_NOT_FOUND,
                            SVN_NO_ERROR));
+      if (! success)
+        saw_a_problem = TRUE;
     }
   svn_pool_destroy(subpool);
 
-  return SVN_NO_ERROR;
+  if (saw_a_problem)
+    return svn_error_create(SVN_ERR_BASE, NULL, NULL);
+  else
+    return SVN_NO_ERROR;
 }

Modified: subversion/trunk/subversion/tests/cmdline/cat_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/cat_tests.py?rev=1068977&r1=1068976&r2=1068977&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/cat_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/cat_tests.py Wed Feb  9 17:07:19 2011
@@ -25,7 +25,7 @@
 ######################################################################
 
 # General modules
-import os
+import os, re
 
 # Our testing module
 import svntest
@@ -55,18 +55,22 @@ def cat_local_directory(sbox):
 
   A_path = os.path.join(sbox.wc_dir, 'A')
 
-  svntest.actions.run_and_verify_svn2('No error where one is expected',
-                                      None, svntest.verify.AnyOutput,
-                                      0, 'cat', A_path)
+  expected_err = "svn: warning: W195007: '" + os.path.abspath(A_path) + \
+      "' refers to a directory\n.*"
+
+  svntest.actions.run_and_verify_svn2(None, None, expected_err,
+                                      1, 'cat', A_path)
 
 def cat_remote_directory(sbox):
   "cat a remote directory"
   sbox.build(create_wc = False, read_only = True)
 
   A_url = sbox.repo_url + '/A'
-  svntest.actions.run_and_verify_svn2('No error where one is expected',
-                                      None, svntest.verify.AnyOutput,
-                                      0, 'cat', A_url)
+  expected_err = "svn: warning: W195007: URL '" + A_url + \
+      "' refers to a directory\n.*"
+
+  svntest.actions.run_and_verify_svn2(None, None, expected_err,
+                                      1, 'cat', A_url)
 
 def cat_base(sbox):
   "cat a file at revision BASE"
@@ -92,9 +96,11 @@ def cat_nonexistent_file(sbox):
   wc_dir = sbox.wc_dir
 
   bogus_path = os.path.join(wc_dir, 'A', 'bogus')
-  svntest.actions.run_and_verify_svn2('No error where one is expected',
-                                      None, svntest.verify.AnyOutput,
-                                      0, 'cat', bogus_path)
+  expected_err = "svn: warning: W200005: '" + os.path.abspath(bogus_path) + \
+      "' is not under version control\n.*"
+
+  svntest.actions.run_and_verify_svn2(None, None, expected_err, 1,
+                                      'cat', bogus_path)
 
 def cat_skip_uncattable(sbox):
   "cat should skip uncattable resources"
@@ -117,15 +123,16 @@ def cat_skip_uncattable(sbox):
       continue
     item_to_cat = os.path.join(dir_path, file)
     if item_to_cat == new_file_path:
-      expected_err = ["svn: warning: W200005: '" + os.path.abspath(item_to_cat) + "'" + \
-                     " is not under version control\n"]
-      svntest.actions.run_and_verify_svn2(None, None, expected_err, 0,
-                                          'cat', item_to_cat)
+      expected_err = "svn: warning: W200005: '" + os.path.abspath(item_to_cat) + "'" + \
+          " is not under version control\n.*"
+      svntest.actions.run_and_verify_svn2(None, None, expected_err, 1,
+                                           'cat', item_to_cat)
+
     elif os.path.isdir(item_to_cat):
-      expected_err = ["svn: warning: W195007: '" + os.path.abspath(item_to_cat) + "'" + \
-                     " refers to a directory\n"]
-      svntest.actions.run_and_verify_svn2(None, None, expected_err, 0,
-                                          'cat', item_to_cat)
+      expected_err = "svn: warning: W195007: '" + \
+          os.path.abspath(item_to_cat) + "' refers to a directory\n.*"
+      svntest.actions.run_and_verify_svn2(None, None, expected_err, 1,
+                                           'cat', item_to_cat)
     else:
       svntest.actions.run_and_verify_svn(None,
                                          ["This is the file '"+file+"'.\n"],
@@ -134,21 +141,32 @@ def cat_skip_uncattable(sbox):
   G_path = os.path.join(dir_path, 'G')
   rho_path = os.path.join(G_path, 'rho')
 
-  expected_out = ["This is the file 'rho'.\n"]
-  expected_err1 = ["svn: warning: W195007: '" + os.path.abspath(G_path) + "'"
-                   + " refers to a directory\n"]
-  svntest.actions.run_and_verify_svn2(None, expected_out, expected_err1, 0,
-                                      'cat', rho_path, G_path)
-
-  expected_err2 = ["svn: warning: W200005: '" + os.path.abspath(new_file_path) + "'"
-                   + " is not under version control\n"]
-  svntest.actions.run_and_verify_svn2(None, expected_out, expected_err2, 0,
-                                      'cat', rho_path, new_file_path)
-
-  svntest.actions.run_and_verify_svn2(None, expected_out,
-                                      expected_err1 + expected_err2, 0,
-                                      'cat', rho_path, G_path, new_file_path)
+  expected_out = "This is the file 'rho'.\n"
+  expected_err1 = "svn: warning: W195007: '" + os.path.abspath(G_path) + \
+      "' refers to a directory\n"
+  svntest.actions.run_and_verify_svn2(None, expected_out, expected_err1, 1,
+                                       'cat', rho_path, G_path)
+
+  expected_err2 = "svn: warning: W200005: '" + os.path.abspath(new_file_path) + \
+      "' is not under version control\n"
+  svntest.actions.run_and_verify_svn2(None, expected_out, expected_err2, 1,
+                                       'cat', rho_path, new_file_path)
+
+  expected_err3 = expected_err1 + expected_err2 + ".*\n" + \
+      "svn: E200000: A problem occurred; see other errors for details\n"
+  expected_err_re = re.compile(expected_err3)
+
+  exit_code, output, error = svntest.main.run_svn(1, 'cat', rho_path, G_path, new_file_path)
 
+  # Verify output
+  if output[0] != expected_out:
+    raise svntest.Failure('Cat failed: expected "%s", but received "%s"' % \
+                          (expected_out, "".join(output)))
+
+  # Verify error
+  if not expected_err_re.match("".join(error)):
+    raise svntest.Failure('Cat failed: expected error "%s", but received "%s"' % \
+                          (expected_err3, "".join(error)))
 
 # Test for issue #3560 'svn_wc_status3() returns incorrect status for
 # unversioned files'.
@@ -167,18 +185,17 @@ def cat_unversioned_file(sbox):
                                       iota_path)
 
   # Now try to cat the deleted file, it should be reported as unversioned.
-  expected_error = ["svn: warning: W200005: '" + os.path.abspath(iota_path) + "'"
-                    + " is not under version control\n"]
-  svntest.actions.run_and_verify_svn2(None, [], expected_error, 0,
-                                      'cat', iota_path)
+  expected_error = "svn: warning: W200005: '" + os.path.abspath(iota_path) + \
+      "' is not under version control\n.*"
+  svntest.actions.run_and_verify_svn2(None, [], expected_error, 1,
+                                       'cat', iota_path)
 
   # Put an unversioned file at 'iota' and try to cat it again, the result
   # should still be the same.
   svntest.main.file_write(iota_path, "This the unversioned file 'iota'.\n")
-  svntest.actions.run_and_verify_svn2(None, [], expected_error, 0,
+  svntest.actions.run_and_verify_svn2(None, [], expected_error, 1,
                                       'cat', iota_path)
 
-
 def cat_keywords(sbox):
   "cat a file with the svn:keywords property"
   sbox.build()
@@ -209,12 +226,24 @@ def cat_url_special_characters(sbox):
   special_urls = [sbox.repo_url + '/A' + '/%2E',
                   sbox.repo_url + '%2F' + 'A']
 
-  expected_err = ["svn: warning: W195007: URL '" + sbox.repo_url + '/A'  + "'"
-                   + " refers to a directory\n"]
+  expected_err = "svn: warning: W195007: URL '" + sbox.repo_url + '/A' + \
+      "' refers to a directory\n.*"
 
   for url in special_urls:
-    svntest.actions.run_and_verify_svn2(None, None, expected_err, 0,
-                                        'cat', url)
+    svntest.actions.run_and_verify_svn2(None, None, expected_err, 1,
+                                         'cat', url)
+
+def cat_non_existing_remote_file(sbox):
+  """cat non-existing remote file"""
+  sbox.build(create_wc = False)
+  non_existing_path = sbox.repo_url + '/non-existing'
+  
+  expected_err = "svn: warning: W160013: File not found.*" + \
+      non_existing_path.split('/')[1]
+
+  # cat operation on non-existing remote path should return 1
+  svntest.actions.run_and_verify_svn2(None, None, expected_err, 1,
+                                      'cat', non_existing_path)
 
 ########################################################################
 # Run the tests
@@ -230,6 +259,7 @@ test_list = [ None,
               cat_unversioned_file,
               cat_keywords,
               cat_url_special_characters,
+              cat_non_existing_remote_file,
              ]
 
 if __name__ == '__main__':



Re: svn commit: r1068977 - in /subversion/trunk/subversion: svn/cat-cmd.c tests/cmdline/cat_tests.py

Posted by Noorul Islam K M <no...@collab.net>.
"Bert Huijben" <be...@qqmail.nl> writes:

>> -----Original Message-----
>> From: cmpilato@apache.org [mailto:cmpilato@apache.org]
>> Sent: woensdag 9 februari 2011 18:07
>> To: commits@subversion.apache.org
>> Subject: svn commit: r1068977 - in /subversion/trunk/subversion:
>> svn/cat-cmd.c tests/cmdline/cat_tests.py
>> 
>> Author: cmpilato
>> Date: Wed Feb  9 17:07:19 2011
>> New Revision: 1068977
>> 
>> URL: http://svn.apache.org/viewvc?rev=1068977&view=rev
>> Log:
>> Fix issue #3713 ("svn cat for non-existing file returns incorrect
>> exit-status").  Make 'svn cat' return a non-zero errorcode when one or
>> more targets fails.  Also print an error message in those situations.
>> 
>> * subversion/svn/cat-cmd.c
>>   (svn_cl__cat): Pass SVN_ERR_FS_NOT_FOUND to svn_cl__try in order to
>>     catch this error when processing non existent URL targets, print
>>     warning and proceed with other targets. Print error message at the
>>     end if operation fails on any one of the targets and return 1.
>> 
>> * subversion/tests/cmdline/cat_tests.py
>>   (cat_local_directory, cat_nonexistent_file, cat_skip_uncattable,
>>    cat_unversioned_file, cat_url_special_characters):
>>     Update return code and use regular expressions to match output.
>>   (cat_non_existing_remote_file): New test.
>>   (test_list): Add reference to new test.
>> 
>> Patch by: Noorul Islam K M <no...@collab.net>
>>           (Tweaked by me.)
>
> FAIL:  cat_tests.py 1: cat a local directory
> FAIL:  cat_tests.py 4: cat a nonexistent file
> FAIL:  cat_tests.py 5: cat should skip uncattable resources
> FAIL:  cat_tests.py 6: cat an unversioned file parent dir thinks exists
>
> These tests fail on all ra-layers and there are a few more errors on the layers that have authorization support.
>
>
> Noorul, can you run at least the ra_local tests before sending a patch?
>

I ran 'make check' and it did not complain. Doesn't that cover ra_local?

Thanks and Regards
Noorul

> (It would be nicer if you could test all relevant ra layers, but that is a bit too much to ask)
>
> 	Bert

Re: svn commit: r1068977 - in /subversion/trunk/subversion: svn/cat-cmd.c tests/cmdline/cat_tests.py

Posted by "C. Michael Pilato" <cm...@collab.net>.
On 02/10/2011 04:52 AM, Bert Huijben wrote:
> FAIL:  cat_tests.py 1: cat a local directory
> FAIL:  cat_tests.py 4: cat a nonexistent file
> FAIL:  cat_tests.py 5: cat should skip uncattable resources
> FAIL:  cat_tests.py 6: cat an unversioned file parent dir thinks exists
> 
> These tests fail on all ra-layers and there are a few more errors on the layers that have authorization support.
>
> Noorul, can you run at least the ra_local tests before sending a patch?
> 
> (It would be nicer if you could test all relevant ra layers, but that is a bit too much to ask)

Whoa, hold on just a second.  Noorul is not to blame here.  Patches are
transmitted against a baseline code snapshot that isn't guaranteed to remain
unchanged between patch submission and patch commit times.  As the reviewer
and committer of the patch, it was *my* job to make sure that it didn't
break anything.

And as it turns out, I thought I did my job well enough.  I ran a full 'make
check' plus executed cat_tests.py over both svn:// and http:// before
committing this patch -- all without a single meaningful failure.  (I *was*
seeing an XPASS in the diff-diff3-test C tests, but that was clearly
unrelated.)  So I really have no clue about what went wrong here.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


RE: svn commit: r1068977 - in /subversion/trunk/subversion: svn/cat-cmd.c tests/cmdline/cat_tests.py

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: cmpilato@apache.org [mailto:cmpilato@apache.org]
> Sent: woensdag 9 februari 2011 18:07
> To: commits@subversion.apache.org
> Subject: svn commit: r1068977 - in /subversion/trunk/subversion:
> svn/cat-cmd.c tests/cmdline/cat_tests.py
> 
> Author: cmpilato
> Date: Wed Feb  9 17:07:19 2011
> New Revision: 1068977
> 
> URL: http://svn.apache.org/viewvc?rev=1068977&view=rev
> Log:
> Fix issue #3713 ("svn cat for non-existing file returns incorrect
> exit-status").  Make 'svn cat' return a non-zero errorcode when one or
> more targets fails.  Also print an error message in those situations.
> 
> * subversion/svn/cat-cmd.c
>   (svn_cl__cat): Pass SVN_ERR_FS_NOT_FOUND to svn_cl__try in order to
>     catch this error when processing non existent URL targets, print
>     warning and proceed with other targets. Print error message at the
>     end if operation fails on any one of the targets and return 1.
> 
> * subversion/tests/cmdline/cat_tests.py
>   (cat_local_directory, cat_nonexistent_file, cat_skip_uncattable,
>    cat_unversioned_file, cat_url_special_characters):
>     Update return code and use regular expressions to match output.
>   (cat_non_existing_remote_file): New test.
>   (test_list): Add reference to new test.
> 
> Patch by: Noorul Islam K M <no...@collab.net>
>           (Tweaked by me.)

FAIL:  cat_tests.py 1: cat a local directory
FAIL:  cat_tests.py 4: cat a nonexistent file
FAIL:  cat_tests.py 5: cat should skip uncattable resources
FAIL:  cat_tests.py 6: cat an unversioned file parent dir thinks exists

These tests fail on all ra-layers and there are a few more errors on the layers that have authorization support.


Noorul, can you run at least the ra_local tests before sending a patch?

(It would be nicer if you could test all relevant ra layers, but that is a bit too much to ask)

	Bert