Peter Samuelson wrote:
> [Daniel Shahaf]
> > > @@ -62,7 +63,12 @@ is_inside_wc_ng(const char *abspath,
> > >
> > > wc_db_path = svn_path_join_many(pool, abspath, SVN_WC_ADM_DIR_NAME,
> > > "wc.db", NULL);
> >
> > Shouldn't this use svn_wc_get_adm_dir()?
>
> Probably ... but since this is 1.6-specific code (it is not and never
> will be in trunk), I think we'd need a good reason to bother changing
> it.
>
> > > - SVN_ERR(svn_io_check_path(wc_db_path, &kind, pool));
> > > + err = svn_io_check_path(wc_db_path, &kind, pool);
> > > + if (err)
> > > + {
> > > + svn_error_clear(err);
> > > + return SVN_NO_ERROR;
> > > + }
> > >
> >
> > Given the rationale (in the log message), do we want to ignore *any*
> > errors here? Or only errors related to (for example) permissions.
>
> Well, as we just saw with the r908980 group, we can't always predict
> all the errors we can get. For example, would you have thought of
> ELOOP (too many symbolic link traversals)? EFAULT? ENAMETOOLONG?
> (I got those from 'man 2 open' on my system.)
>
> There are many reasons you may not be able to open
> ../../../../.svn/wc.db, which may not even be on the same filesystem as
> the current directory (common, if /home is its own partition). I don't
> think it is the job of is_inside_wc_db() to discover these things and
> abort your operation. If it can't read wc.db, the answer should be "we
> didn't find a wc-ng", not "there might be a wc-ng up there somewhere,
> because we can't prove there isn't". In other words, it's a question
> of where the burden of proof lies.
>
> The worst that can happen is, we allow the user to create a 1.6 wc
> where there was already a 1.7 wc rooted one or more directories up from
> us which we weren't able to get to for whatever reason. I think that's
> a pretty benign failure case.
>
> Peter
+1 to what Peter says.
- Julian
Received on 2010-03-31 18:52:46 CEST