bug-cvs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: lockinfo


From: Mark D. Baushke
Subject: Re: lockinfo
Date: Tue, 10 May 2005 01:54:32 -0700

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Kendy Kutzner <address@hidden> writes:

> On 2005-05-09T11:25:06-0700, Mark D. Baushke wrote:
> > > - empty/nonexistent lockinfo: everything should work as ever
> > > - layout $CVSROOT/a, $CVSROOT/a/b, $CVSROOT/c
> > > - lockinfo contains
> > >   ^a false
> > >   ^a/b true
> > > - checkout a should fail
> > > - checkout b should work (i'm not too sure)
> > 
> > I believe CVS processes directories
> > alphabetically depth-first. So, in this case,
> > b would also fail unless you explicitly did a
> > 
> >      cvs checkout a/b
> 
> you are right, that's what i'm talking about
> when i wrote 'checkout b'

Okay. I just wanted to be clear what was the
expected output of the designer of the patch.

> > In point of fact, the addition of this new
> > feature may be a good time to alter how cvs
> > works.
> 
> as long as i don't have to write it :)

Yup, I understand that feeling very well indeed... :-)

> > If you are explicitly controlling checkout
> > behavior, then you could choose to non-fatally
> > skip directories for which you do not have
> > permissions... it would be as if they were
> > really private to some other set of users and
> > not even visible to the user implicitly trying
> > to check them out.
> 
> Than CVS internals really have to be changed. My
> little patch makes the lock creation fail. When
> lock creation fails, CVS aborts the current
> operation. One possible change: first lock all
> directories which are going to be used. If all
> locks were successfully created, do the
> checkout: somehow
>       if (start_recursion(lock..)){
>               start_recursion(checkout..)
>               start_recursion(unlock..)
>       }

Yeah, it really would be easier to implement this
variation in a read-lock trigger if CVS were using
the lock manager of CVSNT instead of the current
way it does locks.

However, I don't really want to worry about that
feature of CVSNT right now.

> > Although it may be desirable to force an error
> > if the user explictly asked for a
> > 'read-locked' directory on the command line
> > instead of just running into one during the
> > checkout process.
> 
> But as I said, that's exactly the behaviour when
> someone enforces 'dont read (dont create locks)'
> with file system permissions.

Okay.

> > Addition of this kind of 'read-lock' might be
> > considered to be 'better' than just use
> > operating-system directory permissions.
> 
> It's not that elegant, but more flexible. And it
> works with all kinds of file systems.

Well, your patch acts in addition to the locks
imposed by ther other kinds of file systems
already supported...

> > I still remain unconvinced that the current
> > implementation of your patch is desirable.
> 
> Since my patch is GPL, everyone is free to
> change it / ignore it. In which direction do you
> would change it?

I am not sure.

> > > +Before the lock is created, the @file{loginfo} in CVSROOT is
> >
> > The above line references `loginfo' instad of
> > `lockinfo' which seems wrong.
> 
> Because it is wrong :)
> But I think I don't need to send a patch for that :)

True.

> > I suspect that some mention that this is for
> > read-locks instead of dealing with promotable
> > or write locks...
> 
> I thought write locks are an entire different
> ball game? But feel free to correct me.

Sure, but the paragraph you added leaves ambiguity
that the trigger might also apply for other kinds
of locks. The idea would be to be clear in the
documentation that this particular trigger is NOT
used for anything other than read-locks.

> > it also begs the question as to the correct
> > name for this file being 'readinfo' instead of
> > 'lockinfo' ...
> 
> As I wrote, anybody can change that.

Sure.


Okay. The patch is posted and there is at least
some idea of how to write the sanity.sh tests
for trivial exercise of it.

I honestly don't know if it is a good idea to
introduce this patch to feature in this way at
this time.

What do the other maintainers think of this kind
of extension?

        -- Mark
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (FreeBSD)

iD8DBQFCgHbI3x41pRYZE/gRAqHNAJ9pN3V1IoCW56tFqhZyNYsoOye16ACgixS3
GIQQcdhLtqlnGzrMl5zK9IM=
=AwyR
-----END PGP SIGNATURE-----




reply via email to

[Prev in Thread] Current Thread [Next in Thread]