subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Fuhrmann <stefan.fuhrm...@wandisco.com>
Subject Re: svn commit: r1708003 - in /subversion/trunk/subversion/libsvn_repos: commit.c fs-wrap.c load-fs-vtable.c rev_hunt.c
Date Mon, 12 Oct 2015 07:28:07 GMT
On Mon, Oct 12, 2015 at 12:05 AM, Bert Huijben <bert@qqmail.nl> wrote:

>
>
> > -----Original Message-----
> > From: stefan2@apache.org [mailto:stefan2@apache.org]
> > Sent: zondag 11 oktober 2015 19:11
> > To: commits@subversion.apache.org
> > Subject: svn commit: r1708003 - in
> > /subversion/trunk/subversion/libsvn_repos: commit.c fs-wrap.c load-fs-
> > vtable.c rev_hunt.c
> >
> > Author: stefan2
> > Date: Sun Oct 11 17:11:27 2015
> > New Revision: 1708003
> >
> > URL: http://svn.apache.org/viewvc?rev=1708003&view=rev
> > Log:
> > Switch the remaining FS API calls in lib_repos to the latest FS API.
> >
> > * subversion/libsvn_repos/commit.c
> >   (invoke_commit_cb): There is no point enabling revprop caching here
> >                       because it could only speed up a 3rd access while
> >                       we have only two.
> >
> > * subversion/libsvn_repos/fs-wrap.c
> >   (svn_repos_fs_change_rev_prop4,
> >    svn_repos_fs_revision_prop,
> >    svn_repos_fs_revision_proplist): These queries perform only a single
> >                                     revprop read and that needs to return
> >                                     the latest data.
> >
> > * subversion/libsvn_repos/rev_hunt.c
> >   (svn_repos_get_committed_info): Same.
> >
> > * subversion/libsvn_repos/load-fs-vtable.c
> >   (close_revision): Same.
> >   (revprops_close_revision): This one needs to read the latest data.
> >
> > Modified:
> >     subversion/trunk/subversion/libsvn_repos/commit.c
> >     subversion/trunk/subversion/libsvn_repos/fs-wrap.c
> >     subversion/trunk/subversion/libsvn_repos/load-fs-vtable.c
> >     subversion/trunk/subversion/libsvn_repos/rev_hunt.c
>
> <snip>
> > Modified: subversion/trunk/subversion/libsvn_repos/fs-wrap.c
> > URL:
> > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/fs
> > -wrap.c?rev=1708003&r1=1708002&r2=1708003&view=diff
> > ==========================================================
> > ====================
> > --- subversion/trunk/subversion/libsvn_repos/fs-wrap.c (original)
> > +++ subversion/trunk/subversion/libsvn_repos/fs-wrap.c Sun Oct 11
> > 17:11:27 2015
> > @@ -378,7 +378,8 @@ svn_repos_fs_change_rev_prop4(svn_repos_
> >             * to the hooks to be accurate. */
> >            svn_string_t *old_value2;
> >
> > -          SVN_ERR(svn_fs_revision_prop(&old_value2, repos->fs, rev,
> name,
> > pool));
> > +          SVN_ERR(svn_fs_revision_prop2(&old_value2, repos->fs, rev,
> name,
> > +                                        FALSE, pool, pool));
> >            old_value = old_value2;
> >          }
> >
> > @@ -448,12 +449,13 @@ svn_repos_fs_revision_prop(svn_string_t
> >          *value_p = NULL;
> >
> >        else
> > -        SVN_ERR(svn_fs_revision_prop(value_p, repos->fs,
> > -                                     rev, propname, pool));
> > +        SVN_ERR(svn_fs_revision_prop2(value_p, repos->fs,
> > +                                      rev, propname, FALSE, pool,
> pool));
> >      }
> >    else /* wholly readable revision */
> >      {
> > -      SVN_ERR(svn_fs_revision_prop(value_p, repos->fs, rev, propname,
> > pool));
> > +      SVN_ERR(svn_fs_revision_prop2(value_p, repos->fs, rev, propname,
> > FALSE,
> > +                                    pool, pool));
> >      }
> >
> >    return SVN_NO_ERROR;
> > @@ -486,7 +488,8 @@ svn_repos_fs_revision_proplist(apr_hash_
> >        svn_string_t *value;
> >
> >        /* Produce two property hashtables, both in POOL. */
> > -      SVN_ERR(svn_fs_revision_proplist(&tmphash, repos->fs, rev, pool));
> > +      SVN_ERR(svn_fs_revision_proplist2(&tmphash, repos->fs, rev, FALSE,
> > +                                        pool, pool));
> >        *table_p = apr_hash_make(pool);
> >
> >        /* If they exist, we only copy svn:author and svn:date into the
> > @@ -501,7 +504,8 @@ svn_repos_fs_revision_proplist(apr_hash_
> >      }
> >    else /* wholly readable revision */
> >      {
> > -      SVN_ERR(svn_fs_revision_proplist(table_p, repos->fs, rev, pool));
> > +      SVN_ERR(svn_fs_revision_proplist2(table_p, repos->fs, rev, FALSE,
> > +                                        pool, pool));
>
> From your log message:
> [[
> >   (svn_repos_fs_change_rev_prop4,
> >    svn_repos_fs_revision_prop,
> >    svn_repos_fs_revision_proplist): These queries perform only a single
> >                                     revprop read and that needs to return
> >                                     the latest data.
> ]]
>
> Shouldn't these 3 functions pass TRUE then?
>
> I would say that passing FALSE uses the cached and not the latest data?
>

You are absolutely right. This happened because
the original patch set had the inverse semantics
for this parameter.


>
> <snip>
>
> > Modified: subversion/trunk/subversion/libsvn_repos/rev_hunt.c
> > URL:
> > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/r
> > ev_hunt.c?rev=1708003&r1=1708002&r2=1708003&view=diff
> > ==========================================================
> > ====================
> > --- subversion/trunk/subversion/libsvn_repos/rev_hunt.c (original)
> > +++ subversion/trunk/subversion/libsvn_repos/rev_hunt.c Sun Oct 11
> > 17:11:27 2015
> > @@ -171,7 +171,8 @@ svn_repos_get_committed_info(svn_revnum_
> >    SVN_ERR(svn_fs_node_created_rev(committed_rev, root, path, pool));
> >
> >    /* Get the revision properties of this revision. */
> > -  SVN_ERR(svn_fs_revision_proplist(&revprops, fs, *committed_rev,
> pool));
> > +  SVN_ERR(svn_fs_revision_proplist2(&revprops, fs, *committed_rev,
> > FALSE,
> > +                                    pool, pool));
>
> Same here... also documented in your log message to use latest, while it
> now uses cached.
>

I just went through yesterday's patch set, checked the
parameter values and found a few more instances.
Bad commit day, apparently.

Thanks for reviewing!

-- Stefan^2.

Mime
View raw message