[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Patch for "watch add" bug for review
|
From: |
Jim Hyslop |
|
Subject: |
Patch for "watch add" bug for review |
|
Date: |
Wed, 27 Jul 2005 21:16:34 -0400 |
|
User-agent: |
Mozilla Thunderbird 1.0.2 (Windows/20050317) |
Hi, all
To refresh memories: there's a bug in the trunk version, when 'cvs watch
add' is issued without any options in an empty directory, it clears any
existing watches. The bug was fixed in stable, but there were other
problems in the feature branch. I've finally got around to fixing the
problem on the trunk.
edit.c fixes the original problem. The remaining files fix the related
problem with watch add, including the patch to expand_wild discussed
earlier this week (I did not notice any particular problems with other
commands when the patch was implemented). Basically, the problem boils
down to difficulties determining when to actually apply the default
attributes.
A brief explanation of the approach I took: a single variable indicating
whether or not to set the default attributes is insufficient. It fails
on a command such as:
cvs watch add afile adirectory
So, we now keep track of the directories specified on the command line,
and in the addremove_filesdoneproc, if the current directory matches one
of the directories specified, then we set the default attribute.
Could someone please review the following patch for any problems? If
nobody points out any problems, I'll commit the patch (along with
release notes, etc.) later this week. Hopefully the line-wrapping issues
won't get in the way too much.
Index: edit.c
===================================================================
RCS file: /cvs/ccvs/src/edit.c,v
retrieving revision 1.85
diff -u -r1.85 edit.c
--- edit.c 8 Apr 2005 18:17:01 -0000 1.85
+++ edit.c 28 Jul 2005 00:53:34 -0000
@@ -32,6 +32,7 @@
static int
onoff_fileproc (void *callerdat, struct file_info *finfo)
{
+ fileattr_get0 (finfo->file, "_watched");
fileattr_set (finfo->file, "_watched", turning_on ? "" : NULL);
return 0;
}
@@ -43,7 +44,10 @@
const char *update_dir, List *entries)
{
if (setting_default)
+ {
+ fileattr_get0 (NULL, "_watched");
fileattr_set (NULL, "_watched", turning_on ? "" : NULL);
+ }
return err;
}
Index: sanity.sh
===================================================================
RCS file: /cvs/ccvs/src/sanity.sh,v
retrieving revision 1.1069
diff -u -r1.1069 sanity.sh
--- sanity.sh 10 Jun 2005 19:03:19 -0000 1.1069
+++ sanity.sh 28 Jul 2005 00:55:11 -0000
@@ -1627,7 +1627,7 @@
tests="${tests} close-stdout"
tests="$tests debug-log-nonfatal"
# Watches, binary files, history browsing, &c.
- tests="${tests} devcom devcom2 devcom3 watch4 watch5"
+ tests="${tests} devcom devcom2 devcom3 watch4 watch5 watch6-0 watch6"
tests="${tests} edit-check"
tests="${tests} unedit-without-baserev"
tests="${tests} ignore ignore-on-branch binfiles binfiles2 binfiles3"
@@ -16961,6 +16961,136 @@
modify_repo rm -rf $CVSROOT_DIRNAME/first-dir
;;
+ watch6-0)
+
+ # Make sure that default attributes are being set properly.
+ # Specifying a directory has, it seems, never worked,
+ # and 1.12.10 broke it completely.
+ mkdir watch6-0; cd watch6-0
+
+ dotest watch6-0-setup-1 "$testcvs -Q co -ldtop ."
+ cd top
+ mkdir watch6-0
+ dotest watch6-0-setup-2 "$testcvs -Q add watch6-0"
+ cd watch6-0
+ dotest watch6-0-1 "$testcvs watch add"
+ dotest watch6-0-2 "grep -qE '^D'
$CVSROOT_DIRNAME/watch6-0/CVS/fileattr"
+ dotest watch6-0-3 "$testcvs watch remove"
+ dotest_fail watch6-0-4 "grep -qE '^D'
$CVSROOT_DIRNAME/watch6-0/CVS/fileattr 2>/dev/null"
+
+ dotest watch6-0-5 "$testcvs watch add ."
+ dotest watch6-0-6 "grep -qE '^D'
$CVSROOT_DIRNAME/watch6-0/CVS/fileattr"
+ dotest watch6-0-7 "$testcvs watch remove ."
+ dotest_fail watch6-0-8 "grep -qE '^D'
$CVSROOT_DIRNAME/watch6-0/CVS/fileattr 2>/dev/null"
+
+ # OK, basic add/remove work. Now, make sure it works with named
directories
+ mkdir dir1
+ mkdir dir2
+ mkdir dir3
+ echo afile>afile
+ $testcvs -Q add afile dir1 dir2 dir3
+ $testcvs -Q ci -m "Adding test files"
+
+ # Current directory should not be watched, but there should be a
watch on the file,
+ # and on dir1 & dir2, but not on dir3.
+ dotest watch6-0-9 "$testcvs -Q watch add afile dir1 dir2"
+ dotest_fail watch6-0-10 "grep -qE '^D'
$CVSROOT_DIRNAME/watch6-0/CVS/fileattr 2>/dev/null"
+ dotest watch6-0-11 "grep -qE '^Fafile'
$CVSROOT_DIRNAME/watch6-0/CVS/fileattr"
+ dotest watch6-0-12 "grep -qE '^D'
$CVSROOT_DIRNAME/watch6-0/dir1/CVS/fileattr"
+ dotest watch6-0-13 "grep -qE '^D'
$CVSROOT_DIRNAME/watch6-0/dir2/CVS/fileattr"
+ dotest_fail watch6-0-12 "grep -qE '^D'
$CVSROOT_DIRNAME/watch6-0/dir3/CVS/fileattr 2>/dev/null"
+
+ if $keep; then
+ echo Keeping $TESTDIR and exiting due to --keep
+ exit 0
+ fi
+ cd ../../..
+ rm -rf watch6
+ rm -rf $CVSROOT_DIRNAME/watch6
+
+ ;;
+
+ watch6)
+ # Check that `cvs watch on' does not reset the fileattr file.
+ mkdir watch6; cd watch6
+
+ dotest watch6-setup-1 "$testcvs -Q co -ldtop ."
+ cd top
+ mkdir watch6
+ dotest watch6-setup-2 "$testcvs -Q add watch6"
+
+ # I don't recall why I had these next 3 lines.
+ cd ..
+ dotest watch6-setup-3 "$testcvs -Q co watch6"
+ cd watch6
+
+ mkdir subdir
+ dotest watch6-setup-4 "$testcvs -Q add subdir"
+ cd subdir
+
+ # START watch add/remove sequence
+ dotest watch6-1 "$testcvs -Q watch add"
+ dotest watch6-2 \
+"grep '_watchers' $CVSROOT_DIRNAME/watch6/subdir/CVS/fileattr >/dev/null"
+
+ dotest watch6-3 "$testcvs watch on"
+ dotest watch6-4 \
+"grep '_watchers' $CVSROOT_DIRNAME/watch6/subdir/CVS/fileattr >/dev/null"
+ dotest watch6-5 \
+"grep '_watched' $CVSROOT_DIRNAME/watch6/subdir/CVS/fileattr >/dev/null"
+
+ dotest watch6-6 "$testcvs watch off"
+ dotest watch6-7 \
+"grep '_watchers' $CVSROOT_DIRNAME/watch6/subdir/CVS/fileattr >/dev/null"
+ dotest_fail watch6-8 \
+"grep '_watched' $CVSROOT_DIRNAME/watch6/subdir/CVS/fileattr >/dev/null"
+
+ dotest watch6-9 "$testcvs watch remove"
+ dotest_fail watch6-10 \
+"test -d $CVSROOT_DIRNAME/test-directory/subdir/CVS"
+ dotest_fail watch6-11 \
+"test -f $CVSROOT_DIRNAME/test-directory/subdir/CVS/fileattr"
+ # END watch add/remove sequence
+
+ echo Hi there >afile
+ dotest watch6-12 "$testcvs -Q add afile"
+ dotest watch6-13 "$testcvs ci -m 'A file' afile" \
+"$CVSROOT_DIRNAME/watch6/subdir/afile,v <-- afile
+initial revision: 1.1"
+
+ # START watch add/remove sequence
+ dotest watch6-14 "$testcvs -Q watch add"
+ dotest watch6-15 \
+"grep '_watchers' $CVSROOT_DIRNAME/watch6/subdir/CVS/fileattr >/dev/null"
+
+ dotest watch6-16 "$testcvs watch on"
+ dotest watch6-17 \
+"grep '_watchers' $CVSROOT_DIRNAME/watch6/subdir/CVS/fileattr >/dev/null"
+ dotest watch6-18 \
+"grep '_watched' $CVSROOT_DIRNAME/watch6/subdir/CVS/fileattr >/dev/null"
+
+ dotest watch6-19 "$testcvs watch off"
+ dotest watch6-20 \
+"grep '_watchers' $CVSROOT_DIRNAME/watch6/subdir/CVS/fileattr >/dev/null"
+ dotest_fail watch6-21 \
+"grep '_watched' $CVSROOT_DIRNAME/watch6/subdir/CVS/fileattr >/dev/null"
+
+ dotest watch6-22 "$testcvs watch remove"
+ dotest_fail watch6-23 \
+"test -d $CVSROOT_DIRNAME/test-directory/subdir/CVS"
+ dotest_fail watch6-24 \
+"test -f $CVSROOT_DIRNAME/test-directory/subdir/CVS/fileattr"
+ # END watch add/remove sequence
+
+ if $keep; then
+ echo Keeping $TESTDIR and exiting due to --keep
+ exit 0
+ fi
+ cd ../../..
+ rm -r watch6
+ rm -rf $CVSROOT_DIRNAME/watch6
+ ;;
+
edit-check)
Index: watch.c
===================================================================
RCS file: /cvs/ccvs/src/watch.c,v
retrieving revision 1.43
diff -u -r1.43 watch.c
--- watch.c 20 Aug 2004 21:02:31 -0000 1.43
+++ watch.c 28 Jul 2005 00:55:12 -0000
@@ -54,6 +54,8 @@
int add_tunedit_pending;
int add_tcommit_pending;
+ TRACE( TRACE_FUNCTION, "modify_watchers ( %s )", file );
+
who = getcaller ();
who_len = strlen (who);
@@ -222,30 +224,57 @@
return 0;
}
+static int addremove_filesdoneproc (void * callerdat, int err, const
char * repository,
+ const char *update_dir, List
* entries)
+{
+ int set_default = the_args.setting_default;
+ int dir_check = 0;
+
+ while ( !set_default && dir_check < the_args.num_dirs )
+ {
+ /* If we are recursing, then just see if the first part of update_dir
+ matches any of the specified directories. Otherwise, it must be an
exact
+ match. */
+ if ( the_args.local )
+ set_default = strcmp( update_dir, the_args.dirs[ dir_check ] )==0;
+ else
+ set_default = strncmp( update_dir, the_args.dirs[ dir_check ],
strlen( the_args.dirs[ dir_check ] ) ) == 0;
+ dir_check++;
+ }
+
+ if (set_default)
+ watch_modify_watchers (NULL, &the_args);
+ return err;
+}
static int
watch_addremove (int argc, char **argv)
{
int c;
- int local = 0;
int err;
int a_omitted;
+ int arg_index;
+ int max_dirs;
a_omitted = 1;
the_args.commit = 0;
the_args.edit = 0;
the_args.unedit = 0;
+ the_args.num_dirs = 0;
+ the_args.dirs = NULL;
+ the_args.local = 0;
+
optind = 0;
while ((c = getopt (argc, argv, "+lRa:")) != -1)
{
switch (c)
{
case 'l':
- local = 1;
+ the_args.local = 1;
break;
case 'R':
- local = 0;
+ the_args.local = 0;
break;
case 'a':
a_omitted = 0;
@@ -279,6 +308,25 @@
argc -= optind;
argv += optind;
+ the_args.num_dirs = 0;
+ max_dirs = 4; /* Arbitrary choice. */
+ the_args.dirs = xmalloc( sizeof( const char * ) * max_dirs );
+
+ TRACE (TRACE_FUNCTION, "watch_addremove (%d)", argc);
+ for ( arg_index=0; arg_index<argc; ++arg_index )
+ {
+ TRACE( TRACE_FUNCTION, "\t%s", argv[ arg_index ]);
+ if ( isdir( argv[ arg_index ] ) )
+ {
+ if ( the_args.num_dirs >= max_dirs )
+ {
+ max_dirs *= 2;
+ the_args.dirs = (const char ** )xrealloc( (void *)the_args.dirs,
max_dirs );
+ }
+ the_args.dirs[ the_args.num_dirs++ ] = argv[ arg_index ];
+ }
+ }
+
if (a_omitted)
{
the_args.edit = 1;
@@ -292,7 +340,7 @@
start_server ();
ign_setup ();
- if (local)
+ if (the_args.local)
send_arg ("-l");
/* FIXME: copes poorly with "all" if server is extended to have
new watch types and client is still running an old version. */
@@ -305,7 +353,7 @@
if (!the_args.edit && !the_args.unedit && !the_args.commit)
option_with_arg ("-a", "none");
send_arg ("--");
- send_files (argc, argv, local, 0, SEND_NO_CONTENTS);
+ send_files (argc, argv, the_args.local, 0, SEND_NO_CONTENTS);
send_file_names (argc, argv, SEND_EXPAND_WILD);
send_to_server (the_args.adding ?
"watch-add\012" : "watch-remove\012",
@@ -316,14 +364,17 @@
the_args.setting_default = (argc <= 0);
- lock_tree_promotably (argc, argv, local, W_LOCAL, 0);
+ lock_tree_promotably (argc, argv, the_args.local, W_LOCAL, 0);
err = start_recursion
- (addremove_fileproc, NULL, NULL, NULL, NULL,
- argc, argv, local, W_LOCAL, 0, CVS_LOCK_WRITE,
+ (addremove_fileproc, addremove_filesdoneproc, NULL, NULL, NULL,
+ argc, argv, the_args.local, W_LOCAL, 0, CVS_LOCK_WRITE,
NULL, 1, NULL);
Lock_Cleanup ();
+ free( (void *)the_args.dirs );
+ the_args.dirs = NULL;
+
return err;
}
Index: watch.h
===================================================================
RCS file: /cvs/ccvs/src/watch.h,v
retrieving revision 1.7
diff -u -r1.7 watch.h
--- watch.h 20 Aug 2004 21:02:31 -0000 1.7
+++ watch.h 28 Jul 2005 00:55:12 -0000
@@ -38,8 +38,18 @@
/* Should we set the default? This is here for passing among various
routines in watch.c (a good place for it if there is ever any
reason
- to make the stuff reentrant), not for watch_modify_watchers. */
+ to make the stuff reentrant), not for watch_modify_watchers.
+ This is only set if there are no arguments specified, e.g. 'cvs
watch add' */
int setting_default;
+
+ /* List of directories specified on the command line, to set the
+ default attributes. */
+ const char ** dirs;
+ int num_dirs;
+
+ /* Is this recursive? */
+ int local;
+
};
/* Modify the watchers for FILE. *WHAT tells what to do to them.
Index: filesubr.c
===================================================================
RCS file: /cvs/ccvs/windows-NT/filesubr.c,v
retrieving revision 1.58
diff -u -r1.58 filesubr.c
--- filesubr.c 31 May 2005 07:13:31 -0000 1.58
+++ filesubr.c 28 Jul 2005 01:12:35 -0000
@@ -827,13 +827,12 @@
char *last_forw_slash, *last_back_slash, *end_of_dirname;
int dirname_length = 0;
- /* FIXME: If argv[i] is ".", this code will expand it to the
- name of the current directory in its parent directory which
- will cause start_recursion to do all manner of strange things
- with it (culminating in an error). This breaks "cvs co .".
- As nearly as I can guess, this bug has existed since
- expand_wild was first created. At least, it is in CVS 1.9 (I
- just tried it). */
+ if ( strcmp( argv[i], "." ) == 0 )
+ {
+ new_argv[new_argc] = (char *) xmalloc ( 2 );
+ strcpy( new_argv[ new_argc++ ], "." );
+ continue;
+ }
/* FindFirstFile doesn't return pathnames, so we have to do
this ourselves. Luckily, it's no big deal, since globbing
--
Jim
| [Prev in Thread] |
Current Thread |
[Next in Thread] |
- Patch for "watch add" bug for review,
Jim Hyslop <=