Hi Noorul,
comments inline:
On Fri, Feb 11, 2011 at 12:49:04PM +0530, Noorul Islam K M wrote:
> ===================================================================
> --- subversion/tests/cmdline/basic_tests.py (revision 1068757)
> +++ subversion/tests/cmdline/basic_tests.py (working copy)
> @@ -2690,6 +2690,85 @@
> [], 'ls',
> url)
>
> +def ls_non_existent_wc_target(sbox):
> + "ls a non-existent wc target"
> +
> + sbox.build(read_only = True)
> + wc_dir = sbox.wc_dir
> +
> + non_existent_path = os.path.join(wc_dir, 'non-existent')
> +
> + expected_err = "svn: warning: W155010: The node '" + \
> + re.escape(os.path.abspath(non_existent_path)) + "' was not found"
> +
> + svntest.actions.run_and_verify_svn2(None, None, expected_err,
> + 1, 'ls', non_existent_path)
> +
> +def ls_non_existent_url_target(sbox):
> + "ls a non-existent url target"
> +
> + sbox.build(read_only = True, create_wc = False)
> +
> + non_existent_url = sbox.repo_url + '/non-existent'
> + expected_err = "svn: warning: W160013: .*"
> +
> + svntest.actions.run_and_verify_svn2(None, None, expected_err,
> + 1, 'ls', non_existent_url)
> +
> +def ls_multiple_wc_targets(sbox):
> + "ls multiple wc targets"
> +
> + sbox.build(read_only = True)
> + wc_dir = sbox.wc_dir
> +
> + alpha = os.path.join(wc_dir, 'A/B/E/alpha')
> + beta = os.path.join(wc_dir, 'A/B/E/beta')
I think you need this here instead:
alpha = os.path.join(wc_dir, 'A', 'B', 'E', alpha')
beta = os.path.join(wc_dir, 'A', 'B', 'E', 'beta')
Otherwise you'll get something like this on Windows:
C:\foo\bar\wc_dir\A/B/E/alpha
C:\foo\bar\wc_dir\A/B/E/beta
> + non_existent_path = os.path.join(wc_dir, 'non-existent')
> +
> + # All targets are existing
> + svntest.actions.run_and_verify_svn2(None, None, [],
> + 0, 'ls', alpha, beta)
> +
> + # One non-existing target
> + expected_err = "svn: warning: W155010: The node '" + \
> + re.escape(os.path.abspath(non_existent_path)) + "' was not found.\n" +
> \
> + ".*\nsvn: E200000: A problem occurred; see other errors for details\n"
> + expected_err_re = re.compile(expected_err)
> +
> + exit_code, output, error = svntest.main.run_svn(1, 'ls', alpha,
> + non_existent_path, beta)
> +
> + # Verify error
> + if not expected_err_re.match("".join(error)):
> + raise svntest.Failure('Cat failed: expected error "%s", but received '
> + '"%s"' % (expected_err, "".join(error)))
> +
> +def ls_multiple_url_targets(sbox):
> + "ls multiple url targets"
> +
> + sbox.build(read_only = True, create_wc = False)
> +
> + alpha = sbox.repo_url + '/A/B/E/alpha'
> + beta = sbox.repo_url + '/A/B/E/beta'
> + non_existent_url = sbox.repo_url + '/non-existent'
> +
> + # All targets are existing
> + svntest.actions.run_and_verify_svn2(None, None, [],
> + 0, 'ls', alpha, beta)
> +
> + # One non-existing target
> + expected_err = "svn: warning: W160013: .*\n" + \
> + ".*\nsvn: E200000: A problem occurred; see other errors for details\n"
> + expected_err_re = re.compile(expected_err)
> +
> + exit_code, output, error = svntest.main.run_svn(1, 'ls', alpha,
> + non_existent_url, beta)
> +
> + # Verify error
> + if not expected_err_re.match("".join(error)):
> + raise svntest.Failure('Cat failed: expected error "%s", but received
> "%s"' % \
> + (expected_err, "".join(error)))
> +
> ########################################################################
> # Run the tests
>
> @@ -2751,6 +2830,10 @@
> basic_relocate,
> delete_urls_with_spaces,
> ls_url_special_characters,
> + ls_non_existent_wc_target,
> + ls_non_existent_url_target,
> + ls_multiple_wc_targets,
> + ls_multiple_url_targets,
4 tests! Nice :)
> ]
>
> if __name__ == '__main__':
> Index: subversion/svn/list-cmd.c
> ===================================================================
> --- subversion/svn/list-cmd.c (revision 1068757)
> +++ subversion/svn/list-cmd.c (working copy)
> @@ -215,6 +215,8 @@
> apr_pool_t *subpool = svn_pool_create(pool);
> apr_uint32_t dirent_fields;
> struct print_baton pb;
> + svn_boolean_t saw_a_problem = FALSE;
Maybe give this variable a more specific name, like 'seen_nonexistent_target'?
> + svn_error_t *err;
>
> SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
> opt_state->targets,
> @@ -280,14 +282,29 @@
> SVN_ERR(svn_cl__error_checked_fputs(sb->data, stdout));
> }
>
> - SVN_ERR(svn_client_list2(truepath, &peg_revision,
> - &(opt_state->start_revision),
> - opt_state->depth,
> - dirent_fields,
> - (opt_state->xml || opt_state->verbose),
> - opt_state->xml ? print_dirent_xml :
> print_dirent,
> - &pb, ctx, subpool));
> + err = svn_client_list2(truepath, &peg_revision,
> + &(opt_state->start_revision),
> + opt_state->depth,
> + dirent_fields,
> + (opt_state->xml || opt_state->verbose),
> + opt_state->xml ? print_dirent_xml :
> print_dirent,
> + &pb, ctx, subpool);
>
> + if (err)
> + {
> + /* If one of the targets is a non-existent URL or wc-entry,
> + don't bail out. Just warn and move on to the next target. */
> + if (err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND ||
> + err->apr_err == SVN_ERR_FS_NOT_FOUND)
> + svn_handle_warning2(stderr, err, "svn: ");
> + else
> + return svn_error_return(err);
> +
> + svn_error_clear(err);
> + err = NULL;
> + saw_a_problem = TRUE;
> + }
> +
> if (opt_state->xml)
> {
> svn_stringbuf_t *sb = svn_stringbuf_create("", pool);
> @@ -301,5 +318,8 @@
> if (opt_state->xml && ! opt_state->incremental)
> SVN_ERR(svn_cl__xml_print_footer("lists", pool));
>
> - return SVN_NO_ERROR;
> + if (saw_a_problem)
> + return svn_error_create(SVN_ERR_BASE, NULL, NULL);
> + else
> + return SVN_NO_ERROR;
> }
