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

Reply via email to