Re: 'cvs add' client/server semantics (was Re: Triggers)

[Paul Sander]

On Jan 30, 2005, at 10:24 PM, address@hidden wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Paul Sander <address@hidden> writes:
>
> > Wait a second.  The "OK for addition, but wrong for commit" is exactly
> > the status quo.  The "cvs add" command succeeds, "cvs commit" fails
> > due to commitinfo.  What I'm proposing is "bad for addition, bad for
> > commit", where "cvs add" fails on those occasions when we know at that
> > moment that "cvs commit" will also fail.  That's not to say that we
> > will *always* know at add time that the commit will fail; failures can
> > occur due to problems in their content which are clearly not ready to
> > check at add time.
>
> I find that concrete examples are useful...

[ Reasonable example involving MyFoo.h vs. myfoo.h naming in a mixed 
Unix/Windows shop omitted ]

> Now, I will grant that it is certainly possible for a user to 'fix'
> their checked out tree to work around this problem. However, it may be
> desirable to get it right the first time rather than waste time later.
>
> However, I am not sure how this argument for an advisory trigger at
> 'cvs add' time which then becomes a manditory 'enforcement trigger'
> at commit time would not also be potentially very confusing to the
> users.

I agree that it could be a potential source of confusion for the users. 
 They're confused about the whole concurrent development paradigm, too. 
 Both sources of confusion can be cleared up with the proper training.

Also, I'm recommending that add-time triggers be optional, not 
advisory.  "Optional" means that that they can be turned off.  
"Advisory" means that they can be ignored.  There are two differences 
between the two concepts:  Advisory triggers must always contact the 
server to provide their advice, and they never block an operation.  
Optional triggers contact the server only when they're turned on, and 
they block additions when they're turned on and fail.  I also strongly 
recommend that, if the "cvs add" command is used in such a way that the 
trigger fires and fails, the Entries file not be modified and the exit 
code should indicate an error condition (not a warning).

> At present, it is clear from both sides that the 'cvs add' behavior is
> broken. I have probably missed some of the points, but let me try to
> summarize:
>
>   - a new directory that is added with 'cvs add' does not go thru any
>     peremptory enforcement checks at all (a cvs add trigger OR a policy
>     that cvs add is a local-only operation would partially fix this
>     provided that some other trigger addressed the creation of a new
>     directory in the repository).

I'm not sure I fully understand what was said in this paragraph, but 
I'd like to say that I believe that adding directories should also be 
triggerable events, and that policies enforcing directory additions 
could be potentially different from policies enforcing file additions.  
Expanding on Mark's example, a second policy might require that 
directory names can not contain period (".") characters.  To make this 
happen, the trigger must be aware of the kind of artifact that's being 
added.

>   - related to this, the 'cvs import' functionality is likewise 
> 'broken'
>     as it currently may stomp on existing tags and could create files
>     and directories that are not acceptable to an administrators
>     policies. There are presently no enforcer triggers that prevent a
>     particular user from doing an import into the repository.

This is true, and integrating triggers into import requires a lot of 
thought.  I would suggest as a topic of discussion that import should 
scan the input tree and run all triggers in advance of the first change 
of persistent state.  But this is a very touchy subject because the 
vendors that make the drops may in fact make changes to the tree that 
violate local policy.  The CVS admin is then faced with a dilemma:  
Should the tree be imported as-is for accurate tracking of vendor 
drops, or should the tree be adjusted according to local policy and 
potentially breaking the vendor drop?

>   - there are good reasons for both 'cvs add' and 'cvs rm' to be
>     considered as local operations that should not contact the server:
>
>     a) allows read-only users to generate 'cvs diff -N' output to send
>        to a maintainer

I think this is common only when the user has read-only access to the 
repository, correct?

>     b) lets the local user experiment with re-oranization of the
>        hierarchy of a module. (It is unclear if there would be any
>        easy way to 'rename' files in the local tree and resolve them
>        in a reasonble manner with the cvs server at a later date.)

Depends on what you mean by "resolve".  After mv'ing a directory in a 
workspace to something new, CVS still maps it to its original location 
in the repository.  If you're thinking about somehow having the rename 
be reflected in the repository, then you're treading dangerously close 
to another heated argument.  :-)

