Re: lockinfo
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'
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 :)
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..)
}
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.
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.
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?
+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 :)
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.
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.
Kendy
--
pgpKKxZXZH7Le.pgp
Description: PGP signature
___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: lockinfo
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Kendy Kutzner [EMAIL PROTECTED] 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-
___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: lockinfo
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Mark D. Baushke wrote: 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? I don't have a strong opinion either way. Some users might find it useful to have read ACLs made available this way. Me, I prefer to offload any ACL decisions on the file system since it is almost certainly more secure. As long as I don't have to do the work, though, and without thinking deeply about your discussion about whether soft or hard failures are better, I have no objections to this patch. Cheers, Derek -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.0 (Cygwin) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFCgMRDLD1OTBfyMaQRAs1XAJ9Pu3LSUAoKAPaF67L8oiKlIfBuugCfXay/ t2xW87sMgKYXgcbk4bW4PdI= =D8vS -END PGP SIGNATURE- ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: lockinfo
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Kendy Kutzner [EMAIL PROTECTED] writes:
On 2005-05-06T09:09:37-0700, Mark D. Baushke wrote:
To be considered for inclusion in a CVS FEATURE release, you should
provide changes to the doc/cvs.texinfo
attached
Comments included inline below...
as well as at least a few test
cases (if you don't understand how to work with sanity.sh that is okay,
but we need a series of step-by-step commands that illustrate how to
verify that everything works with your new feature and how the new
feature interacts with existing features).
I didn't get everything from sanity.sh, but here are some ideas for test
cases:
- 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
- checkout c should work
For this particular addition, I have a problem with how it works.
Assumption: I have a repository where there exists one directory called
Private that is 'read-locked' such that userA is able to checkout
directory Private and userB is not.
If userB starts a 'cvs checkout -d top .' command. What should you
expect to happen:
a) no files are checked out because Private will fail and that
'should' stop the checkout command from providing partial
information.
b) all directories that are processed up to, but not including the
Private directory are checked out. However, any files that
would logically have been checked out of directories after
Private are not checked out because the directory traversal
will stop when Private is first reached.
c) all directories other than Private will be checked out of the
repository and the user will receive a Lock Info failed message
that they may or may not notice indicating that the Private
directory was not checked out.
With your current patch, I believe userB will observe behavior 'b',
but I am not sure if that is true or not.
As I see it, you are right, it should be 'b'.
Yeah, this is what I expected.
It is not clear to me if behavior 'a' or behavior 'c' is the correct
behavior, but I believe behavior 'b' is not desirable.
I see your worries. But i think that's the way CVS works:
directory-by-directory. There is no commit-atomicity, there is no
lock-atomicity. I think the behaviour is comparable to a scenario where
userB does not have (file-system) write permissions to the directory.
I added the solicited changes to cvs.texinfo as well as new changes to
src/mkmodules.c to create a default version of lockinfo.
In point of fact, the addition of this new feature may be a good time to
alter how cvs works. 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. 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.
Addition of this kind of 'read-lock' might be considered to be 'better'
than just use operating-system directory permissions.
I still remain unconvinced that the current implementation of your patch
is desirable.
Kendy
--
Only in cvs_my/: autom4te.cache
diff -r -u cvs-1.12.12/doc/cvs.texinfo cvs_my/doc/cvs.texinfo
--- cvs-1.12.12/doc/cvs.texinfo 2005-04-14 17:56:36.0 +0200
+++ cvs_my/doc/cvs.texinfo2005-05-09 17:45:41.070237832 +0200
@@ -6569,6 +6569,16 @@
the change to @file{b/three.c} and not the change to
@file{a/two.c}.
[EMAIL PROTECTED]
+Before the lock is created, the @file{loginfo} in CVSROOT is
The above line references `loginfo' instad of `lockinfo' which seems
wrong.
+consulted. Every non-comment line in that file should
+contain a directory pattern and a filter. On the first
+match between the current active directory and the pattern
+the filter is executed with the directory as parameter. The
+special pattern ALL matches always. When the exit code of
+the filter is not zero, the lock creation failes. If the
+exit code is zero, normal lock creation procedure continues.
+
I suspect that some mention that this is for read-locks instead of
dealing with promotable or write locks... it also begs the question
as to the correct name for this file being 'readinfo' instead of
'lockinfo' ...
@node Watches
@section Mechanisms to track who is editing files
@cindex Watches
Only in cvs_my/src: autom4te.cache
diff -r -u cvs-1.12.12/src/cvs.h cvs_my/src
lockinfo
Attached is a patch to current release which introduces a file
$CVSROOT/lockinfo. The file has the same properties as other *info files
in that directory.
Intended usage: Directories are read-locked before any read-operation is
done. A script (along the lines of contrib/cvs_acl.pl) can check whether
this read operation is allowed. This way read ACLs can be implemented.
Kendy
--
diff -u -r cvs-1.12.12/src/cvs.h cvs_my/src/cvs.h
--- cvs-1.12.12/src/cvs.h 2005-04-14 16:14:55.0 +0200
+++ cvs_my/src/cvs.h2005-05-03 16:44:52.837176991 +0200
@@ -163,6 +163,7 @@
#define CVSROOTADM_CONFIG config
#defineCVSROOTADM_HISTORY history
#defineCVSROOTADM_IGNORE cvsignore
+#define CVSROOTADM_LOCKINFOlockinfo
#defineCVSROOTADM_LOGINFO loginfo
#defineCVSROOTADM_MODULES modules
#define CVSROOTADM_NOTIFY notify
diff -u -r cvs-1.12.12/src/lock.c cvs_my/src/lock.c
--- cvs-1.12.12/src/lock.c 2005-04-14 16:13:26.0 +0200
+++ cvs_my/src/lock.c 2005-05-03 17:54:40.813153559 +0200
@@ -476,7 +476,23 @@
}
}
+int
+check_lock_info_proc(const char * rep, const char * filter, void * ud)
+{
+ const char * srepos = Short_Repository(rep);
+ char * cmdline = Xasprintf(%s %s, filter, rep);
+ run_setup (cmdline);
+ free(cmdline);
+ return(abs(run_exec(RUN_TTY, RUN_TTY, RUN_TTY, RUN_NORMAL |
RUN_SIGIGNORE)));
+}
+int
+check_lock_info(const char * xrep)
+{
+ int n;
+ n = Parse_Info (CVSROOTADM_LOCKINFO, xrep, check_lock_info_proc,
PIOPT_ALL, NULL);
+ return n;
+}
/*
* Create a lock file for readers
@@ -489,6 +505,11 @@
TRACE (TRACE_FUNCTION, Reader_Lock(%s), xrepository);
+if (check_lock_info(xrepository)){
+ error(0, 0, Lock Info failed);
+ return 1;
+}
+
if (noexec || readonlyfs)
return 0;
___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: lockinfo
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hi Kendy, To be considered for inclusion in a CVS FEATURE release, you should provide changes to the doc/cvs.texinfo as well as at least a few test cases (if you don't understand how to work with sanity.sh that is okay, but we need a series of step-by-step commands that illustrate how to verify that everything works with your new feature and how the new feature interacts with existing features). For this particular addition, I have a problem with how it works. Assumption: I have a repository where there exists one directory called Private that is 'read-locked' such that userA is able to checkout directory Private and userB is not. If userB starts a 'cvs checkout -d top .' command. What should you expect to happen: a) no files are checked out because Private will fail and that 'should' stop the checkout command from providing partial information. b) all directories that are processed up to, but not including the Private directory are checked out. However, any files that would logically have been checked out of directories after Private are not checked out because the directory traversal will stop when Private is first reached. c) all directories other than Private will be checked out of the repository and the user will receive a Lock Info failed message that they may or may not notice indicating that the Private directory was not checked out. With your current patch, I believe userB will observe behavior 'b', but I am not sure if that is true or not. It is not clear to me if behavior 'a' or behavior 'c' is the correct behavior, but I believe behavior 'b' is not desirable. Thanks, -- Mark -BEGIN PGP SIGNATURE- Version: GnuPG v1.2.3 (FreeBSD) iD8DBQFCe5bB3x41pRYZE/gRAijIAKCjSmhK3qaFKhr4hSzfMSGE7+OhqgCg1b3I KE0BhXnAAZJN2GdnfpgijmU= =dQ2J -END PGP SIGNATURE- ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
