[RFC] making TreeTransform more resistant to unexpected failures / writing problems

[Kuno Meyer]

On 07.08.2007 21:57, Aaron Bentley wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Kuno Meyer wrote:
>> On 07.08.2007 06:21, Aaron Bentley wrote:
>>> I am infuriated by the way Windows conflates opening a file with locking
>>> it, and this is exacerbated by Visual Studio's habit of opening files at
>>> random.
>> Nothing to be infuriated. Visual Studio just holds locks
> 
> My understanding is that Windows will not let you e.g. delete a file
> that is open, regardless of whether or not the other client has
> explicitly locked it.

Yes, this is the default behaviour, hence valid for >95% of all 
applications. But for the Win32 CreateFile method, there is a 
FILE_SHARE_DELETE flag.

(Details: http://en.wikipedia.org/wiki/File_locking#File_locking_in_Windows)

> 
> This is completely legal, and very convenient, on most POSIX systems.
> 
> It is infuriating that there's no way of making the transform *succeed*
> on win32, whereas, on UNIX, we can always succeed, assuming we have the
> correct write permissions to the directories.  It is infuriating that
> the best we can hope for on win32 is to sometimes succeed, and mostly
> fail safe.
> 
>> on files that
>> don't deserve version control
> 
> So you are saying that Visual Studio's Intellisense doesn't read actual
> source files ever?

yes and no: read my next paragraph

> 
>> (binaries and generated intermediate files
>> with cached data), and versioning such files is clearly a mistake.
> 
> So you're saying Andrew King had versioned generated/intermediate files?


The following is my experience with Visual Studio (as my daily working 
environment) with C#-Projects:

- VS *does not* hold locks on source code files (*.cs and others), 
project files(*.csproj) and solution files (*.sln).

Thus everything that needs to be versioned is kept lock-free.

((Side remark: My experience is that, however, it is not advisable to 
update the .csproj-Files while they are opened in VS, because the reload 
mechanism seems to be (or perhaps was) broken for project files, 
sometimes not reloading them. Updating .cs files and .sln files outside 
of VS is no problem. Because of this, I usually close the solution 
before performing any VCS operations.))


- For referenced assemblies not stored in the global assembly cache, VS 
holds a local copy per project (a VS project is a compilation unit, 
resulting in a DLL/EXE/...) as data source for Intellisense. These 
reference assemblies are being kept locked almost always.

((Side remark: Under VS 2003, this locking scheme was the reason why 
building projects into the same target directory led to problems. 
Deployment had to be performed in a post-build step. This has been 
corrected with VS 2005.))


- Other locking could happen when some process runs runs with binaries 
under version control. This may be clear, but sometimes the references 
are a bit hidden (service processes, IIS web projects, ...)


Andrew King didn't state more details in his post, but I'm pretty sure 
that if his ".bzrignore" would have contained "*.suo;*.user;bin;obj" 
(for Non-Web C# projects, some more for other languages and project 
types), he wouldn't have had the locking problems.

BTW: There was a recent discussion to this subject
http://thread.gmane.org/gmane.comp.version-control.bazaar-ng.general/28578/focus=28583


So yes and no:
- Intellisense uses source-code parsing for files of the current 
project/compilation unit
- Intellisense uses reflection-based information retrieval for 
referenced (external) assemblies


> 
>> It occasionally may happen that a user forgets to close such documents
>> when running bzr
> 
> In Unix, there's no need to remember to close such documents when
> running bzr.

Windows is not Unix ;-)

> 
>> (as this may happen in other situations as well where
>> file locks are being hold during a long time). Bazaar doesn't cope well
>> even with the static case of locks constantly being hold.
> 
> Locks under Unix are typically advisory, not mandatory.

As I stated: Windows has different default behaviour.

> 
>> And bug 67699 talks of write-protection flags as well.
> 
> Write-protection of files on Unix does not prevent renames, it prevents
>  writes.

On Windows, it prevents writes and deletion, but not renames. The error 
happens when deleting the (moved) file from the limbo directory.

> 
>>> I think that you can't predict whether this failure mode will occur.
>>> The only thing you can hope to do is recover gracefully when it does
>>> occur, e.g. by rolling back the transform or retrying the operation.
>> I'm sorry to say this, but implementing a rollback is way too much work
>> for the time I have available for bazaar, based on a) my knowledge of
>> cross-platform development, b) my knowledge of the internals of Bazaar
>> and c) the complexity of TreeTransform. The TreeTransform class is in my
>> opinion a bit underdocumented as well.
> 
> Your opinion is certainly valid, but I have done my best.  I have
> documented most of the public methods and many private ones, too.  As
> the main author of TreeTransform, I may not have the perspective needed
> to update the documentation, but I would welcome advice or patches.

Didn't wanted to blame you. It was just my impression after having 
looked for half an hour into the code.

> 
>> So, I am considering to implement at least an improved treatment of
>> write-protection flags:
>>
>> 1) Write-protected files shall not break TreeTransform.finalize() (where
>> the limbo dir gets cleared)
> 
> I don't think that changing finalize will change much.  If a transform
> has been successfully applied, then there will be nothing in limbo anyway.

At least, this is the position where my script in the last comment of 
bug 67699 fails

> 
>> 2) Newly created or modified files shall not have the write protection
>> flag set (enforce chmod u+w)
>>
>> Would this be acceptable an acceptable behaviour?
> 
> That is not necessary or desirable on UNIX.  In fact, we try to preserve
> the existing file permissions if at all possible.  How would this help
> on Windows?

This was my intention for this second proposal (ignoring 
write-protection flags):

1) Bazaar has a quite nasty behaviour when occurring access problems 
while tree transformation. Therefore, if we can work around the problem 
easily (e.g. by ignoring the write protection flag), we should do so.

2) Write protection in the working copy somehow contradicts the idea of 
a VCS (especially a distributed VCS), where we have versioning and 
branching to remember a specific state.

And for my personal use case:

3) Write protection of individual files is an (unnecessary) feature of 
all Microsoft Version Control systems, and I have a lot of crashed 
transations because of this. Perhaps I'm not the only developer working 
in this way?


Kuno