You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2013/06/17 12:04:38 UTC
svn commit: r1493703 - in /subversion/trunk/subversion/libsvn_wc: upgrade.c
wc_db.c wc_db.h
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.
Modified:
subversion/trunk/subversion/libsvn_wc/upgrade.c
subversion/trunk/subversion/libsvn_wc/wc_db.c
subversion/trunk/subversion/libsvn_wc/wc_db.h
Modified: subversion/trunk/subversion/libsvn_wc/upgrade.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/upgrade.c?rev=1493703&r1=1493702&r2=1493703&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/upgrade.c (original)
+++ subversion/trunk/subversion/libsvn_wc/upgrade.c Mon Jun 17 10:04:38 2013
@@ -2196,13 +2196,15 @@ svn_wc_upgrade(svn_wc_context_t *wc_ctx,
upgrade_working_copy_baton_t cb_baton;
svn_error_t *err;
int result_format;
+ svn_boolean_t bumped_format;
/* Try upgrading a wc-ng-style working copy. */
SVN_ERR(svn_wc__db_open(&db, NULL /* ### config */, TRUE, FALSE,
scratch_pool, scratch_pool));
- err = svn_wc__db_bump_format(&result_format, local_abspath, db,
+ err = svn_wc__db_bump_format(&result_format, &bumped_format,
+ db, local_abspath,
scratch_pool);
if (err)
{
@@ -2224,6 +2226,17 @@ svn_wc_upgrade(svn_wc_context_t *wc_ctx,
SVN_ERR_ASSERT(result_format == SVN_WC__VERSION);
+ if (bumped_format && notify_func)
+ {
+ svn_wc_notify_t *notify;
+
+ notify = svn_wc_create_notify(local_abspath,
+ svn_wc_notify_upgraded_path,
+ scratch_pool);
+
+ notify_func(notify_baton, notify, scratch_pool);
+ }
+
return SVN_NO_ERROR;
}
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);
/* Make sure we return a different error than expected for upgrades from
entries */
Modified: subversion/trunk/subversion/libsvn_wc/wc_db.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.h?rev=1493703&r1=1493702&r2=1493703&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.h (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.h Mon Jun 17 10:04:38 2013
@@ -2913,11 +2913,15 @@ svn_wc__db_upgrade_get_repos_id(apr_int6
* Upgrading subdirectories of a working copy is not supported.
* If WCROOT_ABSPATH is not a working copy root SVN_ERR_WC_INVALID_OP_ON_CWD
* is returned.
+ *
+ * If BUMPED_FORMAT is not NULL, set *BUMPED_FORMAT to TRUE if the format
+ * was bumped or to FALSE if the wc was already at the resulting format.
*/
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);
/* @} */
Re: svn commit: r1493703 - in
/subversion/trunk/subversion/libsvn_wc: upgrade.c wc_db.c wc_db.h
Posted by Stefan Sperling <st...@elego.de>.
On Tue, Jun 18, 2013 at 05:08:55PM +0200, Bert Huijben wrote:
> > > 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,
Ok. So I'll make this tweak if you don't mind.
> 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)
Well, that's an unrelated issue. I was just asking about the above
code snippet. I wasn't looking for any more worms in this can :)
RE: svn commit: r1493703 - in /subversion/trunk/subversion/libsvn_wc: upgrade.c wc_db.c wc_db.h
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Stefan Sperling [mailto:stsp@elego.de]
> Sent: dinsdag 18 juni 2013 12:33
> To: dev@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@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
Re: svn commit: r1493703 - in
/subversion/trunk/subversion/libsvn_wc: upgrade.c wc_db.c wc_db.h
Posted by Stefan Sperling <st...@elego.de>.
On Mon, Jun 17, 2013 at 10:04:38AM -0000, rhuijben@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);
>
> /* Make sure we return a different error than expected for upgrades from
> entries */
>