[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r959257 - /subversion/trunk/subversion/libsvn_wc/wc_db_pdh.c

From: Greg Stein <gstein_at_gmail.com>
Date: Wed, 30 Jun 2010 14:28:48 -0400

On Wed, Jun 30, 2010 at 13:55, Philip Martin <philip.martin_at_wandisco.com> wrote:
> Greg Stein <gstein_at_gmail.com> writes:
>
>> I completely disagree with this.
>>
>> Didn't I just get done describing the intent here?
>
> You didn't respond to my questions in the last thread.

Gee. I'm sorry. I have a life. I don't have time to respond to
everything RIGHT NOW. Maybe I needed to think about it? Maybe I'm
traveling. Maybe I just want to sit back and play Mass Effect 2 for a
while.

>> We allow the
>> upgrade to continue. If there is an impact on work queue items, then
>> they must be upgraded, too.
>
> We don't generally support running old workqueues with newer code, so
> why is uprade so different?  Take this recent commit:
>
>  http://svn.apache.org/viewvc?rev=959374&view=rev
>
> it removes SVN_WC__LOG_DELETE_LOCK.  Every time we remove one of the
> log actions we effectively broke any workqueue that included them.
> Nobody has written any code to migrate those workqueues.

Yup. And because that would only affect devs, and when the workqueue
code throws an error saying "dunno what 'delete-lock' means", then
that dev has an easy way to fix themselves. It is an obvious failure
mode.

I don't care about breaking devs. The interim formats before 1.7 is
released can enforce a cleanup first. But even that is a pain. I'd
rather just see something fail in an obvious way and let the dev fix
it up.

BUT: the need-clean-wq-for-upgrade is NOT true moving forward from 1.7.

>> Clients are not expected to back up to previous revisions and run "svn
>> cleanup". We ONLY plan to require that for the 1.6 -> 1.7 transition.
>>>From then on, we can/should/will perform an upgrade with a non-empty
>> work queue. Every single auto-upgrade in the past has operated in this
>> fashion, and we will continue to do so from 1.7 onwards.
>>
>> Please revert. This is contrary to the design that I explained just
>> yesterday.
>
> Why is it so important to support this?  It doesn't affect many people
> but writing the code to upgrade log items is a big effort.  And it's

We don't have to upgrade today's workqueue items. We simply have to
have a policy to upgrade them once 1.7 is released. The process that I
encoded in the db-opening logic was to support THAT purpose.

> not code that will get used by anyone other than people running dev
> code.  It will be poorly tested and a maintenance burden.  How long do
> we keep it?  How far back in wcng history do we support?
>
> Take this workqueue item:
>
> 13|(loggy 30 /home/pm/sw/subversion/obj/wc1 68 <mv
>   dest=".svn/text-base/f.svn-base"
>   name=".svn/svn-ZIxV3z"/>
> )
>
> It will break the working copy on upgrade because the wrong text-base
> will get copied into the pristine store.

Right. Let it break.

At 1.7 release time, the above style of wq item could be hard to
upgrade, so we could make it more semantic-based. "install TEMP into
pristine store as CHECKSUM". That would be much more resilient to
changes in the working copy format.

>  When we upgrade and run the
> workqueue it will probably fail because the .svn/text-base no longer
> exists.  How will the user handle that?  Isn't it better to get an
> error saying "cleanup first"?

Nope. Users cannot generally downgrade their client to run a cleanup.
Historically, we have always auto-upgraded the working copies, even
with stale logs in them.

The 1.7 upgrade process is too invasive and time-consuming, so we
decided to have a manual process ('svn upgrade'). But moving forward,
the intent is to continue an auto-upgrade mechanism even with stale
workqueues.

> That's just one example.  I don't know exactly which workqueue items
> break.  SVN_WC__LOG_CP_AND_TRANSLATE probably breaks.  There could be
> ones that are no longer in log.c that break, as well as non-loggy
> ones.

Yup. And you'll also note Bert's latest change to the postcommit
workqueue item. Before fetching the "no_unlock" parameter from the
stored skel, he first checked whether it exists. If that portion of
the skel does not exist (stale item!), then it just assumes
no_unlock=TRUE.

Another approach is to rename the work item in the OP_FOO symbols.
Stale items become obvious when the new code cannot run them.

I hope that clarifies things, and explains why this change should be
reverted. I understand your desire to avoid upgrading stale work
items. We don't have to do that *today*, but we need to plan for it in
the future, and organize our code/process for that.

Cheers,
-g
Received on 2010-06-30 20:30:28 CEST

This is an archived mail posted to the Subversion Dev mailing list.