Karl Fogel <kfogel@newton.ch.collab.net> writes:
> Yes, the entire call stack down has to document the error being
> dependend on...
You didn't address my point. Documenting this one error code in
these few functions doesn't really help. Someone looking to see
which error codes any of these functions returns will still have
to read the code since no one has gone through documenting all
the error codes yet. Someone going through to document all those
error codes will still have to read the code to do the actual
documenting.
> Our practice has been, if some caller tests for a specific error, then
> that particular error should be documented in the callee, and all the
> way down as necessary.
That's a pretty haphazard way to write documentation.
> (Most errors are never tested for explicitly, they're just handled at
> the top level and the error string is thrown at the user.)
That is the state today. In my experience, error codes usually
end up consumed more by programmers making use of the interface
in question than end users. We will see more and more testing
for specific error codes as i have done as time goes on.
> Er, yup, we need a general non-fatal internal feedback system, and we
> don't have one. :-(
I'm not convinced that an additional or replacement system is
needed. The current svn_error system is sufficient, SVN_ERR
usage simply needs to be scaled back in favor of something along
the lines of what i mentioned and you approved for
svn_client_revert.
> Sure, we do that in a lot of functions already. I think it would be
> fine to do that here.
OK, how does this patch look? The diff is identical to the one i
sent last time. The only difference is the addition of the first
part, a change to libsvn_client/revert.c.
Index: subversion/libsvn_client/revert.c
===================================================================
--- subversion/libsvn_client/revert.c (revision 5754)
+++ subversion/libsvn_client/revert.c (working copy)
@@ -49,28 +49,32 @@
don't know if path is a directory. It gets a bit messy. */
SVN_ERR (svn_wc_adm_probe_open (&adm_access, NULL, path, TRUE, recursive,
pool));
- SVN_ERR (svn_wc_is_wc_root (&wc_root, path, adm_access, pool));
+ if ((err = svn_wc_is_wc_root (&wc_root, path, adm_access, pool)))
+ goto out;
if (! wc_root)
{
const svn_wc_entry_t *entry;
- SVN_ERR (svn_wc_entry (&entry, path, adm_access, FALSE, pool));
+ if ((err = svn_wc_entry (&entry, path, adm_access, FALSE, pool)))
+ goto out;
if (entry->kind == svn_node_dir)
{
svn_node_kind_t kind;
- SVN_ERR (svn_io_check_path (path, &kind, pool));
+ if ((err = svn_io_check_path (path, &kind, pool)))
+ goto out;
if (kind == svn_node_dir)
{
/* While we could add the parent to the access baton set, there
is no way to close such a set. */
svn_wc_adm_access_t *dir_access;
- SVN_ERR (svn_wc_adm_close (adm_access));
- SVN_ERR (svn_wc_adm_open (&adm_access, NULL,
- svn_path_dirname (path, pool),
- TRUE, FALSE, pool));
- SVN_ERR (svn_wc_adm_open (&dir_access, adm_access, path,
- TRUE, recursive, pool));
+ if ((err = svn_wc_adm_close (adm_access))
+ || (err = svn_wc_adm_open (&adm_access, NULL,
+ svn_path_dirname (path, pool),
+ TRUE, FALSE, pool))
+ || (err = svn_wc_adm_open (&dir_access, adm_access, path,
+ TRUE, recursive, pool)))
+ goto out;
}
}
}
@@ -80,6 +84,8 @@
ctx->notify_func, ctx->notify_baton,
pool);
+ out:
+
SVN_ERR (svn_wc_adm_close (adm_access));
/* Sleep to ensure timestamp integrity. */
Index: subversion/clients/cmdline/revert-cmd.c
===================================================================
--- subversion/clients/cmdline/revert-cmd.c (revision 5754)
+++ subversion/clients/cmdline/revert-cmd.c (working copy)
@@ -64,9 +64,23 @@
for (i = 0; i < targets->nelts; i++)
{
const char *target = ((const char **) (targets->elts))[i];
-
- SVN_ERR (svn_client_revert (target, recursive, ctx, subpool));
+ svn_error_t *err;
+ err = svn_client_revert (target, recursive, ctx, subpool);
+ if (err)
+ {
+ if (err->apr_err == SVN_ERR_ENTRY_NOT_FOUND)
+ {
+ if (!opt_state->quiet)
+ {
+ svn_handle_warning (stderr, err);
+ }
+ continue;
+ }
+ else
+ return err;
+ }
+
svn_pool_clear (subpool);
}
Index: subversion/clients/cmdline/update-cmd.c
===================================================================
--- subversion/clients/cmdline/update-cmd.c (revision 5754)
+++ subversion/clients/cmdline/update-cmd.c (working copy)
@@ -66,13 +66,28 @@
for (i = 0; i < condensed_targets->nelts; i++)
{
const char *target = ((const char **) (condensed_targets->elts))[i];
+ svn_error_t *err;
- SVN_ERR (svn_client_update
+ err = svn_client_update
(target,
&(opt_state->start_revision),
opt_state->nonrecursive ? FALSE : TRUE,
ctx,
- pool));
+ pool);
+ if (err)
+ {
+ if (err->apr_err == SVN_ERR_ENTRY_NOT_FOUND)
+ {
+ if (!opt_state->quiet)
+ {
+ svn_handle_warning (stderr, err);
+ }
+ continue;
+ }
+ else
+ return err;
+ }
+
}
return SVN_NO_ERROR;
Index: subversion/clients/cmdline/proplist-cmd.c
===================================================================
--- subversion/clients/cmdline/proplist-cmd.c (revision 5754)
+++ subversion/clients/cmdline/proplist-cmd.c (working copy)
@@ -97,11 +97,25 @@
const char *target = ((const char **) (targets->elts))[i];
apr_array_header_t *props;
int j;
-
- SVN_ERR (svn_client_proplist (&props, target,
- &(opt_state->start_revision),
- opt_state->recursive, ctx, pool));
-
+ svn_error_t *err;
+
+ err = svn_client_proplist (&props, target,
+ &(opt_state->start_revision),
+ opt_state->recursive, ctx, pool);
+ if (err)
+ {
+ if (err->apr_err == SVN_ERR_ENTRY_NOT_FOUND)
+ {
+ if (!opt_state->quiet)
+ {
+ svn_handle_warning (stderr, err);
+ }
+ continue;
+ }
+ else
+ return err;
+ }
+
for (j = 0; j < props->nelts; ++j)
{
svn_client_proplist_item_t *item
Index: subversion/clients/cmdline/main.c
===================================================================
--- subversion/clients/cmdline/main.c (revision 5754)
+++ subversion/clients/cmdline/main.c (working copy)
@@ -350,15 +350,15 @@
" for example, when redirecting binary property values to a file).\n",
{'R', 'r', svn_cl__revprop_opt, svn_cl__strict_opt,
SVN_CL__AUTH_OPTIONS} },
-
+
{ "proplist", svn_cl__proplist, {"plist", "pl"},
"List all properties on files, dirs, or revisions.\n"
"usage: 1. proplist [PATH [PATH ... ]]\n"
" 2. proplist --revprop -r REV [URL]\n\n"
" 1. Lists versioned props in working copy.\n"
" 2. Lists unversioned remote props on repos revision.\n",
- {'v', 'R', 'r', svn_cl__revprop_opt, SVN_CL__AUTH_OPTIONS} },
-
+ {'v', 'R', 'r', 'q', svn_cl__revprop_opt, SVN_CL__AUTH_OPTIONS} },
+
{ "propset", svn_cl__propset, {"pset", "ps"},
"Set PROPNAME to PROPVAL on files, dirs, or revisions.\n\n"
"usage: 1. propset PROPNAME [PROPVAL | -F VALFILE] PATH [PATH [PATH ... ]]\n"
@@ -394,7 +394,7 @@
" foo/bar -r 1234 http://example.com/repos/zag\n",
{'F', 'q', 'r', svn_cl__targets_opt, 'R', svn_cl__revprop_opt,
SVN_CL__AUTH_OPTIONS, svn_cl__encoding_opt} },
-
+
{ "revert", svn_cl__revert, {0},
"Restore pristine working copy file (undo all local edits)\n"
"usage: revert PATH [PATH [PATH ... ]]\n\n"
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Apr 29 10:28:52 2003