>     c) allows a user to continue to do useful work even when access to
>        the server is unavailable (it might be down for maintenance, or
>        the user may be working from a disconnected network).

Yes, this is an issue.

>   - there are good reasons for 'cvs add' to have an advisory process
>     (which becomes an enforcement at cvs commit time)
>
>     a) inform the user early that a proposed addition will fail at such
>        a time that the user does a 'cvs commit' so as to minimize the
>        amount of work that is done under a mistaken assumption that the
>        commit would succeed.

I'd go a little further and say this:  Inform the user early that *an 
attempted* addition will fail at such a time that user does a 'cvs 
commit'...

>     b) provide for a simpler kind of trigger implementation to just
>        implement policy on newly created files at commit time.

I'm not sure this is an issue.  Would you elaborate, please?

>     c) the addition of pre-<event> and post-<event> triggers to cvs
>        commands that presently modify the repository allows more 
> control
>        for a cvs administrator to enforce policy pro-actively. If the
>        'cvs add' commands continues to allow modification of the
>        repository (creation of new directories), then there needs to be
>        a way to catch abuses of policy.

There certainly needs to be a way for the administrator to trap 
violations of policy with regard to the addition of directories.  
Whether or not the directory is actually created in the repository by 
the "cvs add" command is not at issue with regard to the add-time 
trigger topic, but there are other arguments that "cvs add" should not 
in fact create the directory in the repository but rather defer it 
until commit time.  Those arguments are valid.

> So, it comes down to a choice of how should the 'cvs add' command
> operate going forward and where is the correct place to add 'enforcer'
> triggers for the administration of a repository.
>
> I am finding Paul's arguments for additional flexibility for the
> repository administrator to have more proactive control over the
> repository to be an interesting approach. Certainly he is not 
> suggesting
> any client/server protocol change and the addition of a trigger is very
> simple to accomplish. No client changes are required, so only the 
> server
> needs to be upgraded to add the new functionality.
>
> Greg's arguments for not contacting the server on a 'cvs add' would
> require the semantics of a 'cvs commit' to be extended to allow for new
> directories to be created, but one of his new clients should be able to
> do the right thing with an existing server by actually sending the 'cvs
> add' command first for directories (assuming there is an easy way to
> keep track of a locally created directory) and then following with a
> 'cvs commit' later. Although, it seems that a number of places in the
> client will be impacted to remember that a locally created directory
> should not be traversed when talking to the server (cvs log, cvs 
> status,
> cvs diff, cvs history, ...).

I think that traversal can be done as if empty directories were copied 
out from the server.  I assume that when creating a directory, "cvs 
add" conjures up metadata that are undistinguishable from those of an 
empty directory that was updated without pruning.

> Note: It would still be possible for the server-side "cvs add"
> capability to be given an 'enforcement' trigger even with Greg's 
> changes
> that 'cvs add' no longer be contacted by new clients until a
> "cvs commit", but I get the impression that Greg would object to this
> change.
>
> So, from an implementation point of view, it would appear that Paul's
> proposal is easier than Greg's. However, it is not necessarily true 
> that
> taking the 'easy' path is the right thing to do.

'Course, Greg has claimed in the past that he already has a patch...

I happen to think that both changes should be made, though none of my 
use cases makes use of disconnected adds.

> Have I missed any arguments pro or con?
>
> Are there any additional complexities of either implementation change
> that should be considered for protocol compatibility?

I have some questions for Greg regarding his patch:  Assume the 
repository contains a directory tree foo/bar.  Both directories contain 
files.  The user performs the following sequence of commands:

cvs checkout -l foo
cd foo
mkdir bar
cvs add bar
cd bar

At this point, is the workspace in a state in which the user can 
successfully invoke the "cvs add" and "cvs update" commands on new and 
existing files contained by the bar directory, before the next commit?  
What happens if a user invokes "cvs add" to create a file that already 
exists in the repository but not in the workspace, given that the 
client has conjured up its own metadata about the bar directory?

Seems to me that it should just work, but I have to ask.  :-)

--
Paul Sander       | "Lets stick to the new mistakes and get rid of the 
old
address@hidden | ones" -- William Brown