Julian Foad wrote on Thu, Aug 23, 2018 at 10:21:17 +0100:
> +++ subversion/libsvn_fs_fs/recovery.c (working copy)
> @@ -468,15 +468,15 @@ recover_body(void *baton, apr_pool_t *po
> /* Prune younger-than-(newfound-youngest) revisions from the rep
> - cache if sharing is enabled taking care not to create the cache
> - if it does not exist. */
> - if (ffd->rep_sharing_allowed)
> + cache, no matter whether sharing is currently enabled, taking care
> + not to create the cache if it does not exist. */
> + if (ffd->format >= SVN_FS_FS__MIN_REP_SHARING_FORMAT)
Looks good to me: that should fix both #4077 and #4214.
I would only suggest expanding the comment to explain why it's important that
the condition is what it is; for example:
/* Prune younger-than-(newfound-youngest) revisions from the rep
cache, taking care not to create the cache if it does not exist.
We do this whenever rep-cache.db exists, whether it's currently enabled
or not, to prevent a data loss that could result from having revisions
created after this 'recover' operation referring to rep-cache.db rows
that were created before the recover and that point to revisions younger-
than-(newfound-youngest).
*/
(Possibly with some rewording to make it easier to read in three years when
we've all forgotten the context.)
Cheers,
Daniel
> {
> svn_boolean_t rep_cache_exists;
>
> SVN_ERR(svn_fs_fs__exists_rep_cache(&rep_cache_exists, fs, pool));
> if (rep_cache_exists)
> SVN_ERR(svn_fs_fs__del_rep_reference(fs, max_rev, pool));
