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

Re: svn commit: r1096619 - in /subversion/trunk/subversion/libsvn_wc: translate.c translate.h workqueue.c

From: Hyrum K Wright <hyrum_at_hyrumwright.org>
Date: Tue, 26 Apr 2011 10:25:17 -0500

On Tue, Apr 26, 2011 at 12:35 AM, Greg Stein <gstein_at_gmail.com> wrote:
> On Mon, Apr 25, 2011 at 21:06, Hyrum K Wright <hyrum_at_hyrumwright.org> wrote:
>>...
>> Which kinda irks me, actually.  You'd think something as simple as
>> "ensure the permissions on disk match what's in the db" would be
>> fairly straightforward, but it ends up have all kinds of special cases
>> and various shortcuts.  In some ways, I guess I'd settle for all of
>> the logic being encapsulated in a single function/set-of-functions,
>> rather than being spread across half-a-dozen functions and two files.
>>
>> As a first step, then, it'd be nice to know what special handling is
>> needed for commit/update/revert, as opposed to propset.  I'm happy to
>> go digging, but any insights others have would be useful.
>
> I completely agree with this assessment. I ran into the same kind of
> thing dealing with "translation" ... or "subst" ... depending on which
> file or random function you happened to look at. Fall 2008, I narrowed
> a lot of this down, but it still remains a bit broader of an API than
> I would prefer. ... and setting file attributes is seemingly falling
> into the same category. When working on that stuff, I certainly recall
> finding stuff that would set those flags scattered around, under
> different names, each for a specialized use.
>
> If you can rationalize them, then kudos. Would love to see it.
>
> Bert mentioned changing perms inadvertedly. Not sure that I read and
> grok'd that entirely. But I do tend to agree with one of your
> assessments: the file should always be sync'd with the "topmost"
> props. Get that done first, and then worry about perf later. And
> regarding perf: meh. I tend to believe the changes in the props
> driving these things is *minimal*, so perf is going to be a moot point
> in the typical case. Correctness, code understandability, and
> determinism is the most important goal.

I think I've got all the special cases and concerns isolated to a
single function now. There are certainly a few performance
improvements which could be made (grabbing all the data from the db in
one go, rather than several queries), but the logic is all in the
right spot, including a goofy special case for lock test 40.

Bert's concern, as I understand it, is that before there were code
paths in which we didn't touch the perms at all, but now we
unconditionally clobber whatever perms were there with our own. I'm
fine with that, actually, but I'm not sure what the performance
implications are (I'd presume that OS caches would make the
performance negligible, even on network filesystems).

-Hyrum
Received on 2011-04-26 17:25:49 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.