Re: no feedback on snapshot? coreutils-7.5 coming soon
C de-Avillez wrote: On Wed, 2009-08-12 at 23:06 +0200, Jim Meyering wrote: C de-Avillez wrote: Yet another one, on check-root. I am starting to wonder if this may be a problem with my setup, since nobody has reported errors on tail. FAIL: tail-2/append-only (exit: 1) == Thanks for the reports. How did you run those tests? When I do it like this (per README), they all pass. sudo env PATH=$PATH NON_ROOT_USERNAME=$USER make -k check-root Yes, I did run them this way. I have just re-run them under git master (so that I would have the current status, without Pádraig's fixes). It failed the same. The following is the command line I used: hg...@xango2:/usr/src/buildd/coreutils $ sudo env PATH=$PATH NON_ROOT_USERNAME=$USER make check-root And here's the log: FAIL: tail-2/append-only (exit: 1) == ... + chattr +a f + echo x + test 1 = 0 + fail=0 + sleep 1 + pid=29813 + tail --pid=29813 -f f + fail=1 What type of file system is that? (i.e., run this: df -hT .) Please try again, but without redirecting output to /dev/null: sleep 1 pid=$! tail --pid=$pid -f f || fail=1 i.e., apply this patch: diff --git a/tests/tail-2/append-only b/tests/tail-2/append-only index 0395271..0b4a959 100755 --- a/tests/tail-2/append-only +++ b/tests/tail-2/append-only @@ -39,7 +39,7 @@ fail=0 sleep 1 pid=$! -tail --pid=$pid -f f /dev/null 21 || fail=1 +tail --pid=$pid -f f || fail=1 chattr -a f 2/dev/null Exit $fail
Re: no feedback on snapshot? coreutils-7.5 coming soon
Jim Meyering wrote: C de-Avillez wrote: FAIL: tail-2/append-only (exit: 1) == ... + chattr +a f + echo x + test 1 = 0 + fail=0 + sleep 1 + pid=29813 + tail --pid=29813 -f f + fail=1 What type of file system is that? (i.e., run this: df -hT .) I think this was also fixed with my inotify fixup patch. cheers, Pádraig.
Re: no feedback on snapshot? coreutils-7.5 coming soon
Pádraig Brady wrote: Jim Meyering wrote: C de-Avillez wrote: FAIL: tail-2/append-only (exit: 1) == ... + chattr +a f + echo x + test 1 = 0 + fail=0 + sleep 1 + pid=29813 + tail --pid=29813 -f f + fail=1 What type of file system is that? (i.e., run this: df -hT .) I think this was also fixed with my inotify fixup patch. Oh. Good! Thanks.
Re: no feedback on snapshot? coreutils-7.5 coming soon
On Sun, 2009-08-16 at 13:22 +0100, Pádraig Brady wrote: Jim Meyering wrote: C de-Avillez wrote: FAIL: tail-2/append-only (exit: 1) == ... + chattr +a f + echo x + test 1 = 0 + fail=0 + sleep 1 + pid=29813 + tail --pid=29813 -f f + fail=1 What type of file system is that? (i.e., run this: df -hT .) I think this was also fixed with my inotify fixup patch. Seems to, since I cannot repeat it anymore. But, for the record, the FS is an ext3. Cheers, ..C.. signature.asc Description: This is a digitally signed message part
Re: no feedback on snapshot? coreutils-7.5 coming soon
Pádraig Brady wrote: I was wondering about printing the warning, and was wary about now silently not preserving symlink times. I.E. being silently inconsistent. I guess it's better to be quiet in this case? That also means I can reinstate the mv/part-symlink test on systems without utimensat(). Updated patch attached. I too had my doubts initially, but then noticed that rsync is also silent about this. I pushed your change with a few small log/comment tweaks. Thanks again! From f0a1f0df2295aaed418f201bbec67df120e1ac17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= p...@draigbrady.com Date: Thu, 13 Aug 2009 10:39:10 +0100 Subject: [PATCH] cp,mv: fix issues with preserving timestamps of copied symlinks * src/copy.c (copy_internal): On systems without utimensat don't use utimens on a symlink, as that would dereference the symlink. * tests/cp/abuse: To work around possible attribute preservation failures breaking the test, use cp -dR rather than cp -a. --- src/copy.c | 19 +++ tests/cp/abuse |2 +- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/copy.c b/src/copy.c index bed90c4..bf9230b 100644 --- a/src/copy.c +++ b/src/copy.c @@ -118,18 +118,18 @@ static bool owner_failure_ok (struct cp_options const *x); static char const *top_level_src_name; static char const *top_level_dst_name; -/* Wrap utimensat-with-AT_FDCWD and utimens, to keep these - cpp directives out of the main code. */ +/* Set the timestamp of symlink, FILE, to TIMESPEC. + If this system lacks support for that, simply return 0. */ static inline int -utimensat_if_possible (char const *file, struct timespec const *timespec) +utimens_symlink (char const *file, struct timespec const *timespec) { - return #if HAVE_UTIMENSAT -utimensat (AT_FDCWD, file, timespec, AT_SYMLINK_NOFOLLOW) + return utimensat (AT_FDCWD, file, timespec, AT_SYMLINK_NOFOLLOW); #else -utimens (file, timespec) + /* Don't set errno=ENOTSUP here as we don't want + to output an error message for this case. */ + return 0; #endif -; } /* Perform the O(1) btrfs clone operation, if possible. @@ -2117,7 +2117,10 @@ copy_internal (char const *src_name, char const *dst_name, timespec[0] = get_stat_atime (src_sb); timespec[1] = get_stat_mtime (src_sb); - if (utimensat_if_possible (dst_name, timespec) != 0) + if ((dest_is_symlink + ? utimens_symlink (dst_name, timespec) + : utimens (dst_name, timespec)) + != 0) { error (0, errno, _(preserving times for %s), quote (dst_name)); if (x-require_preserve) diff --git a/tests/cp/abuse b/tests/cp/abuse index 285c531..e9086b8 100755 --- a/tests/cp/abuse +++ b/tests/cp/abuse @@ -37,7 +37,7 @@ for i in dangling-dest existing-dest; do test $i = existing-dest echo i t test $i = dangling-dest rm -f t - cp -a a/1 b/1 c 2 out fail=1 + cp -dR a/1 b/1 c 2 out fail=1 compare out exp || fail=1 -- 1.6.4.357.gfd68c
Re: no feedback on snapshot? coreutils-7.5 coming soon
Jim Meyering jim at meyering.net writes: AFAIK, I am the only one who has built the latest snapshot: http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/17604 Though it's been only two days. Unless I hear of new bug reports or portability problems soon, expect coreutils-7.5 to be released in the next few days. On cygwin, 'make' completes, but 'make check' issues a warning (and fails if I also add -Werror to CFLAGS): ccache gcc -std=gnu99 -gdwarf-2 -Wall -Werror -Wl,--as-needed -o stdbuf.exe stdbuf.o libver.a ../lib/libcoreutils.a /usr/local/lib/libintl.dll.a -liconv - L/usr/local/lib ../lib/libcoreutils.a -liconv diff progs-makefile progs-readme rm -rf progs-readme progs-makefile ccache gcc -std=gnu99 -I. -I../lib -I../lib -I/usr/local/include -fPIC - gdwarf-2 -Wall -Werror -MT libstdbuf_so-libstdbuf.o -MD -MP - MF .deps/libstdbuf_so-libstdbuf.Tpo -c -o libstdbuf_so-libstdbuf.o `test - f 'libstdbuf.c' || echo './'`libstdbuf.c libstdbuf.c:1: warning: -fPIC ignored for target (all code is position independent) And even if I don't turn on -Werror, 'make check' dies further on: CCLD libstdbuf.so.exe libstdbuf_so-libstdbuf.o: In function `apply_mode': /home/eblake/coreutils/src/libstdbuf.c:108: undefined reference to `_libintl_gettext' /home/eblake/coreutils/src/libstdbuf.c:116: undefined reference to `_libintl_gettext' /home/eblake/coreutils/src/libstdbuf.c:124: undefined reference to `_libintl_gettext' (wow - it really seems like this should be creating libstdbuf.dll or even cygstdbuf.dll, per cygwin naming conventions; not libstdbuf.so.exe). I'm not sure whether libstdbuf will even work for cygwin (as it is not ELF); and part of me is afraid that we would have to use libtool to get this to work portably. Maybe it is easiest to just figure out how to disable stdbuf from even being attempted, let along tested, on cygwin, until I can find more time to investigate it further? I'm now running 'make -k check', and will try to spot any obvious problems. I already have some effort for porting things like freopen O_BINARY issues over to cygwin that I have not yet pushed upstream. But I maintain my stance that, as in the past, I don't want you to feel obligated to hold up the release process just to cater to cygwin. -- Eric Blake
Re: no feedback on snapshot? coreutils-7.5 coming soon
Eric Blake wrote: Jim Meyering jim at meyering.net writes: AFAIK, I am the only one who has built the latest snapshot: http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/17604 Though it's been only two days. Unless I hear of new bug reports or portability problems soon, expect coreutils-7.5 to be released in the next few days. On cygwin, 'make' completes, but 'make check' issues a warning (and fails if I also add -Werror to CFLAGS): ccache gcc -std=gnu99 -gdwarf-2 -Wall -Werror -Wl,--as-needed -o stdbuf.exe stdbuf.o libver.a ../lib/libcoreutils.a /usr/local/lib/libintl.dll.a -liconv - L/usr/local/lib ../lib/libcoreutils.a -liconv diff progs-makefile progs-readme rm -rf progs-readme progs-makefile ccache gcc -std=gnu99 -I. -I../lib -I../lib -I/usr/local/include -fPIC - gdwarf-2 -Wall -Werror -MT libstdbuf_so-libstdbuf.o -MD -MP - MF .deps/libstdbuf_so-libstdbuf.Tpo -c -o libstdbuf_so-libstdbuf.o `test - f 'libstdbuf.c' || echo './'`libstdbuf.c libstdbuf.c:1: warning: -fPIC ignored for target (all code is position independent) And even if I don't turn on -Werror, 'make check' dies further on: CCLD libstdbuf.so.exe libstdbuf_so-libstdbuf.o: In function `apply_mode': /home/eblake/coreutils/src/libstdbuf.c:108: undefined reference to `_libintl_gettext' /home/eblake/coreutils/src/libstdbuf.c:116: undefined reference to `_libintl_gettext' /home/eblake/coreutils/src/libstdbuf.c:124: undefined reference to `_libintl_gettext' (wow - it really seems like this should be creating libstdbuf.dll or even cygstdbuf.dll, per cygwin naming conventions; not libstdbuf.so.exe). I'm not sure whether libstdbuf will even work for cygwin (as it is not ELF); Thanks for the feedback! In that case, the configure-time check for is-an-ELF-system must be failing on cygwin: # Limit stdbuf to ELF systems with GCC optional_pkglib_progs= AC_MSG_CHECKING([whether this is an ELF system]) AC_EGREP_CPP([yes], [#if __ELF__ yes #endif], [elf_sys=yes], [elf_sys=no]) AC_MSG_RESULT([$elf_sys]) if test $elf_sys = yes \ test $GCC = yes; then gl_ADD_PROG([optional_bin_progs], [stdbuf]) gl_ADD_PROG([optional_pkglib_progs], [libstdbuf.so]) fi and part of me is afraid that we would have to use libtool to get this to work Right. we want to avoid that. portably. Maybe it is easiest to just figure out how to disable stdbuf from even being attempted, let along tested, on cygwin, until I can find more time to investigate it further? I'm now running 'make -k check', and will try to spot any obvious problems. I already have some effort for porting things like freopen O_BINARY issues over to cygwin that I have not yet pushed upstream. But I maintain my stance that, as in the past, I don't want you to feel obligated to hold up the release process just to cater to cygwin. Thanks.
Re: no feedback on snapshot? coreutils-7.5 coming soon
Jim Meyering jim at meyering.net writes: In that case, the configure-time check for is-an-ELF-system must be failing on cygwin: No: configure:43745: checking whether this is an ELF system configure:43761: result: no But there might be some missing holes where 'make check' tries to build stdbuf without regards to the result of the configure results. I'm now running 'make -k check', and will try to spot any obvious problems. So far, I'm seeing a suspicious failure: tests/misc/factor.log: factor (GNU coreutils) 7.4.121-ea5b7 ... z... factor: test z: stderr mismatch, comparing z.E (actual) and z.1 (expected) *** z.E Fri Aug 14 10:36:48 2009 --- z.1 Fri Aug 14 10:36:48 2009 *** *** 1,2 ! factor: unknown option -- 1 Try `factor --help' for more information. --- 1,2 ! factor: invalid option -- 1 Try `factor --help' for more information. cont... I'm not sure whether that might be related to the getopt vs. getopt-gnu change though (I've been building incrementally, and may have picked up some state where cygwin's getopt snuck into usage for a while; I'm going to repeat with a pristine bootstrap to see if the problem still persists). The test is still running. Meanwhile, there are a number of failures probably due to the fact that cygwin Admin users can read and delete files that are otherwise unreadable from POSIX permission standpoint (such as rm/rm1). But this is no different than in the past; I've been meaning to find time to write some sort of test filter that checks for superuser privileges to skip these sorts of tests, and I can also try and figure out how to repeat the testsuite run under a different user without Admin rights. Again, don't hold up the release waiting for me (I've had a lot on my plate lately on other fronts, and I maintain the cygwin port of coreutils so whatever doesn't make it upstream will still make it by the time I complete the cygwin packaging). -- Eric Blake
Re: no feedback on snapshot? coreutils-7.5 coming soon
Eric Blake wrote: Jim Meyering jim at meyering.net writes: In that case, the configure-time check for is-an-ELF-system must be failing on cygwin: No: configure:43745: checking whether this is an ELF system configure:43761: result: no But there might be some missing holes where 'make check' tries to build stdbuf without regards to the result of the configure results. Then I suspect a portability problem, because if I simulate failure of that configure-time test, make does not try to build stdbuf or libstdbuf* on gnu/linux. If you can see (looking at src/Makefile) why it's trying to build stdbuf, we should be able to deduce where the problem lies. Portability of gl_ADD_PROG, perhaps... I'm now running 'make -k check', and will try to spot any obvious problems. So far, I'm seeing a suspicious failure: tests/misc/factor.log: factor (GNU coreutils) 7.4.121-ea5b7 ... z... factor: test z: stderr mismatch, comparing z.E (actual) and z.1 (expected) *** z.E Fri Aug 14 10:36:48 2009 --- z.1 Fri Aug 14 10:36:48 2009 *** *** 1,2 ! factor: unknown option -- 1 Try `factor --help' for more information. --- 1,2 ! factor: invalid option -- 1 Try `factor --help' for more information. cont... Thanks for investigating!
Re: no feedback on snapshot? coreutils-7.5 coming soon
Eric Blake ebb9 at byu.net writes: The test is still running. misc/stdbuf hangs (cygwin still has some fifo issues that might be at play, but more importantly, dd appears to be stuck trying to write to a fifo that never gets filled because stdbuf isn't working). I had to kill two different dd processes to get the testsuite to resume. Another suspicious failure: tests/misc/test-pwd.log /home/eblake/coreutils/src/pwd.exe: error while loading shared libraries: ?: cannot open shared object file: No such file or directory pwd-long: does not contain old CWD It looks like the test removes PATH from the environment; but this has the unfortunate side effect of crippling cygwin, which requires both /bin and Windows directories to be in PATH at all times for .dlls to be found and usable in running an executable, even when you don't otherwise use the PATH for the programs that you are invoking. In other words, you should be doing something similar to 'command −p getconf PATH' to determine the bare minimum PATH rather than nuking it altogether; unfortunately, command -p getconf PATH is probably not portable. -- Eric Blake
Re: no feedback on snapshot? coreutils-7.5 coming soon
Eric Blake wrote: Eric Blake ebb9 at byu.net writes: The test is still running. misc/stdbuf hangs (cygwin still has some fifo issues that might be at play, but That should not run if stdbuf is not built. Here's a patch: From 6c077c1633e31c36d17253ee7c946b47791dfd8e Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Wed, 12 Aug 2009 23:05:17 +0200 Subject: [PATCH] * src/remove.c (rm_fts): Handle --one-file-system. --- src/remove.c | 14 ++ 1 files changed, 14 insertions(+), 0 deletions(-) diff --git a/src/remove.c b/src/remove.c index 80be530..ea9e78c 100644 --- a/src/remove.c +++ b/src/remove.c @@ -519,6 +519,20 @@ rm_fts (FTS *fts, FTSENT *ent, struct rm_options const *x) case FTS_NSOK: /* e.g., dangling symlink */ case FTS_DEFAULT: /* none of the above */ ; + /* With --one-file-system, do not attempt to remove a mount point. +fts' FTS_XDEV ensures that we don't process any entries under +the mount point. */ + if (ent-fts_info == FTS_DP + x-one_file_system + FTS_ROOTLEVEL ent-fts_level + ent-fts_statp-st_ino != fts-fts_dev) + { + mark_parent_dirs (ent); + error (0, 0, _(skipping %s, since it's on a different device), +quote (ent-fts_path)); + return RM_ERROR; + } + bool is_dir = ent-fts_info == FTS_DP || ent-fts_info == FTS_DNR; s = prompt (fts, ent, is_dir, x, PA_REMOVE_DIR, NULL); if (s != RM_OK) -- 1.6.4.357.gfd68c more importantly, dd appears to be stuck trying to write to a fifo that never gets filled because stdbuf isn't working). I had to kill two different dd processes to get the testsuite to resume. Another suspicious failure: tests/misc/test-pwd.log /home/eblake/coreutils/src/pwd.exe: error while loading shared libraries: ?: cannot open shared object file: No such file or directory pwd-long: does not contain old CWD It looks like the test removes PATH from the environment; but this has the unfortunate side effect of crippling cygwin, which requires both /bin and Windows directories to be in PATH at all times for .dlls to be found and usable in running an executable, even when you don't otherwise use the PATH for the programs that you are invoking. In other words, you should be doing something similar to 'command −p getconf PATH' to determine the bare minimum PATH rather than nuking it altogether; unfortunately, command -p getconf PATH is probably not portable. If you'd like to write a cygwin-specific patch, to work around that, or even to skip the test, that'd be fine. No pressure, of course.
Re: no feedback on snapshot? coreutils-7.5 coming soon
Eric Blake ebb9 at byu.net writes: The test is still running. The final list of failures on cygwin 1.7 (I'll need to investigate further, and still want to rerun this under pristine conditions. I'm not even going to bother running this under cygwin 1.5.x): FAIL: misc/invalid-opt FAIL: rm/cycle FAIL: chmod/no-x FAIL: rm/isatty FAIL: rm/fail-eacces FAIL: rm/fail-eperm FAIL: rm/inaccessible FAIL: rm/rm1 FAIL: rm/rm2 FAIL: rm/rm3 FAIL: rm/unread2 FAIL: rm/unreadable FAIL: chgrp/no-x FAIL: misc/ls-misc FAIL: misc/factor FAIL: misc/paste FAIL: misc/printf FAIL: misc/pwd-long FAIL: misc/shred-remove FAIL: misc/sort-compress FAIL: misc/stdbuf FAIL: cp/existing-perm-race FAIL: cp/fail-perm FAIL: dd/direct FAIL: du/inacc-dest FAIL: du/inacc-dir FAIL: du/no-x FAIL: install/basic-1 FAIL: ls/stat-failed FAIL: mkdir/p-3 FAIL: mv/hard-3 FAIL: mv/i-2 FAIL: mv/i-3 FAIL: mv/perm-1 FAIL: mv/trailing-slash FAIL: readlink/can-e FAIL: readlink/can-f FAIL: readlink/can-m All 381 tests passed Hmm. There appears to be some sort of bug in how parallel-tests is calculating success, since this claimed that all tests passed even though there are failing and skipped logs. I don't know if the bug is in automake or in coreutils. -- Eric Blake
Re: no feedback on snapshot? coreutils-7.5 coming soon
Pádraig Brady wrote: Jim Meyering wrote: AFAIK, I am the only one who has built the latest snapshot: http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/17604 Though it's been only two days. Unless I hear of new bug reports or portability problems soon, expect coreutils-7.5 to be released in the next few days. checks pass on Fedora 11 and Ubuntu 7.10 2 failures on Fedora Core 5 due to copy::utimensat_if_possible() failing with: $ (cd tests make check TESTS=cp/abuse VERBOSE=yes) cp: preserving times for `c/1': No such file or directory $ (cd tests make check TESTS=mv/part-symlink VERBOSE=yes) mv: preserving times for `loc_reg': Too many levels of symbolic links These highlighted a couple of issue I think on systems without utimensat(). 1. The symlink _target_ gets its time updated 2. If 1 fails then the process returns a failure I've fixed both in the attached patch hopefully by only doing the explicit utimensat() on symlinks, and only giving a warning if errno==ENOTSUP. Note it might be cleaner to add a O_NOFOLLOW flag to utimens() in gnulib, but that would be more invasive. I also tweaked the tests not to fail on systems without utimensat(). cheers, Pádraig. From d337e3c7c081e1262ea13fbe239938c451b0a93d Mon Sep 17 00:00:00 2001 From: =?utf-8?q?P=C3=A1draig=20Brady?= p...@draigbrady.com Date: Thu, 13 Aug 2009 10:39:10 +0100 Subject: [PATCH] cp, mv: fix issues with preserving timestamps of copied symlinks * src/copy.c (copy_internal): On systems without utimensat() don't revert to utimens as that will update the timestamp of the symlink target. Also just warn on systems without utimensat(), don't fail. * tests/cp/abuse: To work around a failure on systems without utimensat(), use cp -dR rather than cp -a. * tests/mv/part-symlink: Skip the test on systems without utimensat(), as mv -b will warn there, messing up the test. --- src/copy.c| 20 +++- tests/cp/abuse|2 +- tests/mv/part-symlink |6 ++ 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/copy.c b/src/copy.c index bed90c4..068eef7 100644 --- a/src/copy.c +++ b/src/copy.c @@ -118,18 +118,17 @@ static bool owner_failure_ok (struct cp_options const *x); static char const *top_level_src_name; static char const *top_level_dst_name; -/* Wrap utimensat-with-AT_FDCWD and utimens, to keep these - cpp directives out of the main code. */ +/* Try to update the timestamp of a symlink. + We don't revert to utimens as that will follow the link. */ static inline int -utimensat_if_possible (char const *file, struct timespec const *timespec) +utimens_symlink (char const *file, struct timespec const *timespec) { - return #if HAVE_UTIMENSAT -utimensat (AT_FDCWD, file, timespec, AT_SYMLINK_NOFOLLOW) + return utimensat (AT_FDCWD, file, timespec, AT_SYMLINK_NOFOLLOW); #else -utimens (file, timespec) + errno = ENOTSUP; + return -1; #endif -; } /* Perform the O(1) btrfs clone operation, if possible. @@ -2117,10 +2116,13 @@ copy_internal (char const *src_name, char const *dst_name, timespec[0] = get_stat_atime (src_sb); timespec[1] = get_stat_mtime (src_sb); - if (utimensat_if_possible (dst_name, timespec) != 0) + if ((dest_is_symlink + ? utimens_symlink (dst_name, timespec) + : utimens (dst_name, timespec)) + != 0) { error (0, errno, _(preserving times for %s), quote (dst_name)); - if (x-require_preserve) + if (x-require_preserve ! (dest_is_symlink errno == ENOTSUP)) return false; } } diff --git a/tests/cp/abuse b/tests/cp/abuse index 285c531..e9086b8 100755 --- a/tests/cp/abuse +++ b/tests/cp/abuse @@ -37,7 +37,7 @@ for i in dangling-dest existing-dest; do test $i = existing-dest echo i t test $i = dangling-dest rm -f t - cp -a a/1 b/1 c 2 out fail=1 + cp -dR a/1 b/1 c 2 out fail=1 compare out exp || fail=1 diff --git a/tests/mv/part-symlink b/tests/mv/part-symlink index 78da70a..576bb40 100755 --- a/tests/mv/part-symlink +++ b/tests/mv/part-symlink @@ -27,6 +27,12 @@ fi cleanup_() { rm -rf $other_partition_tmpdir; } . $abs_srcdir/other-fs-tmpdir +# without utimensat() mv -b will give warnings about not +# being able to preserve the timestamps of symlinks. +# So just skip the test on older systems like this. +grep '^#define HAVE_UTIMENSAT' $CONFIG_HEADER /dev/null || + skip_test_ 'system will warn about preserving timestamps of symlinks' + pwd_tmp=`pwd` # Unset CDPATH. Otherwise, output from the `cd dir' command -- 1.6.2.5
Re: no feedback on snapshot? coreutils-7.5 coming soon
Pádraig Brady wrote: Pádraig Brady wrote: C de-Avillez wrote: Sorry for the delay, got busy. I just built make check, and got two errors. First one is here, I will re-run the second error by itself in a few. Running on Ubuntu 9.10 (kernel 2.6.31.5 with Ubuntu mods, libc6 2.10.1-0ubuntu6). FAIL: tail-2/pid + tail --pid=2147483647 -f /dev/null + fail=1 + timeout 1 tail -s.1 -f /dev/null --pid=2147483647 + test 1 = 124 So tail silently returns with 1 immediately. The only way I can see this happening is in tail_forever_inotify() at: if (follow_mode == Follow_descriptor !found_watchable) return; I'd better try and pay attention in this meeting ;) Meeting over :) Following from the above analysis, does the attached help? cheers, Pádraig. From af996b0e868752c633477a90802d2dcb382725b8 Mon Sep 17 00:00:00 2001 From: =?utf-8?q?P=C3=A1draig=20Brady?= p...@draigbrady.com Date: Wed, 12 Aug 2009 19:01:56 +0100 Subject: [PATCH] tail: fix tail -f failure when inotify used * src/tail.c (tail_inotify_forever): Use the correct bounds in the error check of the return from inotify_add_watch(). Reported by C de-Avillez. --- src/tail.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/tail.c b/src/tail.c index 3c8f425..7d84bec 100644 --- a/src/tail.c +++ b/src/tail.c @@ -1231,7 +1231,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, if (hash_insert (wd_table, (f[i])) == NULL) xalloc_die (); - if (follow_mode == Follow_name || f[i].wd) + if (follow_mode == Follow_name || 0 = f[i].wd) Ten lines above that, we ensure that 0 = f[i].wd is true, so this stmt: if (follow_mode == Follow_name || 0 = f[i].wd) is equivalent to this: if (follow_mode == Follow_name || true) aka, if (true) so perhaps that change should be larger: - if (follow_mode == Follow_name || f[i].wd) -found_watchable = true; + found_watchable = true; Also, the initialization (farther above) of f[i].wd to a valid file descriptor value (0) seems like a mistake: - f[i].wd = 0; + f[i].wd = -1; What do you think?
Re: no feedback on snapshot? coreutils-7.5 coming soon
Pádraig Brady wrote: While I'm at it here's a patch to improve that test. cheers, Pádraig. From c720e160a96b813a7c24c5ac8a9a9a37590f4190 Mon Sep 17 00:00:00 2001 From: =?utf-8?q?P=C3=A1draig=20Brady?= p...@draigbrady.com Date: Wed, 12 Aug 2009 19:46:27 +0100 Subject: [PATCH] tests: improve one of the tail --pid tests * tests/tail-2/pid: Speed up the test by specifying a timeout of 100ms rather than the default 1s. Also instead of failing in the unlikely case were the pid required to be missing pid is present, skip the test. Looks good. Please push.
Re: no feedback on snapshot? coreutils-7.5 coming soon
Jim Meyering wrote: Pádraig Brady wrote: Subject: [PATCH] tail: fix tail -f failure when inotify used * src/tail.c (tail_inotify_forever): Use the correct bounds in the error check of the return from inotify_add_watch(). Reported by C de-Avillez. --- src/tail.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/tail.c b/src/tail.c index 3c8f425..7d84bec 100644 --- a/src/tail.c +++ b/src/tail.c @@ -1231,7 +1231,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, if (hash_insert (wd_table, (f[i])) == NULL) xalloc_die (); - if (follow_mode == Follow_name || f[i].wd) + if (follow_mode == Follow_name || 0 = f[i].wd) Ten lines above that, we ensure that 0 = f[i].wd is true, so this stmt: if (follow_mode == Follow_name || 0 = f[i].wd) is equivalent to this: if (follow_mode == Follow_name || true) aka, if (true) so perhaps that change should be larger: - if (follow_mode == Follow_name || f[i].wd) -found_watchable = true; + found_watchable = true; Also, the initialization (farther above) of f[i].wd to a valid file descriptor value (0) seems like a mistake: - f[i].wd = 0; + f[i].wd = -1; What do you think? Agreed. I'll push the attached soon. cheers, Pádraig. From 0126b5c9276f9206c8b2c35cd7890267c962ae31 Mon Sep 17 00:00:00 2001 From: =?utf-8?q?P=C3=A1draig=20Brady?= p...@draigbrady.com Date: Wed, 12 Aug 2009 19:01:56 +0100 Subject: [PATCH] tail: fix tail -f failure when inotify used * src/tail.c (tail_forever_inotify): Remove the redundant and incorrect error check of the return from inotify_add_watch(). Also initialize the wd member of each File_spec to an invalid value. Reported by C de-Avillez. --- src/tail.c |5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/tail.c b/src/tail.c index 3c8f425..6f83706 100644 --- a/src/tail.c +++ b/src/tail.c @@ -1193,7 +1193,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, if (evlen fnlen) evlen = fnlen; - f[i].wd = 0; + f[i].wd = -1; if (follow_mode == Follow_name) { @@ -1231,8 +1231,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, if (hash_insert (wd_table, (f[i])) == NULL) xalloc_die (); - if (follow_mode == Follow_name || f[i].wd) -found_watchable = true; + found_watchable = true; } } -- 1.6.2.5
Re: no feedback on snapshot? coreutils-7.5 coming soon
Pádraig Brady wrote: Jim Meyering wrote: Pádraig Brady wrote: Subject: [PATCH] tail: fix tail -f failure when inotify used * src/tail.c (tail_inotify_forever): Use the correct bounds in the error check of the return from inotify_add_watch(). Reported by C de-Avillez. --- src/tail.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/tail.c b/src/tail.c index 3c8f425..7d84bec 100644 --- a/src/tail.c +++ b/src/tail.c @@ -1231,7 +1231,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, if (hash_insert (wd_table, (f[i])) == NULL) xalloc_die (); - if (follow_mode == Follow_name || f[i].wd) + if (follow_mode == Follow_name || 0 = f[i].wd) Ten lines above that, we ensure that 0 = f[i].wd is true, so this stmt: if (follow_mode == Follow_name || 0 = f[i].wd) is equivalent to this: if (follow_mode == Follow_name || true) aka, if (true) so perhaps that change should be larger: - if (follow_mode == Follow_name || f[i].wd) -found_watchable = true; + found_watchable = true; Also, the initialization (farther above) of f[i].wd to a valid file descriptor value (0) seems like a mistake: - f[i].wd = 0; + f[i].wd = -1; What do you think? Agreed. I'll push the attached soon. Perfect. Thanks.
Re: no feedback on snapshot? coreutils-7.5 coming soon
Pádraig Brady wrote: Pádraig Brady wrote: Jim Meyering wrote: AFAIK, I am the only one who has built the latest snapshot: http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/17604 Though it's been only two days. Unless I hear of new bug reports or portability problems soon, expect coreutils-7.5 to be released in the next few days. checks pass on Fedora 11 and Ubuntu 7.10 2 failures on Fedora Core 5 due to copy::utimensat_if_possible() failing with: $ (cd tests make check TESTS=cp/abuse VERBOSE=yes) cp: preserving times for `c/1': No such file or directory $ (cd tests make check TESTS=mv/part-symlink VERBOSE=yes) mv: preserving times for `loc_reg': Too many levels of symbolic links These highlighted a couple of issue I think on systems without utimensat(). 1. The symlink _target_ gets its time updated 2. If 1 fails then the process returns a failure I've fixed both in the attached patch hopefully by only doing the explicit utimensat() on symlinks, and only giving a warning if errno==ENOTSUP. Thanks for working on that. Previously, copying with cp -a, symlink times would not be preserved (silently, since cp made no attempt). It sounds like with this change, on systems without utimensat, cp would print a warning for each and every symlink. Note it might be cleaner to add a O_NOFOLLOW flag to utimens() in gnulib, but that would be more invasive. I think changing only copy.c is ok, for now.
Re: no feedback on snapshot? coreutils-7.5 coming soon
Jim Meyering wrote: Pádraig Brady wrote: These highlighted a couple of issues I think on systems without utimensat(). 1. The symlink _target_ gets its time updated 2. If 1 fails then the process returns a failure I've fixed both in the attached patch hopefully by only doing the explicit utimensat() on symlinks, and only giving a warning if errno==ENOTSUP. Thanks for working on that. Previously, copying with cp -a, symlink times would not be preserved (silently, since cp made no attempt). It sounds like with this change, on systems without utimensat, cp would print a warning for each and every symlink. I was wondering about printing the warning, and was wary about now silently not preserving symlink times. I.E. being silently inconsistent. I guess it's better to be quiet in this case? That also means I can reinstate the mv/part-symlink test on systems without utimensat(). Updated patch attached. cheers, Pádraig. From cfd4e430bd433ce3b38561300accad1fa56ee89f Mon Sep 17 00:00:00 2001 From: =?utf-8?q?P=C3=A1draig=20Brady?= p...@draigbrady.com Date: Thu, 13 Aug 2009 10:39:10 +0100 Subject: [PATCH] cp,mv: fix issues with preserving timestamps of copied symlinks * src/copy.c (copy_internal): On systems without utimensat() don't use to utimens as that will set the timestamp of the symlink target. * tests/cp/abuse: To work around possible attribute preservation failures breaking the test, use cp -dR rather than cp -a. --- src/copy.c | 19 +++ tests/cp/abuse |2 +- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/copy.c b/src/copy.c index bed90c4..73c1949 100644 --- a/src/copy.c +++ b/src/copy.c @@ -118,18 +118,18 @@ static bool owner_failure_ok (struct cp_options const *x); static char const *top_level_src_name; static char const *top_level_dst_name; -/* Wrap utimensat-with-AT_FDCWD and utimens, to keep these - cpp directives out of the main code. */ +/* Try to update the timestamp of a symlink. + We don't revert to utimens as that will follow the link. */ static inline int -utimensat_if_possible (char const *file, struct timespec const *timespec) +utimens_symlink (char const *file, struct timespec const *timespec) { - return #if HAVE_UTIMENSAT -utimensat (AT_FDCWD, file, timespec, AT_SYMLINK_NOFOLLOW) + return utimensat (AT_FDCWD, file, timespec, AT_SYMLINK_NOFOLLOW); #else -utimens (file, timespec) + /* Don't set errno=ENOTSUP here as we don't want + to output an error message for this case. */ + return 0; #endif -; } /* Perform the O(1) btrfs clone operation, if possible. @@ -2117,7 +2117,10 @@ copy_internal (char const *src_name, char const *dst_name, timespec[0] = get_stat_atime (src_sb); timespec[1] = get_stat_mtime (src_sb); - if (utimensat_if_possible (dst_name, timespec) != 0) + if ((dest_is_symlink + ? utimens_symlink (dst_name, timespec) + : utimens (dst_name, timespec)) + != 0) { error (0, errno, _(preserving times for %s), quote (dst_name)); if (x-require_preserve) diff --git a/tests/cp/abuse b/tests/cp/abuse index 285c531..e9086b8 100755 --- a/tests/cp/abuse +++ b/tests/cp/abuse @@ -37,7 +37,7 @@ for i in dangling-dest existing-dest; do test $i = existing-dest echo i t test $i = dangling-dest rm -f t - cp -a a/1 b/1 c 2 out fail=1 + cp -dR a/1 b/1 c 2 out fail=1 compare out exp || fail=1 -- 1.6.2.5
no feedback on snapshot? coreutils-7.5 coming soon
AFAIK, I am the only one who has built the latest snapshot: http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/17604 Though it's been only two days. Unless I hear of new bug reports or portability problems soon, expect coreutils-7.5 to be released in the next few days.
Re: no feedback on snapshot? coreutils-7.5 coming soon
Hi Jim, On Wed, Aug 12, 2009 at 12:42:59PM +0200, Jim Meyering wrote: AFAIK, I am the only one who has built the latest snapshot: http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/17604 Though it's been only two days. Unless I hear of new bug reports or portability problems soon, expect coreutils-7.5 to be released in the next few days. FWIW I built it and ran the basic tests (make check) without any errors on Debian GNU/Linux unstable. I don't have access to any other systems or hardware architectures for more interesting testing. Erik -- To do the Unix philosophy right, you have to be loyal to excellence. -- Eric S. Raymond
Re: no feedback on snapshot? coreutils-7.5 coming soon
Erik Auerswald wrote: Hi Jim, On Wed, Aug 12, 2009 at 12:42:59PM +0200, Jim Meyering wrote: AFAIK, I am the only one who has built the latest snapshot: http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/17604 Though it's been only two days. Unless I hear of new bug reports or portability problems soon, expect coreutils-7.5 to be released in the next few days. FWIW I built it and ran the basic tests (make check) without any errors on Debian GNU/Linux unstable. I don't have access to any other systems or hardware architectures for more interesting testing. Thanks. Good feedback helps.
Re: no feedback on snapshot? coreutils-7.5 coming soon
On Wed, 12 Aug 2009, Jim Meyering wrote: AFAIK, I am the only one who has built the latest snapshot: http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/17604 Though it's been only two days. Unless I hear of new bug reports or portability problems soon, expect coreutils-7.5 to be released in the next few days. ./configure make check works for me, non-root, Ubuntu 8.04. === All 341 tests passed (40 tests were not run) === and for completeness, the gnulib test results were === All 124 tests passed (11 tests were not run) === Cheers, Phil
Re: no feedback on snapshot? coreutils-7.5 coming soon
Jim Meyering wrote: AFAIK, I am the only one who has built the latest snapshot: http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/17604 Though it's been only two days. Unless I hear of new bug reports or portability problems soon, expect coreutils-7.5 to be released in the next few days. checks pass on Fedora 11 and Ubuntu 7.10 2 failures on Fedora Core 5 due to copy::utimensat_if_possible() failing with: $ (cd tests make check TESTS=cp/abuse VERBOSE=yes) cp: preserving times for `c/1': No such file or directory $ (cd tests make check TESTS=mv/part-symlink VERBOSE=yes) mv: preserving times for `loc_reg': Too many levels of symbolic links
Re: no feedback on snapshot? coreutils-7.5 coming soon
Pádraig Brady wrote: Jim Meyering wrote: AFAIK, I am the only one who has built the latest snapshot: http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/17604 Though it's been only two days. Unless I hear of new bug reports or portability problems soon, expect coreutils-7.5 to be released in the next few days. checks pass on Fedora 11 and Ubuntu 7.10 2 failures on Fedora Core 5 due to copy::utimensat_if_possible() failing with: $ (cd tests make check TESTS=cp/abuse VERBOSE=yes) cp: preserving times for `c/1': No such file or directory $ (cd tests make check TESTS=mv/part-symlink VERBOSE=yes) mv: preserving times for `loc_reg': Too many levels of symbolic links Thanks for the testing and report! Are these new failures? If they are, it might be worth fixing. Otherwise, FC5 is so old that I won't worry.
Re: no feedback on snapshot? coreutils-7.5 coming soon
Jim Meyering wrote: Thanks for the testing and report! Are these new failures? If they are, it might be worth fixing. Otherwise, FC5 is so old that I won't worry. Reverting the symlink timestamp patch make the tests pass $ wget -q -O- 'http://git.savannah.gnu.org/gitweb/'\ '?p=coreutils.git;a=patch;h=eae535e;hp=1762092' | patch -p1 -R $ make $ (cd tests make check TESTS=cp/abuse VERBOSE=yes) PASS: cp/abuse $ (cd tests make check TESTS=mv/part-symlink VERBOSE=yes) PASS: mv/part-symlink I've to run to a meeting now. sorry, Pádraig.
Re: no feedback on snapshot? coreutils-7.5 coming soon
On Wed, 2009-08-12 at 14:54 +0200, Jim Meyering wrote: Are these new failures? If they are, it might be worth fixing. Otherwise, FC5 is so old that I won't worry. Sorry for the delay, got busy. I just built make check, and got two errors. First one is here, I will re-run the second error by itself in a few. Running on Ubuntu 9.10 (kernel 2.6.31.5 with Ubuntu mods, libc6 2.10.1-0ubuntu6). Did not have time to dig in it. Sorry again. hg...@xango2:/usr/src/buildd/coreutils-snapshot/coreutils-7.4.115-c9c92/tests $ make check TESTS=tail-2/pid VERBOSE=yes make check-TESTS make[1]: Entering directory `/usr/src/buildd/coreutils-snapshot/coreutils-7.4.115-c9c92/tests' make[2]: Entering directory `/usr/src/buildd/coreutils-snapshot/coreutils-7.4.115-c9c92/tests' FAIL: tail-2/pid === GNU coreutils 7.4.115-c9c92: tests/test-suite.log === 1 of 1 test failed. .. contents:: :depth: 2 FAIL: tail-2/pid (exit: 1) == + tail --version tail (GNU coreutils) 7.4.115-c9c92 Copyright (C) 2009 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html. This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Written by Paul Rubin, David MacKenzie, Ian Lance Taylor, and Jim Meyering. + . ./test-lib.sh ++ unset function_test ++ eval 'function_test() { return 11; }; function_test' +++ function_test +++ return 11 ++ test 11 '!=' 11 +++ pwd ++ test_dir_=/usr/src/buildd/coreutils-snapshot/coreutils-7.4.115-c9c92/tests +++ this_test_ +++ echo ././tail-2/pid +++ sed 's,.*/,,' ++ this_test=pid +++ /usr/src/buildd/coreutils-snapshot/coreutils-7.4.115-c9c92/src/mktemp -d --tmp=/usr/src/buildd/coreutils-snapshot/coreutils-7.4.115-c9c92/tests cu-pid.XX ++ t_=/usr/src/buildd/coreutils-snapshot/coreutils-7.4.115-c9c92/tests/cu-pid.7WOIlLer56 ++ trap remove_tmp_ 0 ++ trap 'Exit $?' 1 2 13 15 ++ cd /usr/src/buildd/coreutils-snapshot/coreutils-7.4.115-c9c92/tests/cu-pid.7WOIlLer56 ++ diff --version ++ grep GNU + require_proc_pid_status_ + sleep 2 + local pid=6535 + sleep .5 + grep '^State:[ ]*[S]' /proc/6535/status + kill 6535 + touch here + fail=0 + tail -f here + bg_pid=6539 + tail -s0.1 -f here --pid=6539 + pid=6540 + sleep 0.5 ++ get_process_status_ 6540 ++ sed -n '/^State:[ ]*\([[:alpha:]]\).*/s//\1/p' /proc/6540/status sed: can't read /proc/6540/status: No such file or directory + state= + test -n '' + kill 6539 ./tail-2/pid: line 54: kill: (6539) - No such process + sleep 0.5 ++ get_process_status_ 6540 ++ sed -n '/^State:[ ]*\([[:alpha:]]\).*/s//\1/p' /proc/6540/status sed: can't read /proc/6540/status: No such file or directory + state= + test -n '' + getlimits_ ++ getlimits + eval CHAR_MAX=127 CHAR_OFLOW=128 CHAR_MIN=-128 CHAR_UFLOW=-129 SCHAR_MAX=127 SCHAR_OFLOW=128 SCHAR_MIN=-128 SCHAR_UFLOW=-129 UCHAR_MAX=255 UCHAR_OFLOW=256 SHRT_MAX=32767 SHRT_OFLOW=32768 SHRT_MIN=-32768 SHRT_UFLOW=-32769 INT_MAX=2147483647 INT_OFLOW=2147483648 INT_MIN=-2147483648 INT_UFLOW=-2147483649 UINT_MAX=4294967295 UINT_OFLOW=4294967296 LONG_MAX=9223372036854775807 LONG_OFLOW=9223372036854775808 LONG_MIN=-9223372036854775808 LONG_UFLOW=-9223372036854775809 ULONG_MAX=18446744073709551615 ULONG_OFLOW=18446744073709551616 SIZE_MAX=18446744073709551615 SIZE_OFLOW=18446744073709551616 SSIZE_MAX=9223372036854775807 SSIZE_OFLOW=9223372036854775808 SSIZE_MIN=-9223372036854775808 SSIZE_UFLOW=-9223372036854775809 TIME_T_MAX=9223372036854775807 TIME_T_OFLOW=9223372036854775808 TIME_T_MIN=-9223372036854775808 TIME_T_UFLOW=-9223372036854775809 UID_T_MAX=4294967295 UID_T_OFLOW=4294967296 GID_T_MAX=4294967295 GID_T_OFLOW=4294967296 PID_T_MAX=2147483647 PID_T_OFLOW=2147483648 PID_T_MIN=-2147483648 PID_T_UFLOW=-2147483649 OFF_T_MAX=9223372036854775807 OFF_T_OFLOW=9223372036854775808 OFF_T_MIN=-9223372036854775808 OFF_T_UFLOW=-9223372036854775809 INTMAX_MAX=9223372036854775807 INTMAX_OFLOW=9223372036854775808 INTMAX_MIN=-9223372036854775808 INTMAX_UFLOW=-9223372036854775809 UINTMAX_MAX=18446744073709551615 UINTMAX_OFLOW=18446744073709551616 ++ CHAR_MAX=127 ++ CHAR_OFLOW=128 ++ CHAR_MIN=-128 ++ CHAR_UFLOW=-129 ++ SCHAR_MAX=127 ++ SCHAR_OFLOW=128 ++ SCHAR_MIN=-128 ++ SCHAR_UFLOW=-129 ++ UCHAR_MAX=255 ++ UCHAR_OFLOW=256 ++ SHRT_MAX=32767 ++ SHRT_OFLOW=32768 ++ SHRT_MIN=-32768 ++ SHRT_UFLOW=-32769 ++ INT_MAX=2147483647 ++ INT_OFLOW=2147483648 ++ INT_MIN=-2147483648 ++ INT_UFLOW=-2147483649 ++ UINT_MAX=4294967295 ++ UINT_OFLOW=4294967296 ++ LONG_MAX=9223372036854775807 ++ LONG_OFLOW=9223372036854775808 ++ LONG_MIN=-9223372036854775808 ++ LONG_UFLOW=-9223372036854775809 ++ ULONG_MAX=18446744073709551615 ++ ULONG_OFLOW=18446744073709551616 ++ SIZE_MAX=18446744073709551615 ++ SIZE_OFLOW=18446744073709551616 ++ SSIZE_MAX=9223372036854775807 ++ SSIZE_OFLOW=9223372036854775808 ++
Re: no feedback on snapshot? coreutils-7.5 coming soon
C de-Avillez wrote: Sorry for the delay, got busy. I just built make check, and got two errors. First one is here, I will re-run the second error by itself in a few. Running on Ubuntu 9.10 (kernel 2.6.31.5 with Ubuntu mods, libc6 2.10.1-0ubuntu6). FAIL: tail-2/pid + tail --pid=2147483647 -f /dev/null + fail=1 + timeout 1 tail -s.1 -f /dev/null --pid=2147483647 + test 1 = 124 So tail silently returns with 1 immediately. The only way I can see this happening is in tail_forever_inotify() at: if (follow_mode == Follow_descriptor !found_watchable) return; We should probably issue an error() at that point in any case. I'd better try and pay attention in this meeting ;) cheers, Pádraig.
Re: no feedback on snapshot? coreutils-7.5 coming soon
On Wed, 2009-08-12 at 17:15 +0100, Pádraig Brady wrote: I'd better try and pay attention in this meeting ;) Heh. Please let me help you get distracted ;-) Second error, also on tail: FAIL: tail-2/pid (exit: 1) == + tail --version tail (GNU coreutils) 7.4.115-c9c92 Copyright (C) 2009 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html. This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Written by Paul Rubin, David MacKenzie, Ian Lance Taylor, and Jim Meyering. + . ./test-lib.sh ++ unset function_test ++ eval 'function_test() { return 11; }; function_test' +++ function_test +++ return 11 ++ test 11 '!=' 11 +++ pwd ++ test_dir_=/usr/src/buildd/coreutils-snapshot/coreutils-7.4.115-c9c92/tests +++ this_test_ +++ echo ././tail-2/pid +++ sed 's,.*/,,' ++ this_test=pid +++ /usr/src/buildd/coreutils-snapshot/coreutils-7.4.115-c9c92/src/mktemp -d --tmp=/usr/src/buildd/coreutils-snapshot/coreutils-7.4.115-c9c92/tests cu-pid.XX ++ t_=/usr/src/buildd/coreutils-snapshot/coreutils-7.4.115-c9c92/tests/cu-pid.By9pH2douA ++ trap remove_tmp_ 0 ++ trap 'Exit $?' 1 2 13 15 ++ cd /usr/src/buildd/coreutils-snapshot/coreutils-7.4.115-c9c92/tests/cu-pid.By9pH2douA ++ diff --version ++ grep GNU + require_proc_pid_status_ + sleep 2 + local pid=22028 + sleep .5 + grep '^State:[ ]*[S]' /proc/22028/status + kill 22028 + touch here + fail=0 + bg_pid=22032 + tail -f here + tail -s0.1 -f here --pid=22032 + pid=22033 + sleep 0.5 ++ get_process_status_ 22033 ++ sed -n '/^State:[ ]*\([[:alpha:]]\).*/s//\1/p' /proc/22033/status sed: can't read /proc/22033/status: No such file or directory + state= + test -n '' + kill 22032 ./tail-2/pid: line 54: kill: (22032) - No such process + sleep 0.5 ++ get_process_status_ 22033 ++ sed -n '/^State:[ ]*\([[:alpha:]]\).*/s//\1/p' /proc/22033/status sed: can't read /proc/22033/status: No such file or directory + state= + test -n '' + getlimits_ ++ getlimits + eval CHAR_MAX=127 CHAR_OFLOW=128 CHAR_MIN=-128 CHAR_UFLOW=-129 SCHAR_MAX=127 SCHAR_OFLOW=128 SCHAR_MIN=-128 SCHAR_UFLOW=-129 UCHAR_MAX=255 UCHAR_OFLOW=256 SHRT_MAX=32767 SHRT_OFLOW=32768 SHRT_MIN=-32768 SHRT_UFLOW=-32769 INT_MAX=2147483647 INT_OFLOW=2147483648 INT_MIN=-2147483648 INT_UFLOW=-2147483649 UINT_MAX=4294967295 UINT_OFLOW=4294967296 LONG_MAX=9223372036854775807 LONG_OFLOW=9223372036854775808 LONG_MIN=-9223372036854775808 LONG_UFLOW=-9223372036854775809 ULONG_MAX=18446744073709551615 ULONG_OFLOW=18446744073709551616 SIZE_MAX=18446744073709551615 SIZE_OFLOW=18446744073709551616 SSIZE_MAX=9223372036854775807 SSIZE_OFLOW=9223372036854775808 SSIZE_MIN=-9223372036854775808 SSIZE_UFLOW=-9223372036854775809 TIME_T_MAX=9223372036854775807 TIME_T_OFLOW=9223372036854775808 TIME_T_MIN=-9223372036854775808 TIME_T_UFLOW=-9223372036854775809 UID_T_MAX=4294967295 UID_T_OFLOW=4294967296 GID_T_MAX=4294967295 GID_T_OFLOW=4294967296 PID_T_MAX=2147483647 PID_T_OFLOW=2147483648 PID_T_MIN=-2147483648 PID_T_UFLOW=-2147483649 OFF_T_MAX=9223372036854775807 OFF_T_OFLOW=9223372036854775808 OFF_T_MIN=-9223372036854775808 OFF_T_UFLOW=-9223372036854775809 INTMAX_MAX=9223372036854775807 INTMAX_OFLOW=9223372036854775808 INTMAX_MIN=-9223372036854775808 INTMAX_UFLOW=-9223372036854775809 UINTMAX_MAX=18446744073709551615 UINTMAX_OFLOW=18446744073709551616 ++ CHAR_MAX=127 ++ CHAR_OFLOW=128 ++ CHAR_MIN=-128 ++ CHAR_UFLOW=-129 ++ SCHAR_MAX=127 ++ SCHAR_OFLOW=128 ++ SCHAR_MIN=-128 ++ SCHAR_UFLOW=-129 ++ UCHAR_MAX=255 ++ UCHAR_OFLOW=256 ++ SHRT_MAX=32767 ++ SHRT_OFLOW=32768 ++ SHRT_MIN=-32768 ++ SHRT_UFLOW=-32769 ++ INT_MAX=2147483647 ++ INT_OFLOW=2147483648 ++ INT_MIN=-2147483648 ++ INT_UFLOW=-2147483649 ++ UINT_MAX=4294967295 ++ UINT_OFLOW=4294967296 ++ LONG_MAX=9223372036854775807 ++ LONG_OFLOW=9223372036854775808 ++ LONG_MIN=-9223372036854775808 ++ LONG_UFLOW=-9223372036854775809 ++ ULONG_MAX=18446744073709551615 ++ ULONG_OFLOW=18446744073709551616 ++ SIZE_MAX=18446744073709551615 ++ SIZE_OFLOW=18446744073709551616 ++ SSIZE_MAX=9223372036854775807 ++ SSIZE_OFLOW=9223372036854775808 ++ SSIZE_MIN=-9223372036854775808 ++ SSIZE_UFLOW=-9223372036854775809 ++ TIME_T_MAX=9223372036854775807 ++ TIME_T_OFLOW=9223372036854775808 ++ TIME_T_MIN=-9223372036854775808 ++ TIME_T_UFLOW=-9223372036854775809 ++ UID_T_MAX=4294967295 ++ UID_T_OFLOW=4294967296 ++ GID_T_MAX=4294967295 ++ GID_T_OFLOW=4294967296 ++ PID_T_MAX=2147483647 ++ PID_T_OFLOW=2147483648 ++ PID_T_MIN=-2147483648 ++ PID_T_UFLOW=-2147483649 ++ OFF_T_MAX=9223372036854775807 ++ OFF_T_OFLOW=9223372036854775808 ++ OFF_T_MIN=-9223372036854775808 ++ OFF_T_UFLOW=-9223372036854775809 ++ INTMAX_MAX=9223372036854775807 ++ INTMAX_OFLOW=9223372036854775808 ++ INTMAX_MIN=-9223372036854775808 ++ INTMAX_UFLOW=-9223372036854775809 ++ UINTMAX_MAX=18446744073709551615 ++ UINTMAX_OFLOW=18446744073709551616 + test 2147483647 + tail --pid=2147483647 -f /dev/null + fail=1
Re: no feedback on snapshot? coreutils-7.5 coming soon
Yet another one, on check-root. I am starting to wonder if this may be a problem with my setup, since nobody has reported errors on tail. FAIL: tail-2/append-only (exit: 1) == + tail --version tail (GNU coreutils) 7.4.115-c9c92 Copyright (C) 2009 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html. This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Written by Paul Rubin, David MacKenzie, Ian Lance Taylor, and Jim Meyering. + . ./test-lib.sh ++ unset function_test ++ eval 'function_test() { return 11; }; function_test' +++ function_test +++ return 11 ++ test 11 '!=' 11 +++ pwd ++ test_dir_=/usr/src/buildd/coreutils-snapshot/coreutils-7.4.115-c9c92/tests +++ this_test_ +++ echo ././tail-2/append-only +++ sed 's,.*/,,' ++ this_test=append-only +++ /usr/src/buildd/coreutils-snapshot/coreutils-7.4.115-c9c92/src/mktemp -d --tmp=/usr/src/buildd/coreutils-snapshot/coreutils-7.4.115-c9c92/tests cu-append-only.XX ++ t_=/usr/src/buildd/coreutils-snapshot/coreutils-7.4.115-c9c92/tests/cu-append-only.dkK309eD5m ++ trap remove_tmp_ 0 ++ trap 'Exit $?' 1 2 13 15 ++ cd /usr/src/buildd/coreutils-snapshot/coreutils-7.4.115-c9c92/tests/cu-append-only.dkK309eD5m ++ diff --version ++ grep GNU + require_root_ + uid_is_privileged_ ++ id -u + my_uid=0 + case $my_uid in + NON_ROOT_USERNAME=nobody ++ id -g nobody + NON_ROOT_GROUP=65534 + chattr_a_works=1 + touch f + chattr +a f + echo x + test 1 = 0 + fail=0 + pid=22129 + sleep 1 + fail=1 + chattr -a f + Exit 1 + set +e + exit 1 + exit 1 + remove_tmp_ + __st=1 + cleanup_ + : + cd /usr/src/buildd/coreutils-snapshot/coreutils-7.4.115-c9c92/tests + chmod -R u +rwx /usr/src/buildd/coreutils-snapshot/coreutils-7.4.115-c9c92/tests/cu-append-only.dkK309eD5m + rm -rf /usr/src/buildd/coreutils-snapshot/coreutils-7.4.115-c9c92/tests/cu-append-only.dkK309eD5m + exit 1 signature.asc Description: This is a digitally signed message part
Re: no feedback on snapshot? coreutils-7.5 coming soon
Pádraig Brady wrote: C de-Avillez wrote: Sorry for the delay, got busy. I just built make check, and got two errors. First one is here, I will re-run the second error by itself in a few. Running on Ubuntu 9.10 (kernel 2.6.31.5 with Ubuntu mods, libc6 2.10.1-0ubuntu6). FAIL: tail-2/pid + tail --pid=2147483647 -f /dev/null + fail=1 + timeout 1 tail -s.1 -f /dev/null --pid=2147483647 + test 1 = 124 So tail silently returns with 1 immediately. The only way I can see this happening is in tail_forever_inotify() at: if (follow_mode == Follow_descriptor !found_watchable) return; I'd better try and pay attention in this meeting ;) Meeting over :) Following from the above analysis, does the attached help? cheers, Pádraig. From af996b0e868752c633477a90802d2dcb382725b8 Mon Sep 17 00:00:00 2001 From: =?utf-8?q?P=C3=A1draig=20Brady?= p...@draigbrady.com Date: Wed, 12 Aug 2009 19:01:56 +0100 Subject: [PATCH] tail: fix tail -f failure when inotify used * src/tail.c (tail_inotify_forever): Use the correct bounds in the error check of the return from inotify_add_watch(). Reported by C de-Avillez. --- src/tail.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/tail.c b/src/tail.c index 3c8f425..7d84bec 100644 --- a/src/tail.c +++ b/src/tail.c @@ -1231,7 +1231,7 @@ tail_forever_inotify (int wd, struct File_spec *f, size_t n_files, if (hash_insert (wd_table, (f[i])) == NULL) xalloc_die (); - if (follow_mode == Follow_name || f[i].wd) + if (follow_mode == Follow_name || 0 = f[i].wd) found_watchable = true; } } -- 1.6.2.5
Re: no feedback on snapshot? coreutils-7.5 coming soon
On Wed, 2009-08-12 at 20:22 +0100, Pádraig Brady wrote: While I'm at it here's a patch to improve that test. Thanks, Pádraig. Building testing right now both of them on the snapshot; then on git. signature.asc Description: This is a digitally signed message part
Re: no feedback on snapshot? coreutils-7.5 coming soon
On Wed, 2009-08-12 at 20:22 +0100, Pádraig Brady wrote: While I'm at it here's a patch to improve that test. cheers, Pádraig. OK. Both tests now succeed on the snapshot. I will apply them to git now (where I also had the same error). Cheers, ..C.. signature.asc Description: This is a digitally signed message part
Re: no feedback on snapshot? coreutils-7.5 coming soon
C de-Avillez wrote: Yet another one, on check-root. I am starting to wonder if this may be a problem with my setup, since nobody has reported errors on tail. FAIL: tail-2/append-only (exit: 1) == Thanks for the reports. How did you run those tests? When I do it like this (per README), they all pass. sudo env PATH=$PATH NON_ROOT_USERNAME=$USER make -k check-root
Re: no feedback on snapshot? coreutils-7.5 coming soon
On Wed, 2009-08-12 at 23:06 +0200, Jim Meyering wrote: C de-Avillez wrote: Yet another one, on check-root. I am starting to wonder if this may be a problem with my setup, since nobody has reported errors on tail. FAIL: tail-2/append-only (exit: 1) == Thanks for the reports. How did you run those tests? When I do it like this (per README), they all pass. sudo env PATH=$PATH NON_ROOT_USERNAME=$USER make -k check-root Yes, I did run them this way. I have just re-run them under git master (so that I would have the current status, without Pádraig's fixes). It failed the same. The following is the command line I used: hg...@xango2:/usr/src/buildd/coreutils $ sudo env PATH=$PATH NON_ROOT_USERNAME=$USER make check-root And here's the log: FAIL: tail-2/append-only (exit: 1) == + tail --version tail (GNU coreutils) 7.4.114-e3232 Copyright (C) 2009 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html. This is free software: you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. Written by Paul Rubin, David MacKenzie, Ian Lance Taylor, and Jim Meyering. + . ./test-lib.sh ++ unset function_test ++ eval 'function_test() { return 11; }; function_test' +++ function_test +++ return 11 ++ test 11 '!=' 11 +++ pwd ++ test_dir_=/usr/src/buildd/coreutils/tests +++ this_test_ +++ echo ././tail-2/append-only +++ sed 's,.*/,,' ++ this_test=append-only +++ /usr/src/buildd/coreutils/src/mktemp -d --tmp=/usr/src/buildd/coreutils/tests cu-append-only.XX ++ t_=/usr/src/buildd/coreutils/tests/cu-append-only.qJhExA8KWr ++ trap remove_tmp_ 0 ++ trap 'Exit $?' 1 2 13 15 ++ cd /usr/src/buildd/coreutils/tests/cu-append-only.qJhExA8KWr ++ diff --version ++ grep GNU + require_root_ + uid_is_privileged_ ++ id -u + my_uid=0 + case $my_uid in + NON_ROOT_USERNAME=hggdh ++ id -g hggdh + NON_ROOT_GROUP=1000 + chattr_a_works=1 + touch f + chattr +a f + echo x + test 1 = 0 + fail=0 + sleep 1 + pid=29813 + tail --pid=29813 -f f + fail=1 + chattr -a f + Exit 1 + set +e + exit 1 + exit 1 + remove_tmp_ + __st=1 + cleanup_ + : + cd /usr/src/buildd/coreutils/tests + chmod -R u +rwx /usr/src/buildd/coreutils/tests/cu-append-only.qJhExA8KWr + rm -rf /usr/src/buildd/coreutils/tests/cu-append-only.qJhExA8KWr + exit 1 signature.asc Description: This is a digitally signed message part