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

Re: [Subclipse-dev] Another bug in new locking code in 1.1.9

From: Mark Phippard <markphip_at_gmail.com>
Date: 2006-12-23 14:38:37 CET

On 12/23/06, Thomas M. Hofmann <email@thomashofmann.com> wrote:

> The problem is as follows:
>
> When a lock request is made on a file (read-only in the workspace) that is already locked by another user the file will be read write after the lock command has run. The message about the other user having the lock appears in the console but the read only flag is always removed.
>
> It took me some time to find out how this all works because I wanted to be able to debug down to the svnkit. I know have everything setup for debugging and can explain the problem.
>
> The bug is in the new code of LockResourcesCommand:
>
> try {
> monitor.beginTask(null, 100);
> OperationManager.getInstance().beginOperation(svnClient);
>
> svnClient.lock(resourceFiles,message,force);
> // Ensure resource is writable. SVN will only change read-only
> // flag if the svn:needs-lock property is set.
> for (int i = 0; i < resources.length; i++) {
> makeWritable(resources[i]);
> }
> } catch (SVNClientException e) {
> throw SVNException.wrapException(e);
> } finally {
> OperationManager.getInstance().endOperation();
> monitor.done();
> }
>
> The comment explains the intention of the code. I guess that an exception being thrown in case the locking fails was expected by the author of the new code. Unfortunately, this is not the case if the lock operation completes but returns with the information that some other user has the lock. In this case a handler set on the SVNClientImpl (when using SVNKit) is called with the information about the other user having the lock. This information is printed to the console using the JhlNotificationHandler and NotificationListener.
>
> The code inside the LockResourceCommand will continue to execute and thus all files will be made read write.
>
> So the fix would probly be something like this:
>
> - Register a new handler before svnClient.lock(resourceFiles,message,force) is called and only call the makeWritable in case the handler signals success.
>
>
> Thinking about this some more I really don`t see why it is necessary to make the files read write that were read only before and do not have the needs-lock property.
> I could imagine someone using the lock operation explicitly on a resource that does not have the needs-lock but this would not make much sense for me because the other users wonÄt be prevented from making changes becaus there copies are not read-only in their workspaces.
>
> I think definitly needs to be fixed for 1.1.10 since it maes using locks unsafe. In case the lock dialog is triggered implicitly when a user just begins to modify it and then saves it the modifications will be persistet altough the console shows the error message about the resource being locked.
>
> One thing that I would also like to mention is that I would expect an error dialog in case the resource is already locked by someone else. It is difficult to understand for new users that they need to look at the SVN console to see why something is not working.
>
> What do you think?

I am not going to attempt to tackle the error dialog issue. I do not
disagree with the idea, but I also think we have the Console and that
is what it is for. Imagine a refactor process that was locking 10-20
files.

Your analysis of the error and its importance are all correct. I
think you gave me enough to fix it when I get some time after the
holiday.

Thanks

Mark Phippard
http://markphip.blogspot.com/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subclipse.tigris.org
For additional commands, e-mail: dev-help@subclipse.tigris.org
Received on Sat Dec 23 14:38:42 2006

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

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