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

Re: Revert r23859 - conditionalize skipped sleeping for timestamps on maintainer mode

From: David Glasser <glasser_at_davidglasser.net>
Date: 2007-11-04 06:49:48 CET

On 11/3/07, Justin Erenkrantz <justin@erenkrantz.com> wrote:
> I would like to propose that we revert r23859 (log message and diff below).
>
> In my opinion, we should never have code that is externally different
> based on the value of SVN_DEBUG. If such conditional code paths had a
> bug, we'd never find it easily.

Agreed.

> This will also have the happy side-effect of:
> - Allowing normal users to bypass the sleep for timestamps if it they
> choose to do so (if it's good enough for us to enable in maintainer
> mode, it's good enough for users, IMO)

See, but it's *barely* good enough for us to enable in maintainer
mode. We can only enable it in maintainer mode if we are very careful
to never write a test that changes a file's contents without changing
its size. Otherwise we have tests that spuriously fail. And we *do*
mess this up, and we *do* have to fix it.

svn_sleep_for_timestamps exists for a very real reason. Making it
easy to disable will make it easy for users to hurt themselves. If
all you need is to set an environment variable to make svn "magically
faster", then users will do so cargo-cultishly and hurt themselves.
(And blame svn.)

> - Allowing developers not enabling maintainer-mode to get the test
> suite speedups (as we already code the logic to be smart about
> timestamps in the tests)
>
> But, isolating this optimization only to when maintainer mode is active is bad.
>
> For the record, for me, 'make check' now goes from 46:05.98 to 4:56.59
> with this change reverted.

I do agree about your points about developers not enabling maintainer
mode, and about debug-only code paths. I don't think this should be
coupled to SVN_DEBUG, but I *do* think it's important that it require
a compile-time define (which won't be enabled by binary packagers) to
disable it. Maybe something like
SVN_UNSAFE_DISABLE_SLEEP_FOR_TIMESTAMPS.

--dave

-- 
David Glasser | glasser_at_davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Nov 4 06:49:56 2007

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