> -----Original Message-----
> From: Stefan Sperling [mailto:stsp_at_elego.de]
> Sent: dinsdag 18 juni 2013 12:33
> To: dev_at_subversion.apache.org
> Subject: Re: svn commit: r1493703 - in
> /subversion/trunk/subversion/libsvn_wc: upgrade.c wc_db.c wc_db.h
>
> On Mon, Jun 17, 2013 at 10:04:38AM -0000, rhuijben_at_apache.org wrote:
> > Author: rhuijben
> > Date: Mon Jun 17 10:04:38 2013
> > New Revision: 1493703
> >
> > URL: http://svn.apache.org/r1493703
> > Log:
> > Perform an upgrade notification on every successfull working copy
> upgrade,
> > instead of only from pre-1.7 working copies.
> >
> > This helps GUI clients to determine if the format bump was successfull.
> >
> > * subversion/libsvn_wc/upgrade.c
> > (svn_wc_upgrade): Update caller. Notify on format bumps withing WC-
> NG range.
> >
> > * subversion/libsvn_wc/wc_db.c
> > (svn_wc__db_bump_format): Re-order arguments to match common
> form. Provide
> > optional output argument to tell about an actual format bump.
> >
> > * subversion/libsvn_wc/wc_db.h
> > (svn_wc__db_bump_format): Update prototype.
>
> Hi Bert,
>
> I have one question about this commit, see below.
>
> > Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
> > URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_
> db.c?rev=1493703&r1=1493702&r2=1493703&view=diff
> >
> ==========================================================
> ====================
> > --- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
> > +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Mon Jun 17 10:04:38
> 2013
> > @@ -14990,14 +14990,18 @@ svn_wc__db_verify(svn_wc__db_t *db,
> >
> > svn_error_t *
> > svn_wc__db_bump_format(int *result_format,
> > - const char *wcroot_abspath,
> > + svn_boolean_t *bumped_format,
> > svn_wc__db_t *db,
> > + const char *wcroot_abspath,
> > apr_pool_t *scratch_pool)
> > {
> > svn_sqlite__db_t *sdb;
> > svn_error_t *err;
> > int format;
> >
> > + if (bumped_format)
> > + *bumped_format = FALSE;
> > +
> > /* Do not scan upwards for a working copy root here to prevent
> accidental
> > * upgrades of any working copies the WCROOT might be nested in.
> > * Just try to open a DB at the specified path instead. */
> > @@ -15032,7 +15036,10 @@ svn_wc__db_bump_format(int
> *result_forma
> >
> > SVN_ERR(svn_sqlite__read_schema_version(&format, sdb,
> scratch_pool));
> > err = svn_wc__upgrade_sdb(result_format, wcroot_abspath,
> > - sdb, format, scratch_pool);
> > + sdb, format, scratch_pool);
> > +
> > + if (bumped_format)
> > + *bumped_format = (*result_format > format);
>
> Strictly speaking, I don't think we should modify *bumped_format here
> in case there was an error. So I would expect this code to read like
> this:
>
> if (err == SVN_NO_ERROR && bumped_format)
> *bumped_format = (*result_format > format);
There are no code paths where an error is not returned here (just cleaning
up state), and once we return an error all output values are 'undefined'.
So, yes it could be improved, but there are far bigger issues in the upgrade
code.
(libsvn_client assumes upgrade is only used for <= 1.6 bumps and as such
always upgrades externals from properties, ignoring the EXTERNALS table
completely)
Bert
Received on 2013-06-18 17:10:02 CEST