Re: svn commit: r14842 - trunk/subversion/clients/cmdline

[Julian Foad <ju...@btopenworld.com>]

Ping!  Did this bug ever get fixed?  I think the bug being discussed is that 
when "svn lock" fails because somebody else already has the lock, it was only 
giving a warning but should be giving an error.

- Julian


Peter N. Lundblad wrote:
> On Fri, 27 May 2005, Brian W. Fitzpatrick wrote:
> 
>>On May 27, 2005, at 11:40 AM, Philip Martin wrote:
>>
>>>  I also checked 'wc/b' and it too
>>>was an unlocked file, in that case somebody beat me and deleted it.
>>>
>>>> It's the same thing
>>>>with attempting to 'svn add' a file that's already been added:
>>>>
>>>>$ touch 1 2 3
>>>>$ svn add 2
>>>>A         2
>>>>$ svn add ?
>>>>A         1
>>>>svn: warning: '2' is already under version control
>>>>A         3
>>>>
>>>>In your case, however, wc/x has been replaced with a directory and I
>>>>consider that to be a different case--you can't just lock the
>>>>directory, and you don't already have a lock on it.
>>>>
>>>>Does that make sense?  Am I misunderstanding what you're saying?
>>>
>>>No, it doesn't really make sense.  It seems to be arbitrary whether a
>>>particular failure is treated as non-fatal.  I suppose we do have
>>>other arbitrary behaviour, whether unversioned files are skipped or
>>>not, but it still strikes me as strange.
>>
>>Well, I think you're right--this is a bug.  I think that it's OK to
>>warn on 'svn lock' if we already have the lock in our working copy,
>>but if someone else has the lock, I think an error is in order.
> 
> Seems reasonable to me. So this is a bug, but not in the notifier:-) It
> can't do anything about it because it has a void return value. So it is
> probably the svn_client_lock/unlcok that need to decide which errors are
> to be treated as warnings and pass them to the notifier.
> 
> Regards,
> //Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r14842 - trunk/subversion/clients/cmdline

[Branko Čibej <br...@xbc.nu>]

Julian Foad wrote:

> Peter N. Lundblad wrote:
>
>> On Wed, 3 Aug 2005, Julian Foad wrote:
>>
>>> Ping!  Did this bug ever get fixed?  I think the bug being discussed 
>>> is that
>>> when "svn lock" fails because somebody else already has the lock, it 
>>> was only
>>> giving a warning but should be giving an error.
>>
>>
>> AFAICT, it didn't:-( I think a problematic thing is the
>> SVN_ERR_IS_LOCK_ERROR macro that hardcodes which errors are non-fatal
>> locking errors. That's a public macro:-(
>
>
> Goodness knows how we allowed that to become a public macro.  It seems 
> to be just a convenience for the RA layers.
>
> Fortunately, changing the implementation of it won't break binary or 
> source compatibility (at least not directly, if it has been used 
> sensibly).  But anyway, even if we can't change it, it doesn't matter 
> because we don't have to use it.
>
> So that's not a problem.  Would you care to propose a fix, or shall we 
> file this bug in the issue tracker?

Just deprecate the damn thing and put a private implementatin somewhere 
else (the RA loader? Yikes, do RA layers even have RA-common headers?)

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Which locking errors should be warnings? (was: Re: svn commit: r14842 - trunk/subversion/clients/cmdline)

["Peter N. Lundblad" <pe...@famlundblad.se>]

On Mon, 8 Aug 2005, Julian Foad wrote:

> Peter N. Lundblad wrote:
> > AFAICT, it didn't:-( I think a problematic thing is the
> > SVN_ERR_IS_LOCK_ERROR macro that hardcodes which errors are non-fatal
> > locking errors. That's a public macro:-(
>
> Goodness knows how we allowed that to become a public macro.  It seems to be
> just a convenience for the RA layers.
>
It was converted from a function to a macro in r13618 FWIW.  I don't know
why, but it does't matter that much.

> Fortunately, changing the implementation of it won't break binary or source
> compatibility (at least not directly, if it has been used sensibly).  But
> anyway, even if we can't change it, it doesn't matter because we don't have to
> use it.
>
Good points.  I think deprecation is best here.

> So that's not a problem.  Would you care to propose a fix, or shall we file
> this bug in the issue tracker?
>
Might be better to try to get it resolved right away.  Let's start with
the subject of this mail: what errors should be considered non-fatal.
Since the RA layers take multiple paths, this is not up to the clients
(but it is still on the client side of the RA layers in code that we have
released.)

Note that I don't remember any design discussions regarding which errors
should be non-fatal. That's why I'm not sure about the rationale for some
of the errors. Maybe fitz can clarify somewhat here.

I've been trying to come up with a proposal regarding which errors should
and shouldn't be warnings, but I'm getting stuck.  I think I need the
following question answered first:
- what is our general policy regarding when something is a warning or an
error?

For example, if I try to lock a file in my WC and I already have it locked
in this WC, then you could view the locking operation as a no-op
(forgetting about metadata such as creation time and lock comment). But
when I try to lock a file locked by me, but without a lock token in this
WC, or if I'm locking a file that is out-of-date, why is that error less
fatal than a "path doesn't exist" error? I'm afraid I don't get the
philosophy here.

Someone with a more well-organized head than me needs to sort this out
before we can get any further.  Sorry for this confused mail, but Julian,
now you know why this answer is a little late...

Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r14842 - trunk/subversion/clients/cmdline

[Julian Foad <ju...@btopenworld.com>]

Peter N. Lundblad wrote:
> On Wed, 3 Aug 2005, Julian Foad wrote:
> 
>>Ping!  Did this bug ever get fixed?  I think the bug being discussed is that
>>when "svn lock" fails because somebody else already has the lock, it was only
>>giving a warning but should be giving an error.
> 
> AFAICT, it didn't:-( I think a problematic thing is the
> SVN_ERR_IS_LOCK_ERROR macro that hardcodes which errors are non-fatal
> locking errors. That's a public macro:-(

Goodness knows how we allowed that to become a public macro.  It seems to be 
just a convenience for the RA layers.

Fortunately, changing the implementation of it won't break binary or source 
compatibility (at least not directly, if it has been used sensibly).  But 
anyway, even if we can't change it, it doesn't matter because we don't have to 
use it.

So that's not a problem.  Would you care to propose a fix, or shall we file 
this bug in the issue tracker?

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r14842 - trunk/subversion/clients/cmdline

["Peter N. Lundblad" <pe...@famlundblad.se>]

On Wed, 3 Aug 2005, Julian Foad wrote:

> Ping!  Did this bug ever get fixed?  I think the bug being discussed is that
> when "svn lock" fails because somebody else already has the lock, it was only
> giving a warning but should be giving an error.
>
AFAICT, it didn't:-( I think a problematic thing is the
SVN_ERR_IS_LOCK_ERROR macro that hardcodes which errors are non-fatal
locking errors. That's a public macro:-(

Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org