Noorul Islam K M <noo...@collab.net> writes: > Noorul Islam K M <noo...@collab.net> writes: > >> Noorul Islam K M <noo...@collab.net> writes: >> >>> This patch is a followup of the following thread. All tests pass with >>> this patch. >>> >>> http://svn.haxx.se/dev/archive-2011-01/0210.shtml >>> >>> Log >>> >>> [[[ >>> >>> Make svn 'cat' command to return 1 when one or more targets fails. Also >>> print error message at the end. This also fixes issue #3713. >>> >>> * 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 >>> >>> Patch by: Noorul Islam K M <noorul{_AT_}collab.net> >>> >>> ]]] >>> >>> Thanks and Regards >>> Noorul >>> >>> Index: subversion/tests/cmdline/cat_tests.py >>> =================================================================== >>> --- subversion/tests/cmdline/cat_tests.py (revision 1060693) >>> +++ subversion/tests/cmdline/cat_tests.py (working copy) >>> @@ -25,7 +25,7 @@ >>> ###################################################################### >>> >>> # General modules >>> -import os >>> +import os, re >>> >>> # Our testing module >>> import svntest >>> @@ -50,20 +50,23 @@ >>> sbox.build(read_only = True) >>> >>> A_path = os.path.join(sbox.wc_dir, 'A') >>> + expected_err = "svn: warning: '" + os.path.abspath(A_path) + "'" + \ >>> + " refers to a directory\n.*" >>> >>> - svntest.actions.run_and_verify_svn2('No error where one is expected', >>> - None, svntest.verify.AnyOutput, >>> - 0, 'cat', A_path) >>> + 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: 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" >>> sbox.build(read_only = True) >>> @@ -88,10 +91,13 @@ >>> 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: '" + 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" >>> sbox.build(read_only = True) >>> @@ -113,14 +119,14 @@ >>> continue >>> item_to_cat = os.path.join(dir_path, file) >>> if item_to_cat == new_file_path: >>> - expected_err = ["svn: warning: '" + os.path.abspath(item_to_cat) + >>> "'" + \ >>> - " is not under version control\n"] >>> - svntest.actions.run_and_verify_svn2(None, None, expected_err, 0, >>> + expected_err = "svn: warning: '" + 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: '" + os.path.abspath(item_to_cat) + >>> "'" + \ >>> - " refers to a directory\n"] >>> - svntest.actions.run_and_verify_svn2(None, None, expected_err, 0, >>> + expected_err = "svn: warning: '" + 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, >>> @@ -130,22 +136,33 @@ >>> 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: '" + os.path.abspath(G_path) + "'" >>> - + " refers to a directory\n"] >>> - svntest.actions.run_and_verify_svn2(None, expected_out, expected_err1, 0, >>> + expected_out = "This is the file 'rho'.\n" >>> + expected_err1 = "svn: warning: '" + 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: '" + os.path.abspath(new_file_path) + "'" >>> - + " is not under version control\n"] >>> - svntest.actions.run_and_verify_svn2(None, expected_out, expected_err2, 0, >>> + expected_err2 = "svn: warning: '" + 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) >>> >>> - svntest.actions.run_and_verify_svn2(None, expected_out, >>> - expected_err1 + expected_err2, 0, >>> - 'cat', rho_path, G_path, >>> new_file_path) >>> + expected_err3 = expected_err1 + expected_err2 + ".*\n" + \ >>> + "svn: 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'. >>> def cat_unversioned_file(sbox): >>> @@ -162,15 +179,15 @@ >>> iota_path) >>> >>> # Now try to cat the deleted file, it should be reported as unversioned. >>> - expected_error = ["svn: warning: '" + os.path.abspath(iota_path) + "'" >>> - + " is not under version control\n"] >>> - svntest.actions.run_and_verify_svn2(None, [], expected_error, 0, >>> + expected_error = "svn: warning: '" + 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) >>> >>> >>> @@ -204,13 +221,25 @@ >>> special_urls = [sbox.repo_url + '/A' + '/%2E', >>> sbox.repo_url + '%2F' + 'A'] >>> >>> - expected_err = ["svn: warning: URL '" + sbox.repo_url + '/A' + "'" >>> - + " refers to a directory\n"] >>> + expected_err = "svn: warning: 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, >>> + svntest.actions.run_and_verify_svn2(None, None, expected_err, 1, >>> 'cat', url) >>> >>> +def cat_non_existing_remote_file(sbox): >>> + """cat non exising remote file""" >>> + sbox.build(create_wc = False) >>> + non_existing_path = sbox.repo_url + '/non-existing' >>> + >>> + expected_err = "svn: warning: 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 >>> >>> @@ -225,6 +254,7 @@ >>> cat_unversioned_file, >>> cat_keywords, >>> cat_url_special_characters, >>> + cat_non_existing_remote_file, >>> ] >>> >>> if __name__ == '__main__': >>> Index: subversion/svn/cat-cmd.c >>> =================================================================== >>> --- subversion/svn/cat-cmd.c (revision 1060693) >>> +++ subversion/svn/cat-cmd.c (working copy) >>> @@ -47,6 +47,7 @@ >>> 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 @@ >>> 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_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; >>> } >> >> It will be great if someone could review this so that I can proceed with >> other commands. >> > > Ping. Can someone take a look at this? >
Will this patch be getting any attention? Or is this not important? Thanks and Regards Noorul