On 4/2/07, Nicolás Lichtmaier <nick@reloco.com.ar> wrote:
> I've fixed a small bug, here's the patch.
Thanks. My comments are below.
> [[[
> Fix issue #2363: Ignore unreadable files with a warning.
>
> * subversion/libsvn_subr/config_file.c
> (svn_config__parse_file): If must_exist is false also ignore EACCESS
> errors, but printing a warning to stderr.
>
> * subversion/include/svn_config.h: Adjust docs to the new behaviour.
> ]]]
Ok. The log message is almost correct, but you didn't mention the
symbol you were changing the docs for in svn_config.h. This may seem
logical in the context of the message, but it isn't when you're
grepping through the logs for a symbol.
> Index: subversion/include/svn_config.h
> ===================================================================
> --- subversion/include/svn_config.h (revision 24309)
> +++ subversion/include/svn_config.h (working copy)
> @@ -146,8 +146,8 @@
> /** Read configuration data from @a file (a file or registry path) into
> * @a *cfgp, allocated in @a pool.
> *
> - * If @a file does not exist, then if @a must_exist, return an error,
> - * otherwise return an empty @c svn_config_t.
> + * If @a file does not exist or cannot be read, then if @a must_exist,
> + * return an error, otherwise return an empty @c svn_config_t.
> */
> svn_error_t *svn_config_read(svn_config_t **cfgp,
> const char *file,
> Index: subversion/libsvn_subr/config_file.c
> ===================================================================
> --- subversion/libsvn_subr/config_file.c (revision 24309)
> +++ subversion/libsvn_subr/config_file.c (working copy)
> @@ -401,6 +401,12 @@
> svn_error_clear(err);
> return SVN_NO_ERROR;
> }
> + else if (! must_exist && err && APR_STATUS_IS_EACCES(err->apr_err))
> + {
> + svn_handle_warning(stderr, err);
> + svn_error_clear(err);
> + return SVN_NO_ERROR;
> + }
Unfortunately, things can't work this way. libsvn_subr is a library
and the library-using program may not see the output from
svn_handle_warning() (think GUI situations).
What you could do is create a new notification, send that through the
notification callback in the ctx structure (which I presume is
available to one of the callers of svn_config__parse_file()) and
handle that notification in subversion/svn/notify.c...
> else
> SVN_ERR(err);
Thanks for taking a look at this! I hope you feel like taking another
stab at it!
bye,
Erik.
Received on Mon Apr 2 20:52:14 2007