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

RE: Subversion checked-out files not indexed in Windows search

From: Bert Huijben <bert_at_qqmail.nl>
Date: Tue, 18 Mar 2014 11:20:23 +0100

> -----Original Message-----
> From: Ivan Zhakov [mailto:ivan_at_visualsvn.com]
> Sent: dinsdag 18 maart 2014 11:07
> To: Bert Huijben
> Cc: Branko Čibej; Subversion Development
> Subject: Re: Subversion checked-out files not indexed in Windows search
>
> On 13 March 2014 17:14, Bert Huijben <bert_at_qqmail.nl> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Ivan Zhakov [mailto:ivan_at_visualsvn.com]
> >> Sent: donderdag 13 maart 2014 13:56
> >> To: Branko Čibej
> >> Cc: Subversion Development
> >> Subject: Re: Subversion checked-out files not indexed in Windows search
> >>
> >> On 13 March 2014 16:46, Branko Čibej <brane_at_wandisco.com> wrote:
> >> > On 13.03.2014 13:37, Ivan Zhakov wrote:
> >> >
> >> > On 12 March 2014 18:17, Gareth McCaughan
> >> > <Gareth.McCaughan_at_lightblueoptics.com> wrote:
> >> >
> >> > Ivan Zhakov wrote:
> >> >
> >> > It looks like serious issue. Could you please file issue in Subversion
> >> > issue tracker: http://subversion.tigris.org/issues
> >> >
> >> > Done. Issue #4478.
> >> >
> >> > Gareth, thanks a lot!
> >> >
> >> > It seems we have second reason to create temporary files in target WC
> >> > directory, instead of .svn/tmp. Another problem we had before is
> >> > inherited ACL on Windows discussed year ago:
> >> > http://mail-archives.apache.org/mod_mbox/subversion-
> >>
> dev/201309.mbox/%3C20130928110059.d1bb8d007dfe7b26cbcb4f719cb77fd6
> >> .be88925bf6.wbe%40email16.secureserver.net%3E
> >> >
> >> > I've prepared stupid patch to create temporary files in WC, instead of
> >> > .svn/tmp and it seems working fine.
> >> >
> >> >
> >> > -1. This leaves us with no reliable way to clean up the working copy in
> case
> >> > of an aborted operation.
> >> >
> >> > There should be a better way for managing ACLs on windows that does
> not
> >> > require us to mess up the working copy. I'm perfectly happy with just
> >> > documenting that we don't support different ACLs for .svn and the rest
> of
> >> > the WC, at least for now.
> >> >
> >> Please note that problem reported is not about inherited ACL: now
> >> users got NOT_INDEXED attribute on all WC files, because .svn marked
> >> to by not indexed by Windows Search and tools like that.
> >
> > I'm working on a better patch for this, that doesn't have this problem and
> > will improve performance on the pristine store operations as well.
> That's good news. Could you please share idea of your patch?
>
> I'm thinking of something like this:
> 1. Create temp file in target directory.
> 2. Set DELETE_ON_CLOSE=TRUE using SetFileInformationByHandle()
> 3. Write contents to temporary file
> 4. Set DELETE_ON_CLOSE=FALSE using SetFileInformationByHandle()
> 5. Rename to final location using SetFileInformationByHandle()
> 6. Delete if failed.
> 7. CloseFile()

The idea I was working on didn't involve creating files in locations where they could work against us in a case of a crash. I don't think delete on close will be handled on system failures, just on application failures.

The original reason to set the NOT_INDEXED flag on the .svn subdirectory (patches by stefanking and you), was that this caused quite a huge slowdown as the file indexer on Windows 7 and XP continuously monitors for changes and started indexing the file even before we moved it in place. (Even triggering an NTFS bug in Windows 7 pre SP 1)

The new install code keeps the file under a full lock that allows us to move the file in place without outside handling of a file. (A virusscanner can still read it while we have it open, but as we already have the move and delete privileges we don't have to wait to obtain move rights).

I think we should stick to the original (far pre Subversion 1.0) model of creating files in the .svn wc directories, for the same reason Brane alreadt vetoed your idea of creating the files in the directory. The reasoning behind that is very solid.
That some users want to see permissions different than in all previous versions is not a good reason to slow things down (another open, ACL check, ACL update, close) and/or to make the working copy less stable (possibility of files in the working copy that hides real changes and are easily to accidentally commit).

We never touched ACLs, on any platforms... I don't think we should start with doing that, especially at a performance cost.

I hoped that for the non-indexing bit we could just pass the attribute to CreateFileW(), but this function explicitly ignores this attribute. It only applies it from the parent directory. Using a different tempdir for creating new pristine files would work.

But the reason we set this flag is mostly gone anyway... I think we should just remove our code to touch that attribute and use the apr attribute function like we did before. The additional indexing is not really our problem (and the indexer won't care about a few more MB), and it is very easy to filter on *.svn-base if somebody want that.

        Bert

>
> This will be Windows Vista only code since
> SetFileInformationByHandle() is not available for older platforms. But
> newer Windows SDKs has compatibility library FileExtd.lib for Windows
> XP.
>
>
> >
> > But since when is this a huge problem? We applied this setting since 1.6 if I
> remember correctly...
> > I'm more surprised that you didn't know about this?
> >
> I think setting NOT_INDEXED attribute is big problem, while ACL is
> nice to have but not so critical. I was not aware about this problem
> before.
>
>
> --
> Ivan Zhakov
> CTO | VisualSVN | http://www.visualsvn.com
Received on 2014-03-18 11:21:05 CET

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