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

Re: [PATCH] Issue 1539: "be more permissive with out-of-date dirs"

From: Tobias Schäfer <tobiasschaefer_at_gmx.de>
Date: 2006-01-10 23:36:02 CET

On Monday 09 January 2006 17:12, Julian Foad wrote:
> Tobias Schäfer wrote:
> > a common user problem when committing property changes to a directory is
> > that the directory is out of date and the commit therefore fails. The
> > solution is to update the directory. So far everything seems to be ok.
> >
> > But there are corner cases when subversion claims a directory to be
> > out-of-date when it isn't really. The checks if property change are
> > allowed to be committed are a bit lazy. They only consider the revision
> > number. But since the revision number is only a point in time it doesn't
> > mean that a relevant change has taken place in the directory. A better
> > algorithm is to check if a property change has taken place between the
> > BASE revision of the directory which is about to be commited and HEAD. If
> > there are property changes then the commit of the property change should
> > fail; else succeed.
>
> Maybe. But the fact that no property changes have occurred between BASE
> and HEAD does not necessarily mean that your local property changes are
> safe. Your local changes to svn:ignore and svn:externals may no longer be
> valid if the set of files in the directory has changed.
>
> Does this matter? Is it likely to be a problem? Maybe not. I just
> thought I'd better mention it. What do you and others think?

Interesting point.
I guess it will alway be possible to find a scenario which leads to effects
which aren't optimal - unless we choose a most conservative approach.

See possible solution 2b) later on.

> In "update", we allow pretty much any changes that do not overlap to be
> merged automatically. However, in "commit", we don't: every item with
> changes must be up to date. We don't allow "commit" to merge property
> changes on a file with text changes to the same file, so what is the
> justification for saying that the directory is "not really out of date" and
> local property changes to merge with remote content changes to that same
> dir?

Ok, I get your point. I had assumed that a property change could only conflict
with a property change. This is syntactically correct, but not necessarily
semantically.

> To address this from a different angle: the current behaviour is
> conservative, assuming that the property changes might conflict
> (semantically) with the content changes. Your suggestion is optimistic: it
> allows you to commit property changes with the hope that they will still be
> applicable to the HEAD state of the repository.

I agree that my suggestion is rather optimistic. On the other hand we are
"only" talking about semantic conflicts which are also reproducible when
simply working with files. Subversion can not possibly prevent all semantic
conflicts but I guess it should try prevent most conflicts without to many
false alarms.

> > My original question on the users mailing list with two scenarios can be
> > found here: http://svn.haxx.se/users/archive-2005-12/0974.shtml
> >
> > Erik Huelsmann kindly pointed be to the enhancement "be more permissive
> > with out-of-date dirs":
> > http://subversion.tigris.org/issues/show_bug.cgi?id=1539
>
> I've just read these. They both tell a slightly different story from what
> you wrote above. They both deal with the case where the local directory is
> in fact completely up to date, but is just not known to be so, i.e. its
> BASE revision number is out of date but all its properties and content are
> up to date. Allowing a prop-change commit in that case is far easier to
> justify.
>
> Would you accept this approach?

Well, this is a conservative approach. But this is the least I would expect.
Currently the checks (comparing revision numbers) are so lazy that
not even this conservative approach is implemented.

> > A further improvement:
> > Instead of relying on checking if there were any property changes in the
> > directory between BASE and HEAD it would be even more intelligent to
> > check for conflicts: if the property/properties about to commited and the
> > property/properties which were changed between BASE and HEAD all have
> > *different* names then the commit succeeds; else fails!
>
> I don't think we want to move in that direction, though I'm open to hearing
> other views.

If we stay with one of the following conservative approaches (2a or 2b) I
fully agree. But if we go so far that properties can be committed if no
property change has taken place between BASE and HEAD of the directory (3a)
then this (3b) is in my opinion the logical consequence.

To summarize this up, we have the following levels ranging from lazy over
conservative to very-optimistic when committing property-changes to a
directory:

1. lazy: check the revision number and bail out with an out-of-date error if
BASE<HEAD. This is currently implemented.
2a) conservative: check recursively below the directory if everything is
up-to-date regarding the contents and properties of all files and
directories. This is what I initially asked on the user-mailing-list.
2b) less-conservative: check non-recursively below the directory if everything
is up-to-date regarding the contents and properties of all files and that no
files or directories have been added or deleted. This adresses your valid
objection concerning svn:ignore and svn:externals without going recursively
through the whole directory.
3a) optimistic: check if there are any property changes between BASE and HEAD
and bail out with an out-of-date error if there are changes; otherwise the
commit succeeds. This is what #1539 suggests and is what I implemented with
my patch.
4b) very-optimistic: check if the property-changes between BASE and HEAD would
be mergable with an update and only bail out on encountering a conflict. This
is what I described with "Further Enhancement" in my mail at the beginning of
this thread. Note: syntactically all is safe, but sematically not.

I would vote for level 2b. If we agree that 2a or 2b is disirable I am willing
to try and provide a patch.

> (A note on coding: we use the 1989 standard of C ("C'89") in which
> C++-style comments are not allowed and variables must be declared at the
> beginning of a block, not after statements.)

thanks, I will fix that before posting any further patches.

Tobias

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jan 10 23:47:48 2006

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