Re: [bug-gnulib] New GNULIB glob module?
One other remark about glob.c. You should remove this comment, as it's obsolete now: /* Comment out all this code if we are using the GNU C Library, and are not actually compiling the library itself. This code is part of the GNU C Library, but also included in many other GNU distributions. Compiling and linking in this code is a waste when using the GNU C library (especially if it is a shared library). Rather than having every GNU program understand `configure --with-gnu-libc' and omit the object files, it is simpler to just do this in the source for each such file. */ Now for comments on m4/glob.m4 and modules/glob. This finishes up this round of reviewing. gl_GLOB_SUBSTITUTE and gl_GLOB both invoke gl_PREREQ_GLOB. This is overkill, surely. I'd say only the former needs to invoke it. > + AC_CHECK_HEADERS([glob.h gnu-versions.h], [], [GLOB_H=glob.h]) > + > + if test -z "$GLOB_H"; then > +AC_EGREP_CPP([gl_gnu_glob_version = 1$], > +[#include > +gl_gnu_glob_version = _GNU_GLOB_INTERFACE_VERSION], > + [], [GLOB_H=glob.h]) > + fi This seems a bit brittle. Why not simply try to compile this program? #include char a[_GNU_GLOB_INTERFACE_VERSION == 1 ? 1 : -1]; > + AC_CHECK_HEADERS_ONCE([dirent.h sys/ndir.h sys/dir.h ndir.h])dnl Substitute AC_REQUIRE([AC_HEADER_DIRENT]) for this line. > + AC_CHECK_FUNCS_ONCE([dirent64 getlogin_r getpwnam_r stat64])dnl The other three functions are fine, but: dirent64 is a function? I thought it was a structure tag. Surely you mean to use AC_CHECK_TYPE([struct dirent64]), and for the code to to refer to HAVE_STRUCT_DIRENT64 rather than HAVE_DIRENT64? Come to think of it, how does the code currently work here? In the normal case (where the maintainer has not used configure's --disable-largefile option, and where the system supports a 64-bit file interface) we want glob to use the 64-bit file interface. Is that what is happening here? I can't quite follow it. > +++ modules/glob 13 May 2005 16:23:46 - > @@ -0,0 +1,43 @@ > +Description: > +Search for files and directories with paths matching a pattern. > + > +Depends-on: > +alloca > +extensions > +fnmatch > +mempcpy > +realloc I don't see why this module depends on the realloc module. It doesn't ever realloc anything to size zero. > +License: > +??? LGPL ??? Yes, it's LGPL. ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: [bug-gnulib] New GNULIB glob module?
Now for commentary on glob.h. Derek Price <[EMAIL PROTECTED]> writes: > --- ../glibc-2.3.5/posix/glob.h 2004-09-16 20:55:15.0 -0400 > +++ lib/glob_.h 2005-05-13 12:21:39.0 -0400 > @@ -19,29 +19,48 @@ > #ifndef _GLOB_H > #define _GLOB_H 1 > > -#include > +#ifdef _LIBC > +# include > +#else > +# include > +# include > +# undef __size_t > +# define __size_t size_t > +#endif Why do we need to include here? All we need is size_t, right? And stddef.h gives us that. > +/* Some system libraries erroneously define these. */ > +#undef GLOB_ERR > ... Which system libraries are these? Perhaps a comment? > +# define getopt __GLOB_ID (getopt) Surely this line is spurious and can be removed. > -#if !defined __USE_FILE_OFFSET64 || __GNUC__ < 2 > +#if !defined _LIBC || !defined __USE_FILE_OFFSET64 || __GNUC__ < 2 Can't we remove at least the "|| __GNUC__ < 2" here? glibc assumes recent versions of GCC. > extern void __REDIRECT_NTH (globfree, (glob_t *__pglob), globfree64); > #endif > - > #ifdef __USE_LARGEFILE64 > extern int glob64 (__const char *__restrict __pattern, int __flags, > int (*__errfunc) (__const char *, int), Let's omit this change; it's a different matter. ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
Re: [bug-gnulib] New GNULIB glob module?
Derek Price <[EMAIL PROTECTED]> writes: > /* Enable GNU extensions in glob.h. */ > -#ifndef _GNU_SOURCE > +#if defined _LIBC && !defined _GNU_SOURCE > # define _GNU_SOURCE 1 > #endif I just checked the glibc source file include/libc-symbols.h, and it defines both _LIBC and _GNU_SOURCE. So this stuff isn't needed at all; please remove these lines from glob.c instead. > + HAVE_STRUCT_DIRENT_D_TYPE is defined by GNULIB's glob.m4 when the same > + member is found. */ A bit wordy; please rephrase to "HAVE_STRUCT_DIRENT_D_TYPE plays the same rule in gnulib." > - qsort ((__ptr_t) &pglob->gl_pathv[oldcount], > + qsort ((void *) &pglob->gl_pathv[oldcount], You can remove the cast entirely, right? > - free ((__ptr_t) pglob->gl_pathv[pglob->gl_offs + i]); > - free ((__ptr_t) pglob->gl_pathv); > + free ((void *) pglob->gl_pathv[pglob->gl_offs + i]); > + free ((void *) pglob->gl_pathv); Similarly, these casts aren't needed. - free ((__ptr_t) array[--i]); + free ((void *) array[--i]); Nor these. > - free ((__ptr_t) array[--i]); > + free ((void *) array[--i]); Nor this one. > + p = mempcpy ((void *) new->name, name, len); Nor this one. > - free ((__ptr_t) names->name); > + free ((void *) names->name); Nor this. > - return (((flags & GLOB_ALTDIRFUNC) > -? (*pglob->gl_stat) (fullname, &st) > -: __stat64 (fullname, &st64)) == 0); > + return ((flags & GLOB_ALTDIRFUNC) > + ? (*pglob->gl_stat) (fullname, &st) == 0 && S_ISDIR (st.st_mode) > + : __stat64 (fullname, &st64) == 0 && S_ISDIR (st64.st_mode)); As long as you're reparenthesizing you might as well get rid of the unnecessary ones around (flags & GLOB_ALTDIRFUNC). > + new->name = malloc (len + 1 > + + ((flags & GLOB_MARK) && isdir)); This line is 80 columns; should probably shrink it to 78, e.g., via. new->name = malloc (...); glob.c is looking pretty good now. I gotta run now, but I'll look at glob.h later today I hope. ___ Bug-cvs mailing list Bug-cvs@gnu.org http://lists.gnu.org/mailman/listinfo/bug-cvs
[PATCH] always treat locally-added files as Modified
This patches a very obscure condition involving a merge followed
by toggling the CVS/Root file followed by diff -N. Patch is
against current (13 May 2005) cvs HEAD.
I grant permission to distribute this patch under the terms of
the GNU Public License.
=
SETUP
=
* File 'file1' has been added on branch B between tags T1 and T2
* Merge T1-T2 via cvs -d /local/repo co -jT1 -jT2 mydir
(it is important to check out from a local directory, not remote)
* Hand-update CVS/Root to :fork:/local/repo or :/remote/repo
(or just add "-d " to CVS command line)
* try to get diffs: cd mydir && cvs diff -N file1
Depending on the version of CVS, the diff will fail with:
cvs server: extra operand
cvs server: Try `diff --help' for more information.
or:
cvs [diff aborted]: could not get info for `file1': No such file or
directory
ANALYSIS
When checking out from a remote (server) repository, CVS/Entries
gets "dummy timestamp" for file1. That's good. When checking
out from a local repository, CVS/Entries gets a genuine timestamp.
Not so good.
When running diff, CVS client.c compares the timestamp and decides
that file1 is unchanged... and sends "Unchanged file1" to the
server. I haven't chased down why exactly this matters, but
it does cause the server to choke.
SOLUTION
src/client.c : always treat a locally-added file (vn_user=="0")
as modified.
This may not be the best solution. It may also be desirable
to set CVS/Entries timestamp to "dummy timestamp" when checking
out locally; and/or to track down the server code that has
trouble with "Unchanged". This solution has the benefit of
being simple to code, simple to test, and not breaking sanity.
It also seems like a proper thing for the code to do.
--
Ed Santiago Toolsmith [EMAIL PROTECTED]
Index: src/ChangeLog
===
RCS file: /cvs/ccvs/src/ChangeLog,v
retrieving revision 1.3191
diff -u -r1.3191 ChangeLog
--- src/ChangeLog 11 May 2005 20:01:47 - 1.3191
+++ src/ChangeLog 13 May 2005 19:39:57 -
@@ -1,3 +1,7 @@
+2005-05-13 Eduardo Santiago <[EMAIL PROTECTED]>
+
+ * client.c (send_fileproc): Treat locally-added file as Is-modified.
+
2005-05-11 Derek Price <[EMAIL PROTECTED]>
* cvs.h (find_files): New proto.
Index: src/client.c
===
RCS file: /cvs/ccvs/src/client.c,v
retrieving revision 1.423
diff -u -r1.423 client.c
--- src/client.c22 Mar 2005 13:19:57 - 1.423
+++ src/client.c13 May 2005 19:06:27 -
@@ -4461,6 +4461,7 @@
}
else if (vers->ts_rcs == NULL
|| args->force
+|| (vers->vn_user && vers->vn_user[0] == '0') /* Locally added? */
|| strcmp (vers->ts_user, vers->ts_rcs) != 0)
{
if (args->no_contents
Index: src/sanity.sh
===
RCS file: /cvs/ccvs/src/sanity.sh,v
retrieving revision 1.1061
diff -u -r1.1061 sanity.sh
--- src/sanity.sh 11 May 2005 20:01:47 - 1.1061
+++ src/sanity.sh 13 May 2005 19:06:55 -
@@ -10113,6 +10113,34 @@
R file3
R file4'
+ # 2005 May 13: obscure effect of:
+ #
+ # cvs {checkout|update} -j... -j... [brings in file1 as 'A'dded]
+ #
+ # CVS/Entries gets a different result depending on whether
+ # the checkout/update was done *locally* or *via server*:
+ #
+ # local : /file1/0/Fri May 13 17:36:18 2005//
+ # server : /file1/0/dummy timestamp//
+ #
+ # Normally not a problem. But if the checkout is local, and
+ # CVS/Root is subsequently changed to server (as typically
+ # happens in replicated CVS mirror situations), 'cvs diff -N file1'
+ # barfs with:
+ #
+ # cvs [diff aborted]: could not get info for `file1': No such file or
directory
+ #
+ if $remote; then :; else
+ dotest_fail join-xx "${testcvs} -d :fork:${CVSROOT} diff -N
file1" \
+'Index: file1
+===
+RCS file: file1
+diff -N file1
+0a1
+> first branch revision of file1'
+ fi
+
+
# Modify file4 locally, and do an update with a merge.
cd ../../1/first-dir
echo 'third revision of file4' > file4
___
Bug-cvs mailing list
Bug-cvs@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-cvs
