Re: [PATCH 2/2] gitlog-to-changelog: support 'tiny change' commits.
Gary V. Vaughan wrote: > Hi Jim, > > On 15 Nov 2011, at 18:14, Jim Meyering wrote: >> Gary V. Vaughan wrote: >>> I think 'Copyright-paper-required: No' is still the best compromise here for ... >> This is setting FSF policy, > > Well, the policy is already set very clearly... > > From http://www.gnu.org/prep/maintain/maintain.html#Legally-Significant That part is not in question. The syntax you are proposing would set policy for GNU projects that use gitlog-to-changelog. ... >> For example, I have a slight preference >> for a semantically positive tag like "Copyright-paperwork-exempt:". > > That seems fine to me too. > > Rather than stalling, what's the next step to keep things in motion? Propose a patch to maintain.texi.
Re: [PATCH 2/2] gitlog-to-changelog: support 'tiny change' commits.
Gary V. Vaughan wrote: > I think 'Copyright-paper-required: No' is still the best compromise here for > the reasons stated earlier in the thread. > > Okay to push? > > The FSF require that all non-trivial patches to its projects be > accompanied by appropriate paperwork, or that any patches that are > applied without that paperwork are marked as such in the > ChangeLog. > * gitlog-to-changelog: Convert `Copyright-paperwork-required: No' > lines from the git log message to standard `(tiny change)' > ChangeLog annotation. > * scripts/git-hooks/commit-msg: Diagnose redundant or malformed > Copyright-paperwork-required lines. This is setting FSF policy, and commit logs (at least on "master") are typically immutable, so I'd rather you defer pushing until the maintainer's guide endorses some precise syntax. I don't want to end up mandating one syntax and then find out later that some other syntax is preferred. For example, I have a slight preference for a semantically positive tag like "Copyright-paperwork-exempt:".
Re: exclude tests refactoring
Bruno Haible wrote:
> Hi Jim,
>
>> Thanks for testing and noticing that.
>> However, I would like to avoid using a temporary file when using GNU diff.
>> How about something like this instead?
>>
>> diff_=$(diff -u "$0" "$0" < /dev/null 2> /dev/null)
>> if test $? = 0; then
>> if test -z "$diff_"; then
>> compare () { diff -u "$@"; }
>> else
>> ...
>> fi
>> elif ...
>
> OK, I modified and applied the patch like this.
>
> Note that with use of $(...) outside double-quotes, namely
>
> diff_out_=$( ( diff -u "$0" "$0" < /dev/null ) 2>/dev/null)
>
> I got a syntax error on OSF/1 and Solaris:
>
> ./test-exclude1.sh: syntax error at line 224: `diff_out_=$' unexpected
> FAIL: test-exclude1.sh
> ./test-exclude2.sh: syntax error at line 224: `diff_out_=$' unexpected
> FAIL: test-exclude2.sh
> ./test-exclude3.sh: syntax error at line 224: `diff_out_=$' unexpected
> FAIL: test-exclude3.sh
> ./test-exclude4.sh: syntax error at line 224: `diff_out_=$' unexpected
> FAIL: test-exclude4.sh
> ./test-exclude5.sh: syntax error at line 224: `diff_out_=$' unexpected
> FAIL: test-exclude5.sh
> ./test-exclude6.sh: syntax error at line 224: `diff_out_=$' unexpected
> FAIL: test-exclude6.sh
> ./test-exclude7.sh: syntax error at line 224: `diff_out_=$' unexpected
> FAIL: test-exclude7.sh
> ./test-exclude8.sh: syntax error at line 224: `diff_out_=$' unexpected
> FAIL: test-exclude8.sh
>
> The reexec magic, which comes earlier in this file, would print debugging
> output like this:
>
> __current__ works just fine
> /bin/sh works just fine
> bash works just fine
> exec bash ./test-exclude1.sh --no-reexec
> PASS: test-exclude1.sh
>
> but when the
>
>diff_out_=$( ( diff -u "$0" "$0" < /dev/null ) 2>/dev/null)
I'm glad you discovered that.
That Solaris /bin/sh behavior is particularly aggravating.
The whole point of re-exec'ing was to avoid exposure to that.
This makes me think that the re-exec'ing script needs to
be separate from the rest.
> line is present, the execution does not even get that far. Apparently the
> parts of init.sh that are parsed by the initial shell (I tried /bin/sh
> and /bin/ksh) extend beyond the point of execution; possibly it includes
> the entire init.sh file.
>
> I could have written
>
>diff_out_="$( ( diff -u "$0" "$0" < /dev/null ) 2>/dev/null)"
>
> This does not produce a parse error. But it is nevertheless quite dangerous
> to let the initial shell parse a piece of code incorrectly, even if we know
> that we will jump out of this shell before the incorrectly parsed code can
> be executed.
>
>
> 2011-11-13 Bruno Haible
> Jim Meyering
>
> Silence successful tests that use 'compare' on AIX, HP-UX, Solaris.
> * tests/init.sh (compare): Remove "No differences encountered" or
> synonymous output from the 'diff' program.
Thanks!
That should do the job.
If I'd known you were doing that right now I would have delayed
today's grep snapshot by 5 or 10 minutes to include it ;-)
Oh well. I'll update from gnulib again before the release.
Re: exclude tests refactoring
Bruno Haible wrote:
> The Solaris 'diff' program understands option '-u', the AIX and HP-UX 'diff'
> programs don't. So here's a proposed patch. It removes the spurious output
> on all 3 platforms.
>
> 2011-11-12 Bruno Haible
>
> Silence successful tests that use 'compare' on AIX, HP-UX, Solaris.
> * tests/init.sh (compare): Remove "No differences encountered" or
> synonymous output from the 'diff' program.
Hi Bruno,
Thanks for testing and noticing that.
However, I would like to avoid using a temporary file when using GNU diff.
How about something like this instead?
diff_=$(diff -u "$0" "$0" < /dev/null 2> /dev/null)
if test $? = 0; then
if test -z "$diff_"; then
compare () { diff -u "$@"; }
else
...
fi
elif ...
> --- tests/init.sh.origSat Nov 12 21:10:28 2011
> +++ tests/init.sh Sat Nov 12 21:10:27 2011
> @@ -222,9 +222,34 @@
> cleanup_ () { :; }
>
> if ( diff -u "$0" "$0" < /dev/null ) > /dev/null 2>&1; then
> - compare () { diff -u "$@"; }
> + compare ()
> + {
> +if diff -u "$@" > diff.out; then
> + # No differences were found, but Solaris 'diff' produces output
> + # "No differences encountered". Hide this output.
> + rm -f diff.out
> + true
> +else
> + cat diff.out
> + rm -f diff.out
> + false
> +fi
> + }
> elif ( diff -c "$0" "$0" < /dev/null ) > /dev/null 2>&1; then
> - compare () { diff -c "$@"; }
> + compare ()
> + {
> +if diff -c "$@" > diff.out; then
> + # No differences were found, but AIX and HP-UX 'diff' produce output
> + # "No differences encountered" or "There are no differences between the
> + # files.". Hide this output.
> + rm -f diff.out
> + true
> +else
> + cat diff.out
> + rm -f diff.out
> + false
> +fi
> + }
> elif ( cmp --version < /dev/null 2>&1 | grep GNU ) > /dev/null 2>&1; then
>compare () { cmp -s "$@"; }
> else
Re: git-version-gen
Eric Blake wrote: > On 11/12/2011 11:53 AM, Simon Josefsson wrote: >> For inetutils (which uses git tags looking like 'inetutils-1_8') we >> noticed that git-version-gen hardcodes an expression when searching for >> particular tags: >> >> && v=`git describe --abbrev=4 --match='v*' HEAD 2>/dev/null \ >> >> I guess there are at least two questions: >> >> 1) Is it really required? > > What did you mean by "it"? The use of "--match=pattern"? Yes, since > that allows you the freedom to have more tags than just official > releases. (I frequently have local tags to intermediate points, but > only push official v1.0 style tags upstream, but since the command runs > locally, git describe tries to use my local tags unless I use --match). > >> >> 2) Any reason against making the expression 'v*' configurable? > > No problem by me - I'd welcome a patch that adds a --label-pattern > option that defaults to 'v*' but can be overridden by a cfg.mk variable. Same here. Patch welcome.
Re: [Platform-testers] new snapshot available: grep-2.9.69-f91c
Jim Meyering wrote:
> That fix was not enough, and even had a typo.
> With these further changes, I've now confirmed that those two
> tests are indeed skipped on Solaris 10, which also lacks those
> definitions.
>
...
>>From e500079a186434daeba99a1ea115690715fd56eb Mon Sep 17 00:00:00 2001
> From: Jim Meyering
> Date: Sat, 12 Nov 2011 16:48:09 +0100
> Subject: [PATCH 2/2] test-exclude2.sh, test-exclude5.sh: fail if test-exclude
> fails
>
> These shell scripts ignored failure of the binary test-exclude,
> so making the latter return 77 didn't cause them to be skipped.
> * tests/test-exclude5.sh: Exit with test-exclude's error status
> when that program fails. Revamp to use init.sh.
> * tests/test-exclude2.sh: Likewise.
> ---
> ChangeLog |7 +++
> tests/test-exclude2.sh | 28 ++--
> tests/test-exclude5.sh | 28 ++--
That worked around the unwarranted test failures.
This revamps the other very similar test-exclude?.sh scripts.
Strangely, my next-to-last sanity-check test succeeded:
./gnulib-tool --create-testdir --with-tests --test exclude
even though I hadn't listed tests/init.sh in the modules file.
I've gone ahead and added it anyway.
>From 293ba0481af81e296d922434983ebb23e79518e3 Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Sat, 12 Nov 2011 17:12:59 +0100
Subject: [PATCH] revamp the other test-exclude?.sh scripts to use init.sh,
too
* tests/test-exclude1.sh: Use init.sh.
* tests/test-exclude2.sh: Likewise.
* tests/test-exclude3.sh: Likewise.
* tests/test-exclude4.sh: Likewise.
* tests/test-exclude5.sh: Likewise.
* tests/test-exclude6.sh: Likewise.
* tests/test-exclude7.sh: Likewise.
* tests/test-exclude8.sh: Likewise.
* modules/exclude-tests (Files): List init.sh.
---
ChangeLog | 11 +++
modules/exclude-tests |1 +
tests/test-exclude1.sh | 25 +++--
tests/test-exclude2.sh |7 ++-
tests/test-exclude3.sh | 25 +++--
tests/test-exclude4.sh | 25 +++--
tests/test-exclude5.sh |7 ++-
tests/test-exclude6.sh | 25 +++--
tests/test-exclude7.sh | 27 +++
tests/test-exclude8.sh | 24 +++-
10 files changed, 106 insertions(+), 71 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index d265d75..2794c4d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,16 @@
2011-11-12 Jim Meyering
+ revamp the other test-exclude?.sh scripts to use init.sh, too
+ * tests/test-exclude1.sh: Use init.sh.
+ * tests/test-exclude2.sh: Likewise.
+ * tests/test-exclude3.sh: Likewise.
+ * tests/test-exclude4.sh: Likewise.
+ * tests/test-exclude5.sh: Likewise.
+ * tests/test-exclude6.sh: Likewise.
+ * tests/test-exclude7.sh: Likewise.
+ * tests/test-exclude8.sh: Likewise.
+ * modules/exclude-tests (Files): List init.sh.
+
test-exclude2.sh, test-exclude5.sh: fail if test-exclude fails
These shell scripts ignored failure of the binary test-exclude,
so making the latter return 77 didn't cause them to be skipped.
diff --git a/modules/exclude-tests b/modules/exclude-tests
index 3dd0225..072a4e6 100644
--- a/modules/exclude-tests
+++ b/modules/exclude-tests
@@ -1,4 +1,5 @@
Files:
+tests/init.sh
tests/test-exclude.c
tests/test-exclude1.sh
tests/test-exclude2.sh
diff --git a/tests/test-exclude1.sh b/tests/test-exclude1.sh
index 9c5f709..d85c4c7 100755
--- a/tests/test-exclude1.sh
+++ b/tests/test-exclude1.sh
@@ -16,19 +16,18 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-TMP=excltmp.$$
-LIST=flist.$$
-ERR=0
+. "${srcdir=.}/init.sh"; path_prepend_ .
+fail=0
# Test literal matches
-cat > $LIST < in < $TMP < expected < out || exit $?
-rm -f $TMP $LIST
-exit $ERR
+# Find out how to remove carriage returns from output. Solaris /usr/ucb/tr
+# does not understand '\r'.
+case $(echo r | tr -d '\r') in '') cr='\015';; *) cr='\r';; esac
+
+# normalize output
+LC_ALL=C tr -d "$cr" < out > k && mv k out
+
+compare expected out || fail=1
+
+Exit $fail
diff --git a/tests/test-exclude2.sh b/tests/test-exclude2.sh
index b38bb7f..33ee734 100755
--- a/tests/test-exclude2.sh
+++ b/tests/test-exclude2.sh
@@ -17,7 +17,6 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
. "${srcdir=.}/init.sh"; path_prepend_ .
-
fail=0
cat > in < out \
- || exit $?
+test-exclude -casefold in -- foo 'foo*' bar foobar baz bar/qux > out || exit $?
# Find out how to remove carriage returns from output. Solaris /usr/ucb/tr
# does not understand '\r'.
cas
Re: [Platform-testers] new snapshot available: grep-2.9.69-f91c
Dagobert Michelsen wrote:
> Hi Jim,
>
> Am 11.11.2011 um 14:42 schrieb Jim Meyering:
>> Thanks for investigating.
>> I've pushed the following fix to gnulib;
>> I'll update grep to use it before the release.
>
> Excellent! Please let me know if you have an updated tarball for me to try.
That fix was not enough, and even had a typo.
With these further changes, I've now confirmed that those two
tests are indeed skipped on Solaris 10, which also lacks those
definitions.
>From 24586b2eeb992e5359e009ed3d1860c5eb90 Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Sat, 12 Nov 2011 16:44:59 +0100
Subject: [PATCH 1/2] test-exclude: fix a typo
* tests/test-exclude.c (main): Test for "leading_dir", not "leading-dir".
---
ChangeLog|5 +
tests/test-exclude.c |2 +-
2 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 6b67bd8..4a29566 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2011-11-12 Jim Meyering
+
+ test-exclude: fix a typo
+ * tests/test-exclude.c (main): Test for "leading_dir", not
"leading-dir".
+
2011-11-11 Bruno Haible
obstack: Fix compilation error on MSVC 9.
diff --git a/tests/test-exclude.c b/tests/test-exclude.c
index 47392d9..88af36a 100644
--- a/tests/test-exclude.c
+++ b/tests/test-exclude.c
@@ -107,7 +107,7 @@ main (int argc, char **argv)
/* Skip this test if invoked with -leading-dir on a system that
lacks support for FNM_LEADING_DIR. */
- if (strcmp (s, "leading-dir") == 0 && FNM_LEADING_DIR == 0)
+ if (strcmp (s, "leading_dir") == 0 && FNM_LEADING_DIR == 0)
exit (77);
/* Likewise for -casefold and FNM_CASEFOLD. */
--
1.7.8.rc0.61.g8a042
>From e500079a186434daeba99a1ea115690715fd56eb Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Sat, 12 Nov 2011 16:48:09 +0100
Subject: [PATCH 2/2] test-exclude2.sh, test-exclude5.sh: fail if test-exclude
fails
These shell scripts ignored failure of the binary test-exclude,
so making the latter return 77 didn't cause them to be skipped.
* tests/test-exclude5.sh: Exit with test-exclude's error status
when that program fails. Revamp to use init.sh.
* tests/test-exclude2.sh: Likewise.
---
ChangeLog |7 +++
tests/test-exclude2.sh | 28 ++--
tests/test-exclude5.sh | 28 ++--
3 files changed, 43 insertions(+), 20 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 4a29566..d265d75 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
2011-11-12 Jim Meyering
+ test-exclude2.sh, test-exclude5.sh: fail if test-exclude fails
+ These shell scripts ignored failure of the binary test-exclude,
+ so making the latter return 77 didn't cause them to be skipped.
+ * tests/test-exclude5.sh: Exit with test-exclude's error status
+ when that program fails. Revamp to use init.sh.
+ * tests/test-exclude2.sh: Likewise.
+
test-exclude: fix a typo
* tests/test-exclude.c (main): Test for "leading_dir", not
"leading-dir".
diff --git a/tests/test-exclude2.sh b/tests/test-exclude2.sh
index 7011754..b38bb7f 100755
--- a/tests/test-exclude2.sh
+++ b/tests/test-exclude2.sh
@@ -16,11 +16,11 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-TMP=excltmp.$$
-LIST=flist.$$
-ERR=0
+. "${srcdir=.}/init.sh"; path_prepend_ .
-cat > $LIST < in < $TMP < expected < out \
+ || exit $?
+
+# Find out how to remove carriage returns from output. Solaris /usr/ucb/tr
+# does not understand '\r'.
+case $(echo r | tr -d '\r') in '') cr='\015';; *) cr='\r';; esac
+
+# normalize output
+LC_ALL=C tr -d "$cr" < out > k
+mv k out
+
+compare expected out || fail=1
-rm -f $TMP $LIST
-exit $ERR
+Exit $fail
diff --git a/tests/test-exclude5.sh b/tests/test-exclude5.sh
index 7f95ea7..3257963 100755
--- a/tests/test-exclude5.sh
+++ b/tests/test-exclude5.sh
@@ -16,28 +16,36 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-TMP=excltmp.$$
-LIST=flist.$$
-ERR=0
+. "${srcdir=.}/init.sh"; path_prepend_ .
+
+fail=0
# Test FNM_LEADING_DIR
-cat > $LIST < in < $TMP < expected < out \
+ || exit $?
+
+# Find out how to remove carriage returns from output. Solaris /usr/ucb/tr
+# does not understand '\r'.
+case $(echo r | tr -d '\r') in '') cr='\015';; *) cr='\r';; esac
+
+# normalize output
+LC_ALL=C tr -d "$cr" < out > k
+mv k out
+
+compare expected out || fail=1
-rm -f $TMP $LIST
-exit $ERR
+Exit $fail
--
1.7.8.rc0.61.g8a042
Re: gl_FUNC_FSTATAT doesn’t support cross-compilation
Ludovic Courtès wrote: >> 2011-11-10 Bruno Haible >> >> fstatat: Make cross-compilation guess succeed everywhere except on AIX. >> * m4/fstatat.m4 (gl_FUNC_FSTATAT): Require AC_CANONICAL_HOST. >> When cross-compiling, guess yes on all platforms except AIX. >> Reported by Ludovic Courtès . > > Thanks! > > Coreutils people: would it be possible for you to update the Gnulib > submodule? Sure. There was also a week's worth of gnulib changes, including more of Bruno's openat-related improvements. >From b64afe06058a88dd81020fa3d548a07c82c8e33e Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Sat, 12 Nov 2011 13:32:30 +0100 Subject: [PATCH] build: update gnulib for fstatat cross-compile improvement --- gnulib |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/gnulib b/gnulib index 667561d..17febe7 16 --- a/gnulib +++ b/gnulib @@ -1 +1 @@ -Subproject commit 667561d720dd1e22b51fe5d308a8f94d49d46b85 +Subproject commit 17febe7763c9ef38ed1ced5f08f022168e198929 -- 1.7.8.rc0.61.g8a042
Re: obstack fix
Bruno Haible wrote: > While testing a 'grep' prerelease on MSVC 9, I got this error: > > obstack.c > obstack.c(406) : error C2381: 'print_and_abort': redefinition; > __declspec(noreturn) differs from > obstack.c(90): see declaration of 'print_and_abort' > make[4]: *** [obstack.obj] Error 2 > > This fixes it. > > > 2011-11-11 Bruno Haible > > obstack: Fix compilation error on MSVC 9. > * lib/obstack.c (print_and_abort): Declare with _Noreturn specifier. Thanks!
Re: [Platform-testers] new snapshot available: grep-2.9.69-f91c
Dagobert Michelsen wrote: > Hi Jim, > > Am 10.11.2011 um 18:09 schrieb Jim Meyering: >> Dagobert Michelsen wrote: >>> I still get failing tests similar to the ones I reported in >>> https://savannah.gnu.org/bugs/?29380 >>> There doesn't seem to have been any fixing in gnulib for this since. >>> Platform is Solaris 9 Sparc with Sun Studio 12. >> ... >> >> Thanks for reporting that, again. >> If you feel like debugging it, that would be nice. >> I think I've seen the same thing on Solaris 10. >> >>>> FAIL: test-exclude2.sh (exit: 1) >>>> >>>> >>>> *** excltmp.21000 Thu Nov 10 13:38:53 2011 >>>> --- - Thu Nov 10 13:38:53 2011 >>>> *** >>>> *** 2,6 >>>> foo*: 1 >>>> bar: 1 >>>> foobar: 0 >>>> ! baz: 1 >>>> bar/qux: 0 >>>> --- 2,6 >>>> foo*: 1 >>>> bar: 1 >>>> foobar: 0 >>>> ! baz: 0 >>>> bar/qux: 0 > > This test checks functionality from lib/exclude.c and that file contains > >> /* Non-GNU systems lack these options, so we don't need to check them. */ >> #ifndef FNM_CASEFOLD >> # define FNM_CASEFOLD 0 >> #endif >> #ifndef FNM_EXTMATCH >> # define FNM_EXTMATCH 0 >> #endif >> #ifndef FNM_LEADING_DIR >> # define FNM_LEADING_DIR 0 >> #endif > > in Solaris 9 /usr/include/fnmatch.h does not contain FNM_CASEFOLD and > is therefore set to 0. > > That means test-exclude2.sh should not be run if FNM_CASEFOLD == 0. > >>>> FAIL: test-exclude5.sh (exit: 1) >>>> >>>> >>>> *** excltmp.21059 Thu Nov 10 13:38:54 2011 >>>> --- - Thu Nov 10 13:38:54 2011 >>>> *** >>>> *** 1,4 >>>> bar: 1 >>>> ! bar/qux: 1 >>>> barz: 0 >>>> foo/bar: 1 >>>> --- 1,4 >>>> bar: 1 >>>> ! bar/qux: 0 >>>> barz: 0 >>>> foo/bar: 1 > > This is similar and should not be run if FNM_LEADING_DIR == 0. > > Could you please forward this to the respective maintainers and ask if they > could adjust the testsuite accordingly? Thanks for investigating. I've pushed the following fix to gnulib; I'll update grep to use it before the release. >From 39a489fa27ab3873e0fc0f65844413f46fcb2117 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Fri, 11 Nov 2011 14:37:59 +0100 Subject: [PATCH] test-exclude: skip tests rather than failing on deficient systems * tests/test-exclude.c (main): Skip tests that use FNM_CASEFOLD and FNM_LEADING_DIR on systems that lack the definitions. This affects at least Solaris 9. Reported and diagnosed by Dagobert Michelsen in http://thread.gmane.org/gmane.comp.gnu.grep.bugs/3947/focus=3950 --- ChangeLog|8 tests/test-exclude.c |9 + 2 files changed, 17 insertions(+), 0 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0c4eff9..d35f6c6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2011-11-11 Jim Meyering + + test-exclude: skip tests rather than failing on deficient systems + * tests/test-exclude.c (main): Skip tests that use FNM_CASEFOLD + and FNM_LEADING_DIR on systems that lack the definitions. This affects + at least Solaris 9. Reported and diagnosed by Dagobert Michelsen in + http://thread.gmane.org/gmane.comp.gnu.grep.bugs/3947/focus=3950 + 2011-11-10 Bruno Haible ptsname_r test: Avoid gcc warning on glibc systems. diff --git a/tests/test-exclude.c b/tests/test-exclude.c index 9c7997d..47392d9 100644 --- a/tests/test-exclude.c +++ b/tests/test-exclude.c @@ -104,6 +104,15 @@ main (int argc, char **argv) exclude_options &= ~flag; else exclude_options |= flag; + + /* Skip this test if invoked with -leading-dir on a system that + lacks support for FNM_LEADING_DIR. */ + if (strcmp (s, "leading-dir") == 0 && FNM_LEADING_DIR == 0) +exit (77); + + /* Likewise for -casefold and FNM_CASEFOLD. */ + if (strcmp (s, "casefold") == 0 && FNM_CASEFOLD == 0) +exit (77); } else if (add_exclude_file (add_exclude, exclude, opt, exclude_options, '\n') != 0) -- 1.7.8.rc0.61.g8a042
Re: [PATCH 1/4] ptsname_r: new module
Bruno Haible wrote:
> Hi Jim, all,
>
>> >> I suggest using @tie{} between os (or program or ...) names and
>> >> versions. That way the line breaks come out ok in both the source and
>> >> the output.
>> >
>> > Indeed, the result looks better (at least in HTML). I tested
>> ...
>> I find that the mark-up renders the texi less readable, and obviously
>> less copy&pastable.
>> Maybe it's just that I don't (yet?) have some Emacs texi-viewing mode
>> enabled that hides those @tie{}s.
>>
>> However, I do recognize the value in better formatting.
>>
>> Tough call. I'm slightly in favor of adding the @tie directives.
>
> I'm also slightly in favour of good typography.
>
> But I'll do it by using no-break spaces in the .texi files. Like this:
Oh, n :-)
Please don't use non-breaking spaces.
Anything but that.
They tend to cause confusion (at least with me)
because they look like a space in many contexts, yet when you search
for the explicit space, you will not find those as matches.
With non-breaking spaces, anyone searching for "Mac OS..." or "AIX 5.x"
in our documentation will have to know about your choice. Most of
us will be surprised to find few or no matches initially.
Non-breaking spaces are generally seen as an undesirable artifact,
and that is why they are highlighted much like trailing blanks.
They represent a way to un-normalize the code.
At least until now. Please do not use them deliberately.
[PATCH] announce-gen: be more concise when there's only one URL+tarball
The generated announcement for coreutils-8.14 was adjusted manually
to make it more concise, now that there is only one (.xz) tarball.
This change to announce-gen makes it generated what I did manually,
and in addition (when you set in cfg.mk, url_dir_list =
http://ftp.gnu.org/gnu/$(PACKAGE)) it lists both the ftp.gnu.org
and ftpmirror.gnu.org addresses automatically, rather than displaying
the less-useful http://www.gnu.org/order/ftp.html link.
>From d5af3423d6dc7bc0356c9ff393db6e97305b07bd Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Wed, 9 Nov 2011 22:32:54 +0100
Subject: [PATCH] announce-gen: be more concise when there's only one
URL+tarball
* build-aux/announce-gen (get_tool_versions): When you distribute
only one type of tarball, combine the first two "Here are..."
sections and make the key-checking grammar independent of
how many tarballs there are.
---
ChangeLog |8 +++
build-aux/announce-gen | 52 ++-
2 files changed, 45 insertions(+), 15 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 2e1f6be..79d385c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2011-11-09 Jim Meyering
+
+ announce-gen: be more concise when there's only one URL+tarball
+ * build-aux/announce-gen (get_tool_versions): When you distribute
+ only one type of tarball, combine the first two "Here are..."
+ sections and make the key-checking grammar independent of
+ how many tarballs there are.
+
2011-11-08 Bruno Haible
More conditional dependencies.
diff --git a/build-aux/announce-gen b/build-aux/announce-gen
index 0eb6b5b..3866381 100755
--- a/build-aux/announce-gen
+++ b/build-aux/announce-gen
@@ -3,7 +3,7 @@ eval '(exit $?0)' && eval 'exec perl -wS "$0" ${1+"$@"}'
if 0;
# Generate a release announcement message.
-my $VERSION = '2011-05-17 20:25'; # UTC
+my $VERSION = '2011-11-09 21:30'; # UTC
# The definition above must lie within the first 8 lines in order
# for the Emacs time-stamp write hook (at end) to update it.
# If you change this file with Emacs, please let the write hook
@@ -453,28 +453,50 @@ FIXME: put comments here
EOF
- print_locations ("compressed sources", @url_dir_list, %size, @tarballs);
- -f $xd
-and print_locations ("xdelta diffs (useful? if so, "
- . "please tell bug-gnulib\@gnu.org)",
- @url_dir_list, %size, $xd);
- my @sig_files = map { "$_.sig" } @tarballs;
- print_locations ("GPG detached signatures[*]", @url_dir_list, %size,
- @sig_files);
+ if (@url_dir_list == 1 && @tarballs == 1)
+{
+ # When there's only one tarball and one URL, use a more concise form.
+ my $m = "$url_dir_list[0]/$tarballs[0]";
+ print "Here are the compressed sources and a GPG detached
signature[*]:\n"
+. " $m\n"
+. " $m.sig\n\n";
+}
+ else
+{
+ print_locations ("compressed sources", @url_dir_list, %size, @tarballs);
+ -f $xd
+and print_locations ("xdelta diffs (useful? if so, "
+ . "please tell bug-gnulib\@gnu.org)",
+ @url_dir_list, %size, $xd);
+ my @sig_files = map { "$_.sig" } @tarballs;
+ print_locations ("GPG detached signatures[*]", @url_dir_list, %size,
+ @sig_files);
+}
+
if ($url_dir_list[0] =~ "gnu\.org")
{
- print "To reduce load on the main server, use a mirror listed at:\n";
- print " http://www.gnu.org/order/ftp.html\n\n";;
+ print "Use a mirror for higher download bandwidth:\n";
+ if (@tarballs == 1 && $url_dir_list[0] =~ m!http://ftp\.gnu\.org/gnu/!)
+{
+ (my $m = "$url_dir_list[0]/$tarballs[0]")
+=~ s!http://ftp\.gnu\.org/gnu/!http://ftpmirror\.gnu\.org/!;
+ print " $m\n"
+ . " $m.sig\n\n";
+
+}
+ else
+{
+ print " http://www.gnu.org/order/ftp.html\n\n";;
+}
}
$print_checksums_p
and print_checksums (@sizable);
print <
Re: problem with latest texinfo.tex
Paul Eggert wrote: > On 11/09/11 11:47, Jim Meyering wrote: >> I'm sure Karl will >> do something about this pretty quickly: >> >> http://thread.gmane.org/gmane.comp.tex.texinfo.bugs/544 > > That URL is missing a digit; it should be: > > http://article.gmane.org/gmane.comp.tex.texinfo.bugs/5444 Yes, indeed. Thanks!
problem with latest texinfo.tex
FYI, texi2dvi was failing with no diagnostic for me, when using the latest texinfo.tex. I'm sure Karl will do something about this pretty quickly: http://thread.gmane.org/gmane.comp.tex.texinfo.bugs/544
Re: split off module fstatat from module openat
Bruno Haible wrote: >> * modules/fstatat: New file, extracted from modules/openat. > > Oops, there was a small mistake here: fstatat() does not call fstat(). > > > 2011-11-08 Bruno Haible > > fstatat: Remove unused dependency. > * modules/fstatat (Depends-on): Remove fstat. > > --- modules/fstatat.orig Wed Nov 9 01:53:58 2011 > +++ modules/fstatat Tue Nov 8 13:24:17 2011 > @@ -14,7 +14,6 @@ > extensions > fchdir > fcntl-h > -fstat > lstat > openat-die > openat-h Good one. Did you find that by using some new tool to check for unnecessary dependencies, or "just" by inspection?
Re: small mkfifoat fix
Bruno Haible wrote: > mkfifoat() and mknodat() are declared in , not . > Therefore this fix: > > > 2011-11-08 Bruno Haible > > mkfifoat: Fix module description. > * modules/mkfifoat (configure.ac): Invoke gl_SYS_STAT_MODULE_INDICATOR, > not gl_UNISTD_MODULE_INDICATOR. > > --- modules/mkfifoat.orig Wed Nov 9 01:53:58 2011 > +++ modules/mkfifoat Wed Nov 9 01:53:15 2011 > @@ -28,8 +28,8 @@ > if test $HAVE_MKNODAT = 0; then >AC_LIBOBJ([mknodat]) > fi > -gl_UNISTD_MODULE_INDICATOR([mkfifoat]) > -gl_UNISTD_MODULE_INDICATOR([mknodat]) > +gl_SYS_STAT_MODULE_INDICATOR([mkfifoat]) > +gl_SYS_STAT_MODULE_INDICATOR([mknodat]) These are too easy ;-) ACK. Thanks.
Re: small renameat fix
Bruno Haible wrote: > lib/renameat.c uses , therefore the module needs this dependency: > > > 2011-11-08 Bruno Haible > > renameat: Fix dependencies. > * modules/renameat (Depends-on): Add stdbool. > > --- modules/renameat.orig Wed Nov 9 01:53:58 2011 > +++ modules/renameat Tue Nov 8 13:47:37 2011 > @@ -21,6 +21,7 @@ > rename [test $HAVE_RENAMEAT = 0 || test $REPLACE_RENAMEAT = 1] > same-inode [test $HAVE_RENAMEAT = 0 || test $REPLACE_RENAMEAT = 1] > save-cwd [test $HAVE_RENAMEAT = 0 || test $REPLACE_RENAMEAT = 1] > +stdbool [test $HAVE_RENAMEAT = 0 || test $REPLACE_RENAMEAT = 1] Yes. Thanks.
Re: faccessat: simplify
Bruno Haible wrote: > The autoconf macro in faccessat.m4 does not need to AC_REQUIRE macros > that are pulled in through the module dependencies anyway. > > > 2011-11-08 Bruno Haible > > faccessat: Simplify autoconf macro. > * m4/faccessat.m4 (gl_FUNC_FACCESSAT): Don't require gl_FUNC_OPENAT, > gl_FUNC_EUIDACCESS. Also fine. Thank you.
Re: faccessat: autoconf macro idiom
Bruno Haible wrote: > Now that 'openat' is cleaned up, it is easy to make the 'faccessat' module > follow the codins style / idioms from May 2011. > > > 2011-11-08 Bruno Haible > > faccessat: Move AC_LIBOBJ invocation to module description. > * m4/faccessat.m4 (gl_PREREQ_FACCESSAT): New macro. > (gl_FUNC_FACESSAT): Don't test for access() here. Move AC_LIBOBJ > invocation from here... > * modules/faccessat (configure.ac): ... to here. Invoke > gl_PREREQ_FACCESSAT. Good. Thanks!
Re: openat: conditional dependencies
Bruno Haible wrote: > Jim Meyering wrote: >> >> 2011-11-05 Bruno Haible >> >> >> >> openat: Conditionalize dependencies. >> ... >> This change looks fine. > > OK, I've applied it. Now, as the last patch of the series, I'm adding > conditional dependencies for all *at modules. No changes of the lib/* > source code are needed for these. > > > 2011-11-08 Bruno Haible > > More conditional dependencies. > * modules/faccessat (Depends-on): Add conditions. > * modules/fchmodat (Depends-on): Likewise. > * modules/fchownat (Depends-on): Likewise. > * modules/fstatat (Depends-on): Likewise. > * modules/mkfifoat (Depends-on): Likewise. > * modules/readlinkat (Depends-on): Likewise. > * modules/symlinkat (Depends-on): Likewise. > * modules/unlinkat (Depends-on): Likewise. > * modules/utimensat (Depends-on): Likewise. > * modules/mkdirat (Depends-on): Add sys_stat. Add conditions. > * modules/linkat (Depends-on): Refine the conditions. > * modules/renameat (Depends-on): Likewise. That looks fine. Thanks for making that series of patches easy to review.
Re: [PATCH 1/4] ptsname_r: new module
Bruno Haible wrote:
> Hi Karl, all,
>
>> > +MacOS X 10.5, FreeBSD 6.0, NetBSD 5.0, OpenBSD 3.8, Minix 3.1.8, AIX
>>
>> Could you please break the line after a comma?
>>
>> I suggest using @tie{} between os (or program or ...) names and
>> versions. That way the line breaks come out ok in both the source and
>> the output.
>
> Indeed, the result looks better (at least in HTML). I tested
>
> @item
> This function is missing on some platforms:
> MacOS@tie{}X@tie{}10.5, FreeBSD@tie{}6.0, NetBSD@tie{}5.0, OpenBSD@tie{}3.8,
> Minix@tie{}3.1.8, AIX@tie{}5.1, HP-UX@tie{}11, IRIX@tie{}6.5,
> Solaris@tie{}11@tie{}2010-11, Cygwin, mingw, MSVC@tie{}9, BeOS.
>
> But it reduces the readability of the .texi file, leading to two problems
> with the way I work currently:
> - Often I point people to the newest .texi files in the repository,
> because we update the documentation on www.gnu.org rather seldomly.
> - Often I copy&paste between these .texi files and email.
>
> Hmm. What do the others think?
I find that the mark-up renders the texi less readable, and obviously
less copy&pastable.
Maybe it's just that I don't (yet?) have some Emacs texi-viewing mode
enabled that hides those @tie{}s.
However, I do recognize the value in better formatting.
Tough call. I'm slightly in favor of adding the @tie directives.
>> Requiring manually broken source lines defeats M-x fill-paragraph.
>
> Basically I was explaining to Eric that he should not use M-x fill-paragraph
> on these paragraphs, because the result that M-x fill-paragraph produces
> makes it more complicated to do mass modifications to 500 files at once.
>
>> (Also, I suggest MacOSX or MacOS@tie{}X instead of MacOS X, for
>> precisely the reason you cite.)
>
> MacOS@tie{}X is fine with me *if* we decide to use it systematically.
> I wouldn't want to have half of the spellings be "MacOS X" and the other
> half "MacOS@tie{}X".
We had similar problems with inconsistent OS naming in coreutils.
I enforced some measure of normalization with this syntax-check rule in cfg.mk:
(obviously, it too would have been defeated by a paragraph fill that
put OS name and version on different lines)
sc_sun_os_names:
@grep -nEi \
'solaris[^[:alnum:]]*2\.(7|8|9|[1-9][0-9])|sunos[^[:alnum:]][6-9]' \
$$($(VC_LIST_EXCEPT)) &&\
{ echo '$(ME): found misuse of Sun OS version numbers' 1>&2; \
exit 1; } || :
Re: GNUmakefile
Simon Josefsson wrote: > I upgraded gnulib in GnuTLS and noticed this problem in a clean > checkout: > > jas@latte:~/src/gnutls master$ make syntax-check > No version control files detected; skipping syntax check > jas@latte:~/src/gnutls master$ > > The reason was that _build-aux was not set when there was no Makefile. > The else-clause of GNUmakefile needs to set the variables the same way > the non-else clause does. I've pushed the patch below. Thanks!
Re: openat: conditional dependencies
Bruno Haible wrote: > Jim, ping? > >> Hi Jim, >> >> Now that the module structure of the *at functions is cleaned up - one >> function provided per module (except for mkfifoat and mknodat which can >> stay in a single module because they are nearly the same thing) -, let >> me try to improve what's built with --conditional-dependencies. >> >> Here's the first proposed patch, for the 'openat' replacement. >> >> The save-cwd code is only needed when $HAVE_OPENAT = 0 (i.e. when the system >> does not have openat()), not when $REPLACE_OPENAT = 1 (i.e. when gnulib >> overrides the system's openat() function). But the .c file currently includes >> save-cwd.h in both cases. Since we cannot include save-cwd.h without having >> invoked gl_SAVE_CWD, we need to depend on 'save-cwd' (and thus compile the >> code of save-cwd.c) _just_ for the sake of the position of the #include >> statement. >> >> Here's the proposed patch. While it is a good general guideline to do all >> #includes before all function definitions and all #ifs, I think it's worth >> deviating from that practice if it can avoid to compile unused compilation >> units. >> >> >> 2011-11-05 Bruno Haible >> >> openat: Conditionalize dependencies. >> * lib/openat.c: Reduce the scope of some #includes. >> * modules/openat (Depends-on): Add conditions. Hi Bruno, Thanks for the ping. I thought I'd already replied, but apparently not. This change looks fine.
[PATCH] maint.mk: extract GPG key ID without using a temporary file
FYI, here's a small improvement to the release process:
>From 103bb35a244bc08ec657bac05a0a1d429d651f45 Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Tue, 8 Nov 2011 10:44:54 +0100
Subject: [PATCH] maint.mk: extract GPG key ID without using a temporary file
* top/maint.mk (gpg_key_ID): Extract GPG key ID from signed tag, but
without using a temporary file. Based on a suggestion from Werner Koch
in http://thread.gmane.org/gmane.comp.encryption.gpg.devel/16496
---
ChangeLog|7 +++
top/maint.mk |6 +++---
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 1761f56..6efe313 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2011-11-07 Jim Meyering
+
+ maint.mk: extract GPG key ID without using a temporary file
+ * top/maint.mk (gpg_key_ID): Extract GPG key ID from signed tag, but
+ without using a temporary file. Based on a suggestion from Werner Koch
+ in http://thread.gmane.org/gmane.comp.encryption.gpg.devel/16496
+
2011-11-07 Eric Blake
grantpt: fix typo
diff --git a/top/maint.mk b/top/maint.mk
index 405c6d0..d2f5830 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -1203,9 +1203,9 @@ bootstrap-tools ?= autoconf,automake,gnulib
# If it's not already specified, derive the GPG key ID from
# the signed tag we've just applied to mark this release.
gpg_key_ID ?= \
- $$(git cat-file tag v$(VERSION) > .ann-sig \
- && gpgv .ann-sig - < /dev/null 2>&1 \
- | sed -n '/.*key ID \([0-9A-F]*\)/s//\1/p'; rm -f .ann-sig)
+ $$(git cat-file tag v$(VERSION) \
+ | gpgv --status-fd 1 --keyring /dev/null - - 2>/dev/null \
+ | sed -n '/^\[GNUPG:\] ERRSIG /{s///;s/ .*//p;q}')
translation_project_ ?= coordina...@translationproject.org
--
1.7.8.rc0.46.g5ae0f
[PATCH] maint.mk: also prohibit inclusion of dirent.h without use
FYI, Here's a probably-incomplete list of regexps for dirent.h. But completeness is not that important: the cost of incompleteness is merely the lack of warning about an unnecessary inclusion. >From bc1c14f003edbadde570d0322433b6b961205340 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Sat, 5 Nov 2011 17:34:09 +0100 Subject: [PATCH] maint.mk: also prohibit inclusion of dirent.h without use * top/maint.mk (sc_prohibit_dirent_without_use): New rule. --- ChangeLog|5 + top/maint.mk | 10 ++ 2 files changed, 15 insertions(+), 0 deletions(-) diff --git a/ChangeLog b/ChangeLog index bbb4c0b..616c5db 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2011-11-05 Jim Meyering + + maint.mk: also prohibit inclusion of dirent.h without use + * top/maint.mk (sc_prohibit_dirent_without_use): New rule. + 2011-11-05 Bruno Haible ldexpl tests: Avoid test failure on MSVC 9. diff --git a/top/maint.mk b/top/maint.mk index 72b7e06..9f4c032 100644 --- a/top/maint.mk +++ b/top/maint.mk @@ -625,6 +625,16 @@ sc_prohibit_stddef_without_use: re='\<($(_stddef_syms_re))\>' \ $(_sc_header_without_use) +_de1 = dirfd|(close|(fd)?open|read|rewind|seek|tell)dir(64)?(_r)? +_de2 = (versionsort|struct dirent|getdirentries|alphasort|scandir(at)?)(64)? +_de3 = MAXNAMLEN|DIR|ino_t|d_ino|d_fileno|d_namlen +_dirent_syms_re = $(_de1)|$(_de2)|$(_de3) +# Prohibit the inclusion of dirent.h without an actual use. +sc_prohibit_dirent_without_use: + h='dirent.h'\ + re='\<($(_dirent_syms_re))\>' \ + $(_sc_header_without_use) + # Prohibit the inclusion of verify.h without an actual use. sc_prohibit_verify_without_use: @h='verify.h' \ -- 1.7.8.rc0.35.gee6df
Re: split module openat
Bruno Haible wrote: > Hi Jim, > > The module 'openat' is still a mix of two different functionalities: > - It provides the POSIX function openat(), > - It compiles the openat_proc_name() function on which the implementations > of fdopendir(), faccessat(), fchmodat(), fchownat(), fstatat(), mkdirat(), > mkfifoat(), mknodat(), readlinkat(), areadlinkat(), symlinkat(), > unlinkat(), linkat(), renameat(), utimensat() rely. > > So that programs that need one of these functions but not openat() don't > include unnecessary code, it's advisable to split off the helper function > openat_proc_name() from the openat() module. > > While doing this, I also noticed that openat.h has a number of dependencies > (stdbool etc. - Paul removed 'dirent' from this list just yesterday), and > it makes the dependency managemant easier if we can put openat.h in a module > of its own. > > Here's the proposed patch. It also revisits the dependencies of all *at > modules. Tested through a megatestdir on a dozen of platforms. > > It has the side effect that in packages that use 'fdopendir' but not 'openat', > lib/openat-proc.c will now be compiled unconditionally, even on platforms > that don't need it. This is mitigated through --conditional-dependencies, > so IMO is acceptable. > > > 2011-11-05 Bruno Haible > > New modules 'at-internal', 'openat-h', split off from module 'openat'. ... > == modules/openat-h > === > Description: > Declarations of functions related to accessing files relative to a directory. > > Files: > lib/openat.h > > Depends-on: > fcntl-h > stdbool > sys_stat > unistd > > configure.ac: > > Makefile.am: > > Include: > "openat.h" > > License: > GPL > > Maintainer: > Jim Meyering, Eric Blake Hi Bruno I noticed a few failing hydra/nixos gzip builds: http://hydra.nixos.org/build/1504105 It is at least in part because you seem to have forgotten to add the openat-h module.
Re: openat includes
Bruno Haible wrote: > Paul Eggert wrote: >> openat_needs_fchdir -- which is a function that >> openat.c must implement. > > If that was the only use of 'bool' in openat.c, I would not have done the > change. But it's also used in line 193. > >> In general in my experience it's OK for foo.c to assume >> interfaces provided by foo.h. > > Sure. But the interface of openat.c is . > > openat.h is just an auxiliary include file. You removed the > include from it yesterday; you may want to remove > include from it tomorrow, when refactoring openat_needs_fchdir > in some way. I see your point, but don't you think that if someone were to remove openat_needs_fchdir and the inclusion of , they would try to verify that openat.c still compiles?
Re: split module openat
Bruno Haible wrote: > The module 'openat' is still a mix of two different functionalities: > - It provides the POSIX function openat(), > - It compiles the openat_proc_name() function on which the implementations > of fdopendir(), faccessat(), fchmodat(), fchownat(), fstatat(), mkdirat(), > mkfifoat(), mknodat(), readlinkat(), areadlinkat(), symlinkat(), > unlinkat(), linkat(), renameat(), utimensat() rely. > > So that programs that need one of these functions but not openat() don't > include unnecessary code, it's advisable to split off the helper function > openat_proc_name() from the openat() module. > > While doing this, I also noticed that openat.h has a number of dependencies > (stdbool etc. - Paul removed 'dirent' from this list just yesterday), and > it makes the dependency managemant easier if we can put openat.h in a module > of its own. > > Here's the proposed patch. It also revisits the dependencies of all *at > modules. Tested through a megatestdir on a dozen of platforms. > > It has the side effect that in packages that use 'fdopendir' but not 'openat', > lib/openat-proc.c will now be compiled unconditionally, even on platforms > that don't need it. This is mitigated through --conditional-dependencies, > so IMO is acceptable. > > 2011-11-05 Bruno Haible > > New modules 'at-internal', 'openat-h', split off from module 'openat'. > * m4/openat.m4 (gl_FUNC_OPENAT): Don't set GNULIB_OPENAT. Don't Avoiding inclusion of unused code is welcome, and the cost+risk is minimal (since you've done all the work and seem to have tested thoroughly). This looks great. Thanks yet again!
Re: split off module mkdirat from module openat
Bruno Haible wrote: > This patch is tested on a dozen of platforms as well, like the other ones. > > Here's a proposed fixup for some small omissions in the module descriptions. > - fchmodat, fchownat, linkat, renameat use lib/at-func.c or lib/at-func2.c > which requires openat_save_fail from module 'openat-die'. > - openat uses lib/at-func.c, which includes , thus requires module > 'dirent'. > - openat-tests: tests-openat.c includes test-open.h. Make sure this file > is present, even if someone uses --avoid=open-tests. > - renameat: No need to compile lib/at-func2.c if renameat() exists and > is being overridden by gnulib. > > 2011-11-04 Bruno Haible > > openat, fchmodat, fchownat, linkat, renameat: Fix dependencies. > * modules/fchmodat (Depends-on): Add openat-die. > * modules/fchownat (Depends-on): Likewise. > * modules/linkat (Depends-on): Likewise. > * modules/renameat (Depends-on): Likewise. > (configure.ac): Don't compile ac-func2.c if REPLACE_RENAMEAT is 1. > * modules/openat (Depends-on): Add dirent. > * modules/openat-tests (Files): Add tests/test-open.h. Those look fine. Glad you caught them. Thanks for all the testing!
[PATCH] at-func*.c: fix comments
FYI, >From 6a72f73905064a64091d16fd0787b96a012b4794 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Fri, 4 Nov 2011 13:33:54 +0100 Subject: [PATCH] at-func*.c: fix comments * lib/at-func2.c: Correct/improve first-line comment. * lib/at-func.c: Correct grammar in first-line comment. --- ChangeLog |6 ++ lib/at-func.c |2 +- lib/at-func2.c |2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 56cced3..2525647 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2011-11-04 Jim Meyering + + at-func*.c: fix comments + * lib/at-func2.c: Correct/improve first-line comment. + * lib/at-func.c: Correct grammar in first-line comment. + 2011-11-04 Bruno Haible closedir: Avoid warning on mingw. diff --git a/lib/at-func.c b/lib/at-func.c index 52868bc..5d1cd14 100644 --- a/lib/at-func.c +++ b/lib/at-func.c @@ -1,4 +1,4 @@ -/* Define an at-style functions like fstatat, unlinkat, fchownat, etc. +/* Define at-style functions like fstatat, unlinkat, fchownat, etc. Copyright (C) 2006, 2009-2011 Free Software Foundation, Inc. This program is free software: you can redistribute it and/or modify diff --git a/lib/at-func2.c b/lib/at-func2.c index da691fe..0983a77 100644 --- a/lib/at-func2.c +++ b/lib/at-func2.c @@ -1,4 +1,4 @@ -/* Define an at-style functions like linkat or renameat. +/* Define 2-FD at-style functions like linkat or renameat. Copyright (C) 2006, 2009-2011 Free Software Foundation, Inc. This program is free software: you can redistribute it and/or modify -- 1.7.8.rc0.35.gee6df
Re: split off module mkdirat from module openat
Bruno Haible wrote: > This proposed patch splits off module mkdirat from module openat. > > 2011-11-04 Bruno Haible > > New module 'mkdirat', split off from module 'openat'. > * m4/mkdirat.m4: New file. extracted from m4/openat.m4. > * m4/openat.m4 (gl_FUNC_OPENAT): Don't require gl_SYS_STAT_H_DEFAULTS. > Don't test for mkdirat. Don't set GNULIB_MKDIRAT, HAVE_MKDIRAT. > * modules/mkdirat: New file, extracted from modules/openat. > * modules/openat (Files): Remove lib/mkdirat.c. > (Depends-on): Remove mkdir. > (configure.ac): Remove AC_LIBOBJ of mkdirat. > (Include): Remove . > * modules/mkdirat-tests: New file, extracted from modules/openat-tests. > * modules/openat-tests (Files): Remove tests/test-mkdirat.c, > tests/test-mkdir.h. > (Depends-on): Remove ignore-value. > (Makefile.am): Remove rules for test-mkdirat. > * doc/posix-functions/mkdirat.texi: Mention module 'mkdirat' instead > of module 'openat'. > * NEWS: Mention the change. Looks fine. Push away. Thanks for all that work!
Re: split off module fstatat from module openat
Bruno Haible wrote: > The next split-off is 'fstatat'. Here's the proposed patch. Tested on several > platforms. > > 2011-11-03 Bruno Haible > > New module 'fstatat', split off from module 'openat'. > * lib/openat.h (statat, lstatat): Enable only if GNULIB_FSTATAT is > defined. > * m4/fstatat.m4: New file. extracted from m4/openat.m4. > * m4/openat.m4 (gl_FUNC_OPENAT): Don't set GNULIB_FSTATAT. Don't invoke > gl_FUNC_FSTATAT. > (gl_FUNC_FSTATAT): Moved to m4/fstatat.m4. > * modules/fstatat: New file, extracted from modules/openat. > * modules/openat (Files): Remove lib/fstatat.c. > (Depends-on): Remove lstat. > (configure.ac): Remove AC_LIBOBJ of fstatat. > * modules/fstatat-tests: New file, extracted from modules/openat-tests. > * modules/openat-tests (Files): Remove tests/test-fstatat.c, > tests/test-lstat.h, tests/test-stat.h. > (Depends-on): Remove getcwd-lgpl. > (Makefile.am): Remove rules for test-fstatat. > * doc/posix-functions/fstatat.texi: Mention module 'fstatat' instead > of module 'openat'. > * NEWS: Mention the change. > * modules/getcwd (Depends-on): Add fstatat. > * modules/linkat (Depends-on): Likewise. > * modules/mkfifoat-tests (Depends-on): Likewise. > * modules/utimensat (Depends-on): Add fstatat. Remove openat. Thanks, Bruno. I've reviewed this change-set in isolation, and it looks fine. I'll trust you and your testing for the rest. Please push.
Re: split off module unlinkat from module openat
Bruno Haible wrote: > The next split-off from module 'openat' is 'unlinkat'. Here is the proposed > patch. It also fixes incorrect conditions in the module dependencies, that > no one had noticed so far. Tested on multiple platforms. > > > 2011-11-02 Bruno Haible > > New module 'unlinkat', split off from module 'openat'. > * m4/unlinkat.m4: New file, extracted from m4/openat.m4. > * m4/openat.m4 (gl_FUNC_OPENAT): Don't set GNULIB_UNLINKAT, > REPLACE_UNLINKAT, HAVE_UNLINKAT. Don't test for unlinkat. > * modules/unlinkat: New file, extracted from modules/openat. Correct > the dependency conditions. > * modules/openat (Files): Remove lib/unlinkat.c. > (Depends-on): Remove rmdir, unlink. > (configure.ac): Remove AC_LIBOBJ of unlinkat. > * modules/unlinkat-tests: New file, extracted from modules/openat-tests. > * modules/openat-tests (Files): Remove tests/test-unlinkat.c, > tests/test-rmdir.h, tests/test-unlink.h. > (Depends-on): Remove unlinkdir. > (Makefile.am): Remove rules for test-unlinkat. > * doc/posix-functions/unlinkat.texi: Mention module 'unlinkat' instead > of module 'openat'. > * NEWS: Mention the change. > * modules/linkat-tests (Depends-on): Add unlinkat. > * modules/mkfifoat-tests (Depends-on): Likewise. > * modules/readlinkat-tests (Depends-on): Likewise. ... > === modules/unlinkat > === ... > Maintainer: > Jim Meyering, Eric Blake ... > --- modules/openat.orig Wed Nov 2 22:25:13 2011 > +++ modules/openatWed Nov 2 11:35:46 2011 Thanks yet again. Please push. With all of this work, you should add your name to those Maintainer: lists.
[PATCH] putenv: indent #definition of "environ" to placate cppi
FYI, >From af0285db3e8dee3f40ba573e845362335ac0cff2 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Wed, 2 Nov 2011 09:15:34 +0100 Subject: [PATCH] putenv: indent #definition of "environ" to placate cppi * lib/putenv.c (environ): Make indentation reflect cpp nesting. --- ChangeLog|3 +++ lib/putenv.c |2 +- 2 files changed, 4 insertions(+), 1 deletions(-) diff --git a/ChangeLog b/ChangeLog index 2218b2e..bdd74c6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,8 @@ 2011-11-02 Jim Meyering + putenv: indent #definition of "environ" to placate cppi + * lib/putenv.c (environ): Make indentation reflect cpp nesting. + gitlog-to-changelog: provide a ChangeLog-repair mechanism Git logs are often treated as immutable, because editing them changes the SHA1 checksums of all descendants. Thus, errors in diff --git a/lib/putenv.c b/lib/putenv.c index 3c33279..2da3376 100644 --- a/lib/putenv.c +++ b/lib/putenv.c @@ -36,7 +36,7 @@ #if _LIBC # if HAVE_GNU_LD -# define environ __environ +# define environ __environ # else extern char **environ; # endif -- 1.7.8.rc0.32.g87bf9
Re: [PATCH] gitlog-to-changelog: provide a ChangeLog-repair mechanism
Jim Meyering wrote:
> Jim Meyering wrote:
>
>> I've been thinking about this for years, ever since the unpleasantness
>> of that first git commit that I mistakenly attributed to myself.
>>
>> With this new --amend=FILE option, you'll be able to maintain
>> a list of pairs where SHA is a 40-byte SHA1
>> (alone on a line) referring to a commit in the current project, and
>> CODE refers to one or more consecutive lines of Perl code.
>> Pairs must be separated by one or more blank lines.
>> Using that file, invoke gitlog-to-changelog with this new option,
>> and it'll do the rest.
> ...
>>
>> Here's a proposed patch. Though note that I will move some description
>> and the example from the ChangeLog into the code:
>> ==
I've gone ahead and pushed that, so I can start using it in coreutils.
If there are substantial objections, I'll be happy to revise.
>From 57789615cb81dba6075ef76d3faddaa10dc61a07 Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Tue, 1 Nov 2011 18:04:21 +0100
Subject: [PATCH] gitlog-to-changelog: provide a ChangeLog-repair mechanism
Git logs are often treated as immutable, because editing them
changes the SHA1 checksums of all descendants. Thus, errors in
git logs tend to stay there forever. However, when we generate
a ChangeLog file -- typically for distribution -- from that git log,
we can actually make corrections in the generated file. The key
lies in recording in machine-readable/applicable form the desired
corrections. See --help for description and an example.
* build-aux/gitlog-to-changelog (parse_amend_file): New function.
(usage): Describe it; alphabetize option descriptions.
(main): Honor the new option, carefully.
---
ChangeLog | 14 +
build-aux/gitlog-to-changelog | 123 +++--
2 files changed, 132 insertions(+), 5 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 1fece4d..2218b2e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2011-11-02 Jim Meyering
+
+ gitlog-to-changelog: provide a ChangeLog-repair mechanism
+ Git logs are often treated as immutable, because editing them
+ changes the SHA1 checksums of all descendants. Thus, errors in
+ git logs tend to stay there forever. However, when we generate
+ a ChangeLog file -- typically for distribution -- from that git log,
+ we can actually make corrections in the generated file. The key
+ lies in recording in machine-readable/applicable form the desired
+ corrections. See --help for description and an example.
+ * build-aux/gitlog-to-changelog (parse_amend_file): New function.
+ (usage): Describe it; alphabetize option descriptions.
+ (main): Honor the new option, carefully.
+
2011-11-01 Jim Meyering
gitlog-to-changelog: avoid an infloop
diff --git a/build-aux/gitlog-to-changelog b/build-aux/gitlog-to-changelog
index 4612d38..56c7e4e 100755
--- a/build-aux/gitlog-to-changelog
+++ b/build-aux/gitlog-to-changelog
@@ -3,7 +3,7 @@ eval '(exit $?0)' && eval 'exec perl -wS "$0" ${1+"$@"}'
if 0;
# Convert git log output to ChangeLog format.
-my $VERSION = '2011-10-31 16:06'; # UTC
+my $VERSION = '2011-11-02 07:53'; # UTC
# The definition above must lie within the first 8 lines in order
# for the Emacs time-stamp write hook (at end) to update it.
# If you change this file with Emacs, please let the write hook
@@ -60,13 +60,15 @@ $ME, they may be preceded by '--'.
OPTIONS:
+ --amend=FILE FILE maps from an SHA1 to perl code (i.e., s/old/new/) that
+ makes a change to SHA1's commit log text or metadata.
+ --append-dot append a dot to the first line of each commit message if
+ there is no other punctuation or blank at the end.
--since=DATE convert only the logs since DATE;
the default is to convert all log entries.
--format=FMT set format string for commit subject and body;
see 'man git-log' for the list of format metacharacters;
the default is '%s%n%b%n'
- --append-dot append a dot to the first line of each commit message if
- there is no other punctuation or blank at the end.
--help display this help and exit
--versionoutput version information and exit
@@ -76,6 +78,23 @@ EXAMPLE:
$ME --since=2008-01-01 > ChangeLog
$ME -- -n 5 foo > last-5-commits-to-branch-foo
+In a FILE specified via --amend, comment lines (starting with "#") are ignored.
+FILE must consist of pairs where SHA is a 40-byte SHA1 (alone on
+a line) referring to a commit in the current project, and CODE refers to one
+or more conse
Re: split off module fchmodat from module openat
Bruno Haible wrote: > The next split-off from module 'openat' is 'fchmodat'. Here's the proposed > patch. Tested on a number of platforms. > > > 2011-11-01 Bruno Haible > > New module 'fchmodat', split off from module 'openat'. > * lib/openat.h (chmodat, lchmodat): Enable only if GNULIB_FCHMODAT is > defined. > * m4/fchmodat.m4: New file, extracted from m4/openat.m4. > * m4/openat.m4 (gl_FUNC_OPENAT): Don't set GNULIB_FCHMODAT. Don't test > for fchmodat, lchmod. Don't set HAVE_FCHMODAT. > * modules/fchmodat: New file, extracted from modules/openat. > * modules/openat (Files): Remove lib/fchmodat.c. > (configure.ac): Remove AC_LIBOBJ of fchmodat. > * modules/fchmodat-tests: New file, extracted from modules/openat-tests. > * modules/openat-tests (Files): Remove tests/test-fchmodat.c. > (Makefile.am): Remove rules for test-fchmodat. > * doc/posix-functions/fchmodat.texi: Mention module 'fchmodat' instead > of module 'openat'. > * NEWS: Mention the change. Hi Bruno, Thanks for what looks like yet another impeccable change. Please push it, and I'll test via coreutils.
Re: [PATCH] gitlog-to-changelog: provide a ChangeLog-repair mechanism
Jim Meyering wrote:
> I've been thinking about this for years, ever since the unpleasantness
> of that first git commit that I mistakenly attributed to myself.
>
> With this new --amend=FILE option, you'll be able to maintain
> a list of pairs where SHA is a 40-byte SHA1
> (alone on a line) referring to a commit in the current project, and
> CODE refers to one or more consecutive lines of Perl code.
> Pairs must be separated by one or more blank lines.
> Using that file, invoke gitlog-to-changelog with this new option,
> and it'll do the rest.
...
>
> Here's a proposed patch. Though note that I will move some description
> and the example from the ChangeLog into the code:
> ==
>
>>From c190d4ffca4643e40cc22a953ef55f2944bebdd8 Mon Sep 17 00:00:00 2001
> From: Jim Meyering
> Date: Tue, 1 Nov 2011 18:04:21 +0100
> Subject: [PATCH] gitlog-to-changelog: provide a ChangeLog-repair mechanism
...
> + # Barf if we fail to use an entry in the --amend=F specified file.
> + my $fail = 0;
> + if ($amend_code)
> +{
> + foreach my $sha (keys %$amend_code)
> +{
> + warn "$ME:$amend_file: unused entry: $sha\n";
> + $fail = 1;
> +}
> +}
That outer "if" was not needed:
diff --git a/build-aux/gitlog-to-changelog b/build-aux/gitlog-to-changelog
index 139f0af..33b5081 100755
--- a/build-aux/gitlog-to-changelog
+++ b/build-aux/gitlog-to-changelog
@@ -287,15 +287,12 @@ sub parse_amend_file($)
or die "$ME: error closing pipe from " . quoted_cmd (@cmd) . "\n";
# FIXME-someday: include $PROCESS_STATUS in the diagnostic
- # Barf if we fail to use an entry in the --amend=F specified file.
+ # Complain about any unused entry in the --amend=F specified file.
my $fail = 0;
- if ($amend_code)
+ foreach my $sha (keys %$amend_code)
{
- foreach my $sha (keys %$amend_code)
-{
- warn "$ME:$amend_file: unused entry: $sha\n";
- $fail = 1;
-}
+ warn "$ME:$amend_file: unused entry: $sha\n";
+ $fail = 1;
}
exit $fail;
Re: [PATCH] gitlog-to-changelog: provide a ChangeLog-repair mechanism
Jim Meyering wrote: > Eric Blake wrote: >> On 11/01/2011 11:15 AM, Jim Meyering wrote: >>> I've been thinking about this for years, ever since the unpleasantness >>> of that first git commit that I mistakenly attributed to myself. >>> >>> With this new --amend=FILE option, you'll be able to maintain >>> a list of pairs where SHA is a 40-byte SHA1 >>> (alone on a line) referring to a commit in the current project, and >>> CODE refers to one or more consecutive lines of Perl code. >>> Pairs must be separated by one or more blank lines. >>> Using that file, invoke gitlog-to-changelog with this new option, >>> and it'll do the rest. >> >> Is there some way to use 'git notes' as a way of generating these >> fixups as notes attached to the various commits they are meant to fix, >> in a known namespace, and to thus generate FILE by doing a git notes >> listing of all commits with an attached note? > > At least for now, I'd prefer not to rely on git notes, since we > haven't established a policy for ensuring how to propagate them > to the shared repository and how to handle merges. After sleeping on it, I've concluded that I'll never want to use git notes for this. This process modifies the ChangeLog file, which is a part of the release tarball. As such, any inputs to that process should be version-controlled, so that the tarball contents is uniquely determined solely by data under version control.
Re: [PATCH] gitlog-to-changelog: provide a ChangeLog-repair mechanism
Eric Blake wrote: > On 11/01/2011 11:15 AM, Jim Meyering wrote: >> I've been thinking about this for years, ever since the unpleasantness >> of that first git commit that I mistakenly attributed to myself. >> >> With this new --amend=FILE option, you'll be able to maintain >> a list of pairs where SHA is a 40-byte SHA1 >> (alone on a line) referring to a commit in the current project, and >> CODE refers to one or more consecutive lines of Perl code. >> Pairs must be separated by one or more blank lines. >> Using that file, invoke gitlog-to-changelog with this new option, >> and it'll do the rest. > > Is there some way to use 'git notes' as a way of generating these > fixups as notes attached to the various commits they are meant to fix, > in a known namespace, and to thus generate FILE by doing a git notes > listing of all commits with an attached note? At least for now, I'd prefer not to rely on git notes, since we haven't established a policy for ensuring how to propagate them to the shared repository and how to handle merges. > That would make using this a bit better than having to hand-maintain a IMHO, it would make the process more fragile, at least for now. > touchup file. But even without 'git notes', having this option to fix > history makes sense, and I'm glad to see it made available. ... > Can this also be used in conjunction with Gary's proposed patches to > add secondary authorship information, so that the changelog entry can > be touched up to list multiple authors? That is, these touchups > should be applied prior to the point of parsing for well-known markers > used to alter changelog metadata. These modifications happen *very* early in the parsing process, while the multi-line log data is still in a single buffer/variable, so yes.
Re: [PATCH] gitlog-to-changelog: provide a ChangeLog-repair mechanism
Jim Meyering wrote: > I've been thinking about this for years, ever since the unpleasantness > of that first git commit that I mistakenly attributed to myself. > > With this new --amend=FILE option, you'll be able to maintain > a list of pairs where SHA is a 40-byte SHA1 > (alone on a line) referring to a commit in the current project, and > CODE refers to one or more consecutive lines of Perl code. > Pairs must be separated by one or more blank lines. > Using that file, invoke gitlog-to-changelog with this new option, > and it'll do the rest. > > Here's the one I expect to use for coreutils: > > - > # This file is expected to be used via gitlog-to-changelog's --amend=FILE > # option. It specifies what changes to make to each given SHA1's commit > # log and metadata, using Perl-eval'able expressions. > > 3a169f4c5d9159283548178668d2fae6fced3030 > # fix title: > s/all tile types/all file types/ > > e181802521d4e19e367dbe8cfa877296bb5dafb2 > # fix the title! > s,seq:,factor:, > > 3ece0355d52e41a1b079c0c46477a32250278c11 > # correct the URL > s,<[^>]+>,<http://bugs.debian.org/412688>, Oops. I corrected that at the last minute and forgot to update. It should be: s,,<http://bugs.debian.org/412688>, Otherwise, it'll mistakenly change my on the first line to that URL.
[PATCH] gitlog-to-changelog: provide a ChangeLog-repair mechanism
I've been thinking about this for years, ever since the unpleasantness of that first git commit that I mistakenly attributed to myself. With this new --amend=FILE option, you'll be able to maintain a list of pairs where SHA is a 40-byte SHA1 (alone on a line) referring to a commit in the current project, and CODE refers to one or more consecutive lines of Perl code. Pairs must be separated by one or more blank lines. Using that file, invoke gitlog-to-changelog with this new option, and it'll do the rest. Here's the one I expect to use for coreutils: - # This file is expected to be used via gitlog-to-changelog's --amend=FILE # option. It specifies what changes to make to each given SHA1's commit # log and metadata, using Perl-eval'able expressions. 3a169f4c5d9159283548178668d2fae6fced3030 # fix title: s/all tile types/all file types/ e181802521d4e19e367dbe8cfa877296bb5dafb2 # fix the title! s,seq:,factor:, 3ece0355d52e41a1b079c0c46477a32250278c11 # correct the URL s,<[^>]+>,<http://bugs.debian.org/412688>, ed5c4e770a27862813c0182be8680abeb005d15b # Wrong bug ID: s,/363011,/350541, # in this: # Suggested by Josselin Mouette in <http://bugs.debian.org/363011> 1379ed974f1fa39b12e2ffab18b3f7a607082202 # Due to a bug in vc-dwim, I mis-attributed a patch by Paul to myself. # Change the Author: to be Paul. Note the escaped "@": s,Jim .*>,Paul Eggert , 209850fd7e1e89cf8937310878bd22d70e3588a5 s/isspace/isblank/ # in this: # * tests/misc/uniq: New file. Test for the above, but only # when isspace(0240). 760bc6f7e73014e934a744a9d46ea8dbf5ba25c8 s/Now, each/Now, the/; s!(elicits.*)\.!first $1, and the second works properly.! # change the log from this: # Without this, `truncate -s '> -1' F` would truncate F to length 0, # and `truncate -s " +1" F` would truncate F to 1 byte. Now, each # elicits a diagnostic. # to this: # Without this, `truncate -s '> -1' F` would truncate F to length 0, # and `truncate -s " +1" F` would truncate F to 1 byte. Now, the # first elicits a diagnostic, and the second works properly. - To compare before/after, I ran this: diff -u <(build-aux/gitlog-to-changelog) \ <(build-aux/gitlog-to-changelog --amend=F) Here are the induced diffs to the generated ChangeLog: --- /proc/self/fd/112011-11-01 18:11:36.348907196 +0100 +++ /proc/self/fd/122011-11-01 18:11:36.349907223 +0100 @@ -9162,7 +9162,7 @@ 2009-09-21 Pádraig Brady - ls: handle disabling of colors consistently for all tile types + ls: handle disabling of colors consistently for all file types * src/ls.c (print_color_indicator): Use consistent syntax for all file and directory subtypes, and fall back to the color of the base type if there is no enabled color for the subtype. @@ -12887,7 +12887,7 @@ 2008-12-01 Jim Meyering - seq: plug a leak + factor: plug a leak * src/factor.c (emit_ul_factor): Call mpz_clear. avoid warnings about initialization of automatic aggregates @@ -13077,7 +13077,7 @@ cp was doing the same, even without --link, for every directory in the source hierarchy, while it can do its job with entries merely for the command-line arguments. Prompted by a report from Patrick Shoenfeld. - Details <http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/15081>. + Details <http://bugs.debian.org/412688>. * src/copy.c (copy_internal): Refrain from remembering name,dev,inode for most files, when invoked via cp --link. Record an infloop-avoidance triple for each directory specified @@ -14531,8 +14531,8 @@ truncate: ignore whitespace in --size parameters Without this, `truncate -s '> -1' F` would truncate F to length 0, - and `truncate -s " +1" F` would truncate F to 1 byte. Now, each - elicits a diagnostic. + and `truncate -s " +1" F` would truncate F to 1 byte. Now, the + first elicits a diagnostic, and the second works properly. * src/truncate.c: Skip leading white space in the --size option argument and any white space after one of the relative modifiers, so that the presence of a +/- modifier can be detected reliably. @@ -15916,7 +15916,7 @@ avoid problems with sign-extended "char" operand to is* functions * src/cut.c (set_fields): Apply to_uchar to isblank operands. * src/uniq.c (find_field): Likewise. - * src/seq.c (scan_arg): Likewise, for isspace. + * src/seq.c (scan_arg): Likewise, for isblank. * tests/misc/uniq: New file. Test for the above, but only when isspace(0240). * tests/Makefile.am (TESTS): Add misc/uniq. @@ -1755
Re: [PATCH] gitlog-to-changelog: fix git-log invocation
Dmitry V. Levin wrote:
> On Mon, Oct 31, 2011 at 05:11:28PM +0100, Jim Meyering wrote:
>> Dmitry V. Levin wrote:
>> > git-log mishandles date strings before 1970-01-01 UTC, and there is
>> > no use to specify --since=1970-01-01 by default anyway.
>> > * build-aux/gitlog-to-changelog: By default, when no --since option
>> > was given, do not specify explicit --since option to git-log.
>> ...
>> > - my $since_date = '1970-01-01 UTC';
>> > + my $since_date = '';
>>
>> No need for the initializer.
>>
>> >my $format_string = '%s%n%b%n';
>> >my $append_dot = 0;
>> >GetOptions
>> > @@ -114,7 +114,12 @@ sub quoted_cmd(@)
>> > 'append-dot' => \$append_dot,
>> > ) or usage 1;
>> >
>> > - my @cmd = (qw (git log --log-size), "--since=$since_date",
>> > + if ($since_date)
>> > +{
>> > + unshift(@ARGV, "--since=$since_date");
>> > +}
>>
>> The above would fail to process any specified value that evaluates to 0.
>> Testing for definedness avoids that nit,
>> and I prefer the two-line construct to the 4-line one:
>>
>> defined $since_date
>> and unshift @ARGV, "--since=$since_date";
>
> Thanks for corrections. Now gitlog-to-changelog without --since will
> hopefully work 24 hours a day. :)
Depending on your definition of "work", you could even say "forever" ;-)
Running it on coreutils, I got this, ad infinitum:
Use of uninitialized value $line[0] in pattern match (m//) at
build-aux/gitlog-to-changelog line 163, line 34755.
>From bc2f20935ff445f1e55b9c7f7e90cd77ef4eb007 Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Tue, 1 Nov 2011 15:11:06 +0100
Subject: [PATCH] gitlog-to-changelog: avoid an infloop
* build-aux/gitlog-to-changelog: Don't infloop for a commit log
that ends up being empty.
---
ChangeLog |6 ++
build-aux/gitlog-to-changelog |7 +--
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 097de6c..1855e40 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2011-11-01 Jim Meyering
+
+ gitlog-to-changelog: avoid an infloop
+ * build-aux/gitlog-to-changelog: Don't infloop for a commit log
+ that ends up being empty.
+
2011-11-01 Bruno Haible
New module 'fchownat', split off from module 'openat'.
diff --git a/build-aux/gitlog-to-changelog b/build-aux/gitlog-to-changelog
index c776313..4612d38 100755
--- a/build-aux/gitlog-to-changelog
+++ b/build-aux/gitlog-to-changelog
@@ -160,8 +160,11 @@ sub quoted_cmd(@)
@line = grep !/^Signed-off-by: .*>$/, @line;
# Remove leading and trailing blank lines.
- while ($line[0] =~ /^\s*$/) { shift @line; }
- while ($line[$#line] =~ /^\s*$/) { pop @line; }
+ if (@line)
+{
+ while ($line[0] =~ /^\s*$/) { shift @line; }
+ while ($line[$#line] =~ /^\s*$/) { pop @line; }
+}
# If there were any lines
if (@line == 0)
--
1.7.7.1.476.g9890
Re: [PATCH 2/2] gitlog-to-changelog: support 'tiny change' commits.
Gary V. Vaughan wrote: >> The notation requirement made more sense back each ChangeLog entry >> was not tied to the associated patch. > > Before modern VCS you mean? Right. > That's before my time then... even CVS still ties > the ChangeLog to the patch does it not? No. With CVS, it was not uncommon to make the ChangeLog-changing delta a separate commit from that of the delta it described.
Re: [PATCH 2/2] gitlog-to-changelog: support 'tiny change' commits.
Gary V. Vaughan wrote:
> Here is another patch to solve a similar issue with ChangeLog `tiny change'
> annotations, by interpreting a 'Copyright-paperwork-required: No' line in
> the git log message to mean that the ChangeLog output requires the `tiny
> change' annotation.
>
> Because the annotation is added to the date_line, there is no suppression
> of consecutive header blocks when all else is equal but for a difference in
> the annotation.
>
> Okay to push?
>
> The FSF insist that all non-trivial patches to its projects are
> accompanied by appropriate paperwork, or that any patches that are
> applied without that paperwork are marked as such in the
> ChangeLog.
> * gitlog-to-changelog: Convert `Copyright-paperwork-required: No'
> lines from the git log message to standard `(tiny change)'
> ChangeLog annotation.
> ---
> ChangeLog |9 +
> build-aux/gitlog-to-changelog | 10 --
> 2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index f370be6..d59d9f9 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,14 @@
> 2011-11-01 Gary V. Vaughan
>
> + gitlog-to-changelog: support `tiny change' commits.
> + The FSF insist that all non-trivial patches to its projects are
Even if somewhere you've felt "insistence" on this issue,
let's just write "request":
s/insist/request/
s/are/be/ (both above and below)
> + accompanied by appropriate paperwork, or that any patches that are
> + applied without that paperwork are marked as such in the
> + ChangeLog.
> + * gitlog-to-changelog: Convert `Copyright-paperwork-required: No'
I find that rather long, and have a slight preference
to avoid an always-negative setting.
Did you consider "Tiny-change: Yes"
or even simply "/^Tiny[- ]change\b/",
so that "Tiny change by ..." will be recognized.
Now that I've thought about it a little more, ...
Actually, I question whether establishing such a convention.
and going to this trouble is worthwhile, since the size of the
change is already known, via the associated patch.
The notation requirement made more sense back each ChangeLog entry
was not tied to the associated patch.
Authorship matters a lot more than marking a change
that is obviously small by some measure as a "(tiny change)".
Let's see what Karl thinks.
> + lines from the git log message to standard `(tiny change)'
> + ChangeLog annotation.
...
> + # Format 'Copyright-paperwork-required: No' as a standard ChangeLog
> + # `(tiny change)' annotation.
> + my $tiny_change = grep /^Copyright-paperwork-required:\s+[Nn]o$/,
> @line;
> + $date_line =~ s/$/ (tiny change)/ if $tiny_change;
> +
># Format 'Co-authored-by: A U Thor ' lines in
># standard multi-author ChangeLog format.
>my @coauthors = grep /^Co-authored-by:.*>$/, @line;
> @@ -167,8 +172,9 @@ sub quoted_cmd(@)
>$prev_date_line = $date_line;
>@prev_coauthors = @coauthors;
>
> - # Omit "Co-authored-by..." and "Signed-off-by..." lines.
> + # Omit meta-data lines we've already interpreted.
>@line = grep !/^(Co-authored|Signed-off)-by: .*>$/, @line;
> + @line = grep !/^Copyright-paperwork-required: /, @line;
>
># Remove leading and trailing blank lines.
>while ($line[0] =~ /^\s*$/) { shift @line; }
> --
> 1.7.7.1
>
> Cheers,
Re: [PATCH 1/2] gitlog-to-changelog: support multi-author commits.
Gary V. Vaughan wrote:
> More generally useful gnulib-local goodness from my Libtool `next' branch:
>
> I'm sure this is far from idiomatic Perl, but I'd very much like for this
> patch or something similar to be pushed so that FSF projects have a means
> to correctly track multiple patch authors with a generated ChangeLog file.
I like the idea.
> The idea is to add bare 'Co-authored-by: ' lines to the git log message,
You mentioned consensus.
What other projects already use that syntax?
> which gitlog-to-changelog adds to the ChangeLog output in the usual format.
> Libtool at least, and I believe other FSF projects have the concept of a
> main author (the one on the date line) and additional authors (following)
> which gels well with keeping the coauthors in a separate array, and only
> suppressing consecutive headers blocks (date-line + coauthors) when the
> main author, date, and co-authors all match. That is, if the same group
> of people apply consecutive patches on the same day, but with a different
> "main author" each time, then a full header block is output each time.
>
> Okay to push?
>
> The FSF cares about keeping track of all authors of patches to its
> projects, but Git doesn't provide obvious support for multi-author
> changesets. Consensus seems to be forming around the use of extra
> Signed-off-by inspired lines in the log message formatted as
> `Co-authored-by: A U Thor ' for round-tripping
> multi-author commits between version control systems.
> * gitlog-to-changelog: Extract `Co-authored-by:' lines from the git
> log message and output in standard ChangeLog multi-author format.
> Reported by Peter Rosin
...
> @@ -124,6 +124,7 @@ sub quoted_cmd(@)
> . "(Is your Git too old? Version 1.5.1 or later is
> required.)\n");
>
>my $prev_date_line = '';
> + my @prev_coauthors = ();
Please drop the initializer.
>while (1)
> {
>defined (my $in = )
> @@ -146,18 +147,28 @@ sub quoted_cmd(@)
>. "(expected date/author/email):\n$author_line\n";
>
>my $date_line = sprintf "%s $2\n", strftime ("%F", localtime ($1));
> - # If this line would be the same as the previous date/name/email
> - # line, then arrange not to print it.
> - if ($date_line ne $prev_date_line)
> +
> + # Format 'Co-authored-by: A U Thor ' lines in
> + # standard multi-author ChangeLog format.
> + my @coauthors = grep /^Co-authored-by:.*>$/, @line;
Here you require only the trailing ">", yet below, the
transformation happens only if there is also a "<".
This is different from Signed-off-by: in that if we ignore a
line because it lacks an email address, then that probably
deserves a diagnostic.
Any annotation that we standardize like this should be
syntax-checked via a git log hook like the one I recently
added to coreutils (see scripts/git-hooks).
An alternative is to accept anything after the ":" and then, to use
"s/\s* + s/^Co-authored-by:\s*([^<]+)\s+ +for (@coauthors);
> +
> + # If this header would be the same as the previous date/name/email/
> + # coauthors header, then arrange not to print it.
> + if ($date_line ne $prev_date_line or "@coauthors" ne "@prev_coauthors")
s/or/||/
> {
>$prev_date_line eq ''
> or print "\n";
>print $date_line;
> + print join ("\n", @coauthors), "\n"
> +unless (@coauthors == 0);
Please write that so that the conditional part is indented:
@coauthors
and print join ("\n", @coauthors), "\n"
> }
>$prev_date_line = $date_line;
> + @prev_coauthors = @coauthors;
>
> - # Omit "Signed-off-by..." lines.
> - @line = grep !/^Signed-off-by: .*>$/, @line;
> + # Omit "Co-authored-by..." and "Signed-off-by..." lines.
> + @line = grep !/^(Co-authored|Signed-off)-by: .*>$/, @line;
>
># Remove leading and trailing blank lines.
>while ($line[0] =~ /^\s*$/) { shift @line; }
> --
> 1.7.7.1
>
> Cheers,
Re: split off module fchownat from module openat
Bruno Haible wrote: > Jim Meyering wrote: >> I'll test via coreutils once it's pushed. > > OK, I've pushed it, with this additional change squashed in: ... Thanks. That works fine, so I've updated coreutils: >From 65527e1c1a69a9ec8201435a6361964bcf31a480 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Tue, 1 Nov 2011 11:36:51 +0100 Subject: [PATCH] build: adapt to gnulib's recent openat/fchownat separation * bootstrap.conf (gnulib_modules): Add fchownat, now that gnulib has moved it into its own module. * gnulib: Update to latest. --- bootstrap.conf |1 + gnulib |2 +- 2 files changed, 2 insertions(+), 1 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index 820cb54..ce87b5d 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -70,6 +70,7 @@ gnulib_modules=" faccessat fadvise fchdir + fchownat fclose fcntl fcntl-safer diff --git a/gnulib b/gnulib index a765077..f2c5670 16 --- a/gnulib +++ b/gnulib @@ -1 +1 @@ -Subproject commit a765077b04a89bffa2a6095c7988fb9a70857d7f +Subproject commit f2c567087196c591677b4542e6f208f37892ec0f -- 1.7.7.1.476.g9890
Re: split off module fchownat from module openat
Bruno Haible wrote: > Hi Jim, Eric, > > Here is a proposed patch to split fchownat() off from the module 'openat'. > > The immediate motivation is that on MSVC, there is no and no > , therefore code that fiddles with owners and groups leads to > many more compilation problems than code that just deals with files > and directories. And the module 'getcwd' depends on 'openat'. > > The long-standing motivation is that a program that just needs to open > a file should not contain (or need to compile) code for changing > ownership, access modes, or for creating or removing files or directories. > > For this reason, I'll submit more patches like this, until the module > 'openat' really only provides openat(). > > This patch is tested on a number of platforms (Linux, MacOS X, FreeBSD, > OpenBSD, NetBSD, Cygwin, etc.) > > > 2011-10-31 Bruno Haible > > New module 'fchownat', split off from module 'openat'. > * lib/openat.h (chownat, lchownat): Enable only if GNULIB_FCHOWNAT is > defined. > * m4/fchownat.m4: New file, extracted from m4/openat.m4. > * m4/openat.m4 (gl_FUNC_OPENAT): Don't set GNULIB_FCHOWNAT. Don't > invoke gl_FUNC_FCHOWNAT. > (gl_FUNC_FCHOWNAT_DEREF_BUG, gl_FUNC_FCHOWNAT_EMPTY_FILENAME_BUG, > gl_FUNC_FCHOWNAT): Moved to m4/fchownat.m4. > * modules/fchownat: New file, extracted from modules/openat. > * modules/openat (Files): Remove lib/fchownat.c. > (Depends-on): Remove lchown. > (configure.ac): Remove AC_LIBOBJ of fchownat. > * modules/fchownat-tests: New file, extracted from modules/openat-tests. > * modules/openat-tests (Files): Remove tests/test-fchownat.c, > tests/test-chown.h, tests/test-lchown.h, tests/nap.h. > (Depends-on): Remove mgetgroups, usleep, stat-time. > (configure.ac): Remove test for getegid. > (Makefile.am): Remove rules for test-fchownat. > * doc/posix-functions/fchownat.texi: Mention module 'fchownat' instead > of module 'openat'. > * NEWS: Mention the change. I'm glad you found the time to work on this. That change looks fine. I'll test via coreutils once it's pushed. Thank you.
Re: [PATCH] gitlog-to-changelog: fix git-log invocation
Dmitry V. Levin wrote:
> git-log mishandles date strings before 1970-01-01 UTC, and there is
> no use to specify --since=1970-01-01 by default anyway.
> * build-aux/gitlog-to-changelog: By default, when no --since option
> was given, do not specify explicit --since option to git-log.
...
> - my $since_date = '1970-01-01 UTC';
> + my $since_date = '';
No need for the initializer.
>my $format_string = '%s%n%b%n';
>my $append_dot = 0;
>GetOptions
> @@ -114,7 +114,12 @@ sub quoted_cmd(@)
> 'append-dot' => \$append_dot,
> ) or usage 1;
>
> - my @cmd = (qw (git log --log-size), "--since=$since_date",
> + if ($since_date)
> +{
> + unshift(@ARGV, "--since=$since_date");
> +}
The above would fail to process any specified value that evaluates to 0.
Testing for definedness avoids that nit,
and I prefer the two-line construct to the 4-line one:
defined $since_date
and unshift @ARGV, "--since=$since_date";
> +
> + my @cmd = (qw (git log --log-size),
> '--pretty=format:%ct %an <%ae>%n%n'.$format_string, @ARGV);
>open PIPE, '-|', @cmd
> or die ("$ME: failed to run `". quoted_cmd (@cmd) ."': $!\n"
Thanks. I've pushed this:
>From 3aee0dcbd2f2ee665609b1d338940c021db7d484 Mon Sep 17 00:00:00 2001
From: "Dmitry V. Levin"
Date: Mon, 31 Oct 2011 19:56:52 +0400
Subject: [PATCH] gitlog-to-changelog: fix git-log invocation
git-log mishandles date strings before 1970-01-01 UTC, and there is
no use to specify --since=1970-01-01 by default anyway.
* build-aux/gitlog-to-changelog: By default, when no --since option
was given, do not specify explicit --since option to git-log.
---
ChangeLog |8
build-aux/gitlog-to-changelog |9 ++---
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index a692aa9..f68a9b0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2011-10-31 Dmitry V. Levin
+
+ gitlog-to-changelog: fix git-log invocation.
+ git-log mishandles date strings before 1970-01-01 UTC, and there is
+ no use to specify --since=1970-01-01 by default anyway.
+ * build-aux/gitlog-to-changelog: By default, when no --since option
+ was given, do not specify explicit --since option to git-log.
+
2011-10-30 Dmitry V. Levin
gitlog-to-changelog: new option --append-dot.
diff --git a/build-aux/gitlog-to-changelog b/build-aux/gitlog-to-changelog
index c3a5ef3..c776313 100755
--- a/build-aux/gitlog-to-changelog
+++ b/build-aux/gitlog-to-changelog
@@ -3,7 +3,7 @@ eval '(exit $?0)' && eval 'exec perl -wS "$0" ${1+"$@"}'
if 0;
# Convert git log output to ChangeLog format.
-my $VERSION = '2011-10-31 07:45'; # UTC
+my $VERSION = '2011-10-31 16:06'; # UTC
# The definition above must lie within the first 8 lines in order
# for the Emacs time-stamp write hook (at end) to update it.
# If you change this file with Emacs, please let the write hook
@@ -102,7 +102,7 @@ sub quoted_cmd(@)
}
{
- my $since_date = '1970-01-01 UTC';
+ my $since_date;
my $format_string = '%s%n%b%n';
my $append_dot = 0;
GetOptions
@@ -114,7 +114,10 @@ sub quoted_cmd(@)
'append-dot' => \$append_dot,
) or usage 1;
- my @cmd = (qw (git log --log-size), "--since=$since_date",
+ defined $since_date
+and unshift @ARGV, "--since=$since_date";
+
+ my @cmd = (qw (git log --log-size),
'--pretty=format:%ct %an <%ae>%n%n'.$format_string, @ARGV);
open PIPE, '-|', @cmd
or die ("$ME: failed to run `". quoted_cmd (@cmd) ."': $!\n"
--
1.7.7.1.476.g9890
Re: [PATCH 0/2] gitlog-to-changelog output adjustments
Dmitry V. Levin wrote:
...
> Subject: [PATCH] gitlog-to-changelog: new option --append-dot
>
> * build-aux/gitlog-to-changelog: New option --append-dot, makes the
> first non-blank line of each commit message terminated with a dot.
> ---
> ChangeLog |6 ++
> build-aux/gitlog-to-changelog | 15 +++
> 2 files changed, 21 insertions(+), 0 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index 30cc2af..76d1c64 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,9 @@
> +2011-10-30 Dmitry V. Levin
> +
> + gitlog-to-changelog: new option --append-dot.
> + * build-aux/gitlog-to-changelog: New option --append-dot, makes the
> + first non-blank line of each commit message terminated with a dot.
> +
> 2011-10-29 Dmitry V. Levin
>
> gitlog-to-changelog: treat a message with only blank lines as empty.
> diff --git a/build-aux/gitlog-to-changelog b/build-aux/gitlog-to-changelog
> index a5fd80d..c6e91c3 100755
> --- a/build-aux/gitlog-to-changelog
> +++ b/build-aux/gitlog-to-changelog
> @@ -65,6 +65,8 @@ OPTIONS:
> --format=FMT set format string for commit subject and body;
>see 'man git-log' for the list of format metacharacters;
>the default is '%s%n%b%n'
> + --append-dot append a dot to the first line of each commit message if
> + there is no other punctuation or a blank symbol at the end.
I shortened that to "or blank":
(though technically blank is "space or TAB", while \s includes several more)
there is no other punctuation or blank at the end.
> --help display this help and exit
> --versionoutput version information and exit
> @@ -102,12 +104,14 @@ sub quoted_cmd(@)
> {
>my $since_date = '1970-01-01 UTC';
>my $format_string = '%s%n%b%n';
> + my $append_dot = 0;
>GetOptions
> (
> help => sub { usage 0 },
> version => sub { print "$ME version $VERSION\n"; exit },
> 'since=s' => \$since_date,
> 'format=s' => \$format_string,
> + 'append-dot' => \$append_dot,
> ) or usage 1;
>
>my @cmd = (qw (git log --log-size), "--since=$since_date",
> @@ -163,6 +167,17 @@ sub quoted_cmd(@)
> }
>else
> {
> + if ($append_dot)
> +{
> + # If the first line of the message has enough room, then
> + if (length $line[0] < 72)
> +{
> + # append a dot if there is no other punctuation
> + # or a blank symbol at the end.
> + $line[0] =~ /[[:punct:]\s]$/ or $line[0] .= ".";
And put the "or ..." on a 2nd line:
$line[0] =~ /[[:punct:]\s]$/
or $line[0] .= '.';
> +}
> +}
Here's the result.
I'll wait for an ACK from you in case there's anything
you'd like to adjust.
>From 6fa93934508389f8f2dd0f921ebb6c720aa49cdf Mon Sep 17 00:00:00 2001
From: "Dmitry V. Levin"
Date: Sun, 30 Oct 2011 22:00:00 +
Subject: [PATCH] gitlog-to-changelog: new option --append-dot
* build-aux/gitlog-to-changelog: New option --append-dot, makes the
first non-blank line of each commit message terminated with a dot.
---
ChangeLog |6 ++
build-aux/gitlog-to-changelog | 18 +-
2 files changed, 23 insertions(+), 1 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 4c4b7e2..a692aa9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2011-10-30 Dmitry V. Levin
+
+ gitlog-to-changelog: new option --append-dot.
+ * build-aux/gitlog-to-changelog: New option --append-dot, makes the
+ first non-blank line of each commit message terminated with a dot.
+
2011-10-30 Bruno Haible
ffsl, ffsll: Avoid compilation error due to 'restrict'.
diff --git a/build-aux/gitlog-to-changelog b/build-aux/gitlog-to-changelog
index a5fd80d..c3a5ef3 100755
--- a/build-aux/gitlog-to-changelog
+++ b/build-aux/gitlog-to-changelog
@@ -3,7 +3,7 @@ eval '(exit $?0)' && eval 'exec perl -wS "$0" ${1+"$@"}'
if 0;
# Convert git log output to ChangeLog format.
-my $VERSION = '2009-10-30 13:46'; # UTC
+my $VERSION = '2011-10-31 07:45'; # UTC
# The definition above must lie within the first 8 lines in order
# for the Emacs time-stamp write hook (at end) to update it.
# If you change this file with Emacs, please let the write hook
@@ -65,6 +65,8 @@ OPTIONS:
--format=FMT set format string for commit subject and body;
see 'man git-log' for the list of format metacharacters;
the default is '%s%n%b%n'
+ --append-dot append a dot to the first line of each commit message if
+ there is no other punctuation or blank at the end.
--help display this help and exit
--versionoutput version information and exit
@@ -102,12 +104,14 @@ sub quoted_cmd(@)
{
my $since_date = '1970-01-01 UTC';
my $format_string
[PATCH] GNUmakefile: reenable "make syntax-check" for most projects
FYI, I've just pushed the following fix. I share blame, as the reviewer. I did not test Friday's change until today. >From a765077b04a89bffa2a6095c7988fb9a70857d7f Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Sun, 30 Oct 2011 21:24:06 +0100 Subject: [PATCH] GNUmakefile: reenable "make syntax-check" for most projects Since Friday's commit 05e2d798, "maint.mk: don't maintain a second build-aux variable", "syntax-check" would do nothing but succeed with the "No version control files detected..." diagnostic (unless you happened to override _build-aux via cfg.mk). * top/GNUmakefile (_autoreconf, _build-aux): Move default definitions to precede inclusion of maint.mk. Otherwise, these variables would be used undefined in any project that does not override the default. --- ChangeLog | 11 +++ top/GNUmakefile |3 ++- 2 files changed, 13 insertions(+), 1 deletions(-) diff --git a/ChangeLog b/ChangeLog index 30cc2af..99e20ff 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2011-10-30 Jim Meyering + + GNUmakefile: reenable "make syntax-check" for most projects + Since Friday's commit 05e2d798, "maint.mk: don't maintain a second + build-aux variable", "syntax-check" would do nothing but succeed with + the "No version control files detected..." diagnostic (unless you + happened to override _build-aux via cfg.mk). + * top/GNUmakefile (_autoreconf, _build-aux): Move default definitions + to precede inclusion of maint.mk. Otherwise, these variables would + be used undefined in any project that does not override the default. + 2011-10-29 Dmitry V. Levin gitlog-to-changelog: treat a message with only blank lines as empty. diff --git a/top/GNUmakefile b/top/GNUmakefile index 6e00ec8..ca88b6b 100644 --- a/top/GNUmakefile +++ b/top/GNUmakefile @@ -45,12 +45,13 @@ include Makefile # Some projects override e.g., _autoreconf here. -include $(srcdir)/cfg.mk -include $(srcdir)/maint.mk # Allow cfg.mk to override these. _build-aux ?= build-aux _autoreconf ?= autoreconf -v +include $(srcdir)/maint.mk + # Ensure that $(VERSION) is up to date for dist-related targets, but not # for others: rerunning autoreconf and recompiling everything isn't cheap. _have-git-version-gen := \ -- 1.7.7.1.476.g9890
Re: [PATCH 0/2] gitlog-to-changelog output adjustments
Dmitry V. Levin wrote:
> After migrating several projects from GNU-Style ChangeLogs to git with
> autogenerated ChangeLog files, I found a minor discrepancy in style
Hi Dmitry,
If they're using gnulib but not listed in users.txt, please
add their names/URLs. Keeping that list up to date helps us
e.g., when contemplating gnulib changes, we can quickly survey how
each of those client projects is using it.
> between the old and generated ChangeLog files. The git commit message
> convention is that the first line of a commit message is a summary which
> is usually not terminated with a dot. As result, gitlog-to-changelog
> produces a strange mix of not dot-terminated sentences and
> dot-terminated ChangeLogs entries, which looks quite unlike old
> ChangeLog files where every sentence used to be dot-terminated.
>
> I suggest the following adjustments to gitlog-to-changelog output:
>
> gitlog-to-changelog: treat messages without non-blank lines as empty
> gitlog-to-changelog: terminate the 1st line of commit message with a dot
>
> ChangeLog | 12
> build-aux/gitlog-to-changelog | 10 +++---
> 2 files changed, 19 insertions(+), 3 deletions(-)
I see where you're coming from, and if you make each summary line
start with a capital letter, like a regular sentence, then that makes
sense. However, at least in projects I maintain, I don't capitalize
that way, and deliberately avoid the trailing ".", too.
Sometimes, when I can't fit a description on that first line,
I write part of it, and then a comma, and continue with the summary
in the body of the message. So maybe you want to append the "."
only if there is no other punctuation at the end.
Given the lack of leading capital letters in my summary "sentences",
I am reluctant to append a trailing period even in the generated ChangeLog.
However, if you want to add an option to enable the suggested behavior,
that would work.
Also, if that first line is of length 72, or maybe even 71, some may
not want to add that period, since with the ChangeLog's leading TAB,
it might make it wrap.
Thanks for the fix in 1/2, I've adjusted it to avoid the
double negative in the logs:
- gitlog-to-changelog: treat messages without non-blank lines as empty
+ gitlog-to-changelog: treat a message with only blank lines as empty
>From 92822428a2739c16caf12e75a4bba24258da3ec3 Mon Sep 17 00:00:00 2001
From: "Dmitry V. Levin"
Date: Sun, 30 Oct 2011 05:01:00 +0400
Subject: [PATCH] gitlog-to-changelog: treat a message with only blank lines
as empty
* build-aux/gitlog-to-changelog: Move the code that removes leading and
trailing blank lines before the code that issues a warning about an
empty commit message.
---
ChangeLog |7 +++
build-aux/gitlog-to-changelog |8
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 603a16f..30cc2af 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2011-10-29 Dmitry V. Levin
+
+ gitlog-to-changelog: treat a message with only blank lines as empty.
+ * build-aux/gitlog-to-changelog: Move the code that removes leading and
+ trailing blank lines before the code that issues a warning about an
+ empty commit message.
+
2011-10-30 Jim Meyering
test-parse-datetime.c: avoid new DST-related false positive test failure
diff --git a/build-aux/gitlog-to-changelog b/build-aux/gitlog-to-changelog
index 4559704..a5fd80d 100755
--- a/build-aux/gitlog-to-changelog
+++ b/build-aux/gitlog-to-changelog
@@ -152,6 +152,10 @@ sub quoted_cmd(@)
# Omit "Signed-off-by..." lines.
@line = grep !/^Signed-off-by: .*>$/, @line;
+ # Remove leading and trailing blank lines.
+ while ($line[0] =~ /^\s*$/) { shift @line; }
+ while ($line[$#line] =~ /^\s*$/) { pop @line; }
+
# If there were any lines
if (@line == 0)
{
@@ -159,10 +163,6 @@ sub quoted_cmd(@)
}
else
{
- # Remove leading and trailing blank lines.
- while ($line[0] =~ /^\s*$/) { shift @line; }
- while ($line[$#line] =~ /^\s*$/) { pop @line; }
-
# Prefix each non-empty line with a TAB.
@line = map { length $_ ? "\t$_" : '' } @line;
--
1.7.7.1.476.g9890
Re: [PATCH] test-parse-datetime.c: avoid new DST-related false positive test failure
Jim Meyering wrote: > With last night's time switch here (from CEST to CST), > I noticed that coreutils' "make check" would fail like this: > > test-parse-datetime.c:142: assertion failed > > Fixed with this: > > Subject: [PATCH] test-parse-datetime.c: avoid new DST-related false positive > test failure > > * tests/test-parse-datetime.c (gmt_offset): Determine the "gmt_offset" > based on the time/date we'll convert, not the current time. > Otherwise, the moment we cross a DST boundary like today's in > Europe, (CEST to CET), that offset ends up being one hour off. > --- > ChangeLog |8 > tests/test-parse-datetime.c | 22 ++ That was for gnulib. The patch below pulls the fix into coreutils. I'll probably make a release in the next two weeks, partly in order to forestall bug reports when more countries switch. Besides, we're already up to three "Bug fixes", even if they're all minor. ** Bug fixes rm -rf DIR would fail with "Device or resource busy" on Cygwin with NWFS and NcFsd file systems. This did not affect Unix/Linux-based kernels. [bug introduced in coreutils-8.0, when rm began using fts] tac no longer fails to handle two or more non-seekable inputs [bug introduced in coreutils-5.3.0] tail -f no longer tries to use inotify on GPFS file systems [you might say this was introduced in coreutils-7.5, along with inotify support, but the GPFS magic number wasn't in the usual places then.] >From 53f48c7b0e4e574a73f5a649f938a0b537489d34 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Sun, 30 Oct 2011 12:42:55 +0100 Subject: [PATCH] tests: update gnulib to latest to avoid FP DST-related test failure Otherwise, "make check" would fail after e.g., a CEST-to-CST daylight savings transition. See http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/28825 --- gnulib |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/gnulib b/gnulib index f1a5c91..56ddf0f 16 --- a/gnulib +++ b/gnulib @@ -1 +1 @@ -Subproject commit f1a5c91522554791317dc2ee763fe8c017c7b810 +Subproject commit 56ddf0fdeb52ce76718e0594db4f567401e90a2c -- 1.7.7.1.476.g9890
[PATCH] test-parse-datetime.c: avoid new DST-related false positive test failure
With last night's time switch here (from CEST to CST),
I noticed that coreutils' "make check" would fail like this:
test-parse-datetime.c:142: assertion failed
Fixed with this:
>From 56ddf0fdeb52ce76718e0594db4f567401e90a2c Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Sun, 30 Oct 2011 18:12:54 +0100
Subject: [PATCH] test-parse-datetime.c: avoid new DST-related false positive
test failure
* tests/test-parse-datetime.c (gmt_offset): Determine the "gmt_offset"
based on the time/date we'll convert, not the current time.
Otherwise, the moment we cross a DST boundary like today's in
Europe, (CEST to CET), that offset ends up being one hour off.
---
ChangeLog |8
tests/test-parse-datetime.c | 22 ++
2 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index f4711dc..603a16f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2011-10-30 Jim Meyering
+
+ test-parse-datetime.c: avoid new DST-related false positive test failure
+ * tests/test-parse-datetime.c (gmt_offset): Determine the "gmt_offset"
+ based on the time/date we'll convert, not the current time.
+ Otherwise, the moment we cross a DST boundary like today's in
+ Europe, (CEST to CET), that offset ends up being one hour off.
+
2011-10-27 Bruno Haible
fstat: Tweak documentation.
diff --git a/tests/test-parse-datetime.c b/tests/test-parse-datetime.c
index b9d08a6..22fe9bc 100644
--- a/tests/test-parse-datetime.c
+++ b/tests/test-parse-datetime.c
@@ -94,20 +94,17 @@ tm_diff (struct tm const *a, struct tm const *b)
#endif /* ! HAVE_TM_GMTOFF */
static long
-gmt_offset ()
+gmt_offset (time_t s)
{
- time_t now;
long gmtoff;
- time (&now);
-
#if !HAVE_TM_GMTOFF
- struct tm tm_local = *localtime (&now);
- struct tm tm_gmt = *gmtime (&now);
+ struct tm tm_local = *localtime (&s);
+ struct tm tm_gmt = *gmtime (&s);
gmtoff = tm_diff (&tm_local, &tm_gmt);
#else
- gmtoff = localtime (&now)->tm_gmtoff;
+ gmtoff = localtime (&s)->tm_gmtoff;
#endif
return gmtoff;
@@ -123,16 +120,17 @@ main (int argc _GL_UNUSED, char **argv)
const char *p;
int i;
long gmtoff;
+ time_t ref_time = 1304250918;
set_program_name (argv[0]);
- gmtoff = gmt_offset ();
+ gmtoff = gmt_offset (ref_time);
/* ISO 8601 extended date and time of day representation,
'T' separator, local time zone */
p = "2011-05-01T11:55:18";
- expected.tv_sec = 1304250918 - gmtoff;
+ expected.tv_sec = ref_time - gmtoff;
expected.tv_nsec = 0;
ASSERT (parse_datetime (&result, p, 0));
LOG (p, expected, result);
@@ -142,7 +140,7 @@ main (int argc _GL_UNUSED, char **argv)
/* ISO 8601 extended date and time of day representation,
' ' separator, local time zone */
p = "2011-05-01 11:55:18";
- expected.tv_sec = 1304250918 - gmtoff;
+ expected.tv_sec = ref_time - gmtoff;
expected.tv_nsec = 0;
ASSERT (parse_datetime (&result, p, 0));
LOG (p, expected, result);
@@ -153,7 +151,7 @@ main (int argc _GL_UNUSED, char **argv)
/* ISO 8601, extended date and time of day representation,
'T' separator, UTC */
p = "2011-05-01T11:55:18Z";
- expected.tv_sec = 1304250918;
+ expected.tv_sec = ref_time;
expected.tv_nsec = 0;
ASSERT (parse_datetime (&result, p, 0));
LOG (p, expected, result);
@@ -163,7 +161,7 @@ main (int argc _GL_UNUSED, char **argv)
/* ISO 8601, extended date and time of day representation,
' ' separator, UTC */
p = "2011-05-01 11:55:18Z";
- expected.tv_sec = 1304250918;
+ expected.tv_sec = ref_time;
expected.tv_nsec = 0;
ASSERT (parse_datetime (&result, p, 0));
LOG (p, expected, result);
--
1.7.7.1.476.g9890
Re: [PATCH] maint.mk: don't maintain a second build-aux variable.
Gary V. Vaughan wrote: > Hi Jim, ... >>> -# Override this in cfg.mk if you use a non-standard build-aux directory. >>> -build_aux ?= $(srcdir)/build-aux >>> +ifneq ($(build_aux),) >>> + $(error '*** set $$(_build-aux) relative to $$(srcdir) instead of >>> $$(build_aux)') >> >> That line is longer than 80. >> Please shorten or split it, and > > Done. > >> use a prefix of "$(ME): " as >> in most of the other diagnostics in this file. Or at least >> include $(ME) somewhere, so people know where it's coming from. > > $ fgrep '$(error ' ~/Devo/gnulib/top/* > top/maint.mk: $(error '*** set $$(_build-aux) relative to $$(srcdir) instead > of $$(build_aux)') > top/maint.mk: $(error '*** you need to > s/_prohibit_regexp/_sc_search_regexp/, and adapt') > > I changed the one error I added as requested, but I guess the other use which > I copied from needs to have $(ME) added too (and be wrapped to avoid breaking > the 80 column limit)? You're welcome to change it if you'd like, but like your addition, that $(error ...) provoker is solely to alert people to a problem arising from a variable name change, and it's due to be removed in a few months: # _sc_search_regexp used to be named _prohibit_regexp. However, # upgrading to the new definition and leaving the old name undefined # would usually convert each custom rule using $(_prohibit_regexp) # (usually defined in cfg.mk) into a no-op. This definition ensures # that people know right away if they're still using the old name. # FIXME: remove in 2012. _prohibit_regexp = \ $(error '*** you need to s/_prohibit_regexp/_sc_search_regexp/, and adapt') Please add a similar FIXME comment (but with say "2013"), so we do eventually remove it. > Okay to push? Yes. Thanks.
Re: [PATCH] maint.mk: don't maintain a second build-aux variable.
Gary V. Vaughan wrote: > Hi Jim, > > On 25 Oct 2011, at 15:18, Gary V. Vaughan wrote: >> On 25 Oct 2011, at 15:05, Jim Meyering wrote: >>> Actually, I think we can both get what we want. >>> I suggest to adjust your patch so that make is guaranteed to fail >>> with a nice diagnostic for anyone who defines build_aux in cfg.mk. >> >> It's in my queue, so I should get to it in a few days. Thanks for the >> feedback. > > Okay to push? > > * maint.mk (build-aux): Removed. The maintainer-makefile module Almost. Please sync at least the above commit-log line from your ChangeLog. That will fix the "build-aux" (s/-/_/) typo. > depends on GNUmakefile, which already maintains a cfg.mk > overridable $(_build-aux) for projects with a non-standard > build-aux directory location, although without the $(srcdir) > prefix. Use that variable consistently instead of introducing a > second one. Adjust all call sites. > --- > ChangeLog| 10 ++ > top/maint.mk | 17 + > 2 files changed, 19 insertions(+), 8 deletions(-) > > diff --git a/ChangeLog b/ChangeLog > index 93ee45e..660b9ce 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,3 +1,13 @@ > +2011-10-27 Gary V. Vaughan > + > + maint.mk: don't maintain a second build-aux variable. > + * maint.mk (build_aux): Removed. The maintainer-makefile module > + depends on GNUmakefile, which already maintains a cfg.mk > + overridable $(_build-aux) for projects with a non-standard > + build-aux directory location, although without the $(srcdir) > + prefix. Use that variable consistently instead of introducing a > + second one. Adjust all call sites. ... > diff --git a/top/maint.mk b/top/maint.mk ... > ME := maint.mk > > -# Override this in cfg.mk if you use a non-standard build-aux directory. > -build_aux ?= $(srcdir)/build-aux > +ifneq ($(build_aux),) > + $(error '*** set $$(_build-aux) relative to $$(srcdir) instead of > $$(build_aux)') That line is longer than 80. Please shorten or split it, and use a prefix of "$(ME): " as in most of the other diagnostics in this file. Or at least include $(ME) somewhere, so people know where it's coming from. Thanks.
Re: sc_prohibit_stddef_without_use works incorrectly
Alfred M. Szmidt wrote:
> The following syntax-check is broken,
>
>_stddef_syms_re = NULL|offsetof|ptrdiff_t|size_t|wchar_t
># Prohibit the inclusion of stddef.h without an actual use.
>sc_prohibit_stddef_without_use:
> h='stddef.h'\
> re='\<($(_stddef_syms_re)) *\(' \
> $(_sc_header_without_use)
>
> This tries to match "NULL (", NULL can exist in other situations:
>
> gettimeofday (&tv, NULL);
>
> which will not be matched by the above, the same goes for ptrdiff_t,
> size_t and wchar_t (as variable type specifiers).
Thanks for the report.
This should fix it:
>From f7dbcea690572e75a85aea1263a222b11414bdd5 Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Tue, 25 Oct 2011 15:49:18 +0200
Subject: [PATCH] maint.mk: fix a bug in sc_prohibit_stddef_without_use
* top/maint.mk (sc_prohibit_stddef_without_use): Don't require / *\(/
after symbols like NULL, size_t, etc.
Reported by Alfred M. Szmidt.
---
ChangeLog|5 +
top/maint.mk |2 +-
2 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 337cd03..546005e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
2011-10-25 Jim Meyering
+ maint.mk: fix a bug in sc_prohibit_stddef_without_use
+ * top/maint.mk (sc_prohibit_stddef_without_use): Don't require / *\(/
+ after symbols like NULL, size_t, etc.
+ Reported by Alfred M. Szmidt.
+
maint.mk: exempt ENODATA from a syntax-check rule
* top/maint.mk (gl_extract_significant_defines_): Also exempt ENODATA
from the sc_prohibit_always-defined_macros syntax-check rule.
diff --git a/top/maint.mk b/top/maint.mk
index be348dd..4c71187 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -618,7 +618,7 @@ _stddef_syms_re = NULL|offsetof|ptrdiff_t|size_t|wchar_t
# Prohibit the inclusion of stddef.h without an actual use.
sc_prohibit_stddef_without_use:
@h='stddef.h' \
- re='\<($(_stddef_syms_re)) *\(' \
+ re='\<($(_stddef_syms_re))\>' \
$(_sc_header_without_use)
# Prohibit the inclusion of verify.h without an actual use.
--
1.7.7.419.g87009
Re: ENODATA
Jim Meyering wrote: > Bruno Haible wrote: > >> Hi Jim, >> >>> The gnulib update induced a new (coreutils-specific) syntax-check failure: >>> >>> src/system.h:# define ENODATA (-1) >>> make[3]: *** [sc_prohibit_always-defined_macros] Error 1 >>> >>> because gnulib now defines that symbol, so I have also removed >>> that definition from coreutils: >>> >>> - >>> -/* Some systems don't define this; POSIX mentions it but says it is >>> - obsolete, so gnulib does not provide it either. */ >>> -#ifndef ENODATA >>> -# define ENODATA (-1) >>> -#endif >>> - >> >> Actually, gnulib's errno.h defines ENODATA only on native Windows systems, >> because MSVC 10 does it as well - to avoid interoperability problems. >> It doesn't guarantee the existence of this macro, and there is no intention >> that it does. ENODATA is not POSIX. It is merely an error code defined >> on _some_ systems. Just like EOTHER, ETOOMANYREFS, ENOSTR are error >> codes defined on _some_ systems. >> >> I would get rid of the syntax-check failure in a different way. > > Oh! Thank you. > I'll definitely revert that part. I've done the following to exempt ENODATA from that test in maint.mk. Exempting the entire system.h file via coreutils' cfg.mk would not have been appropriate, since that's precisely where such definitions are most likely to be. >From f1a5c91522554791317dc2ee763fe8c017c7b810 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Tue, 25 Oct 2011 12:26:49 +0200 Subject: [PATCH] maint.mk: exempt ENODATA from a syntax-check rule * top/maint.mk (gl_extract_significant_defines_): Also exempt ENODATA from the sc_prohibit_always-defined_macros syntax-check rule. Add a comment. See this for more details: http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/28739/focus=28795 --- ChangeLog|8 top/maint.mk |4 +++- 2 files changed, 11 insertions(+), 1 deletions(-) diff --git a/ChangeLog b/ChangeLog index a4ac818..337cd03 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2011-10-25 Jim Meyering + + maint.mk: exempt ENODATA from a syntax-check rule + * top/maint.mk (gl_extract_significant_defines_): Also exempt ENODATA + from the sc_prohibit_always-defined_macros syntax-check rule. + Add a comment. See this for more details: + http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/28739/focus=28795 + 2011-10-23 Jim Meyering fts: close parent dir FD before returning from post-traversal fts_read diff --git a/top/maint.mk b/top/maint.mk index d51fec6..be348dd 100644 --- a/top/maint.mk +++ b/top/maint.mk @@ -749,10 +749,12 @@ gl_other_headers_ ?= \ # Perl -lne code to extract "significant" cpp-defined symbols from a # gnulib header file, eliminating a few common false-positives. +# The exempted names below are defined only conditionally in gnulib, +# and hence sometimes must/may be defined in application code. gl_extract_significant_defines_ = \ /^\# *define ([^_ (][^ (]*)(\s*\(|\s+\w+)/\ && $$2 !~ /(?:rpl_|_used_without_)/\ -&& $$1 !~ /^(?:NSIG)$$/\ +&& $$1 !~ /^(?:NSIG|ENODATA)$$/\ && $$1 !~ /^(?:SA_RESETHAND|SA_RESTART)$$/\ and print $$1 -- 1.7.7.419.g87009
Re: ENODATA
Bruno Haible wrote: > Hi Jim, > >> The gnulib update induced a new (coreutils-specific) syntax-check failure: >> >> src/system.h:# define ENODATA (-1) >> make[3]: *** [sc_prohibit_always-defined_macros] Error 1 >> >> because gnulib now defines that symbol, so I have also removed >> that definition from coreutils: >> >> - >> -/* Some systems don't define this; POSIX mentions it but says it is >> - obsolete, so gnulib does not provide it either. */ >> -#ifndef ENODATA >> -# define ENODATA (-1) >> -#endif >> - > > Actually, gnulib's errno.h defines ENODATA only on native Windows systems, > because MSVC 10 does it as well - to avoid interoperability problems. > It doesn't guarantee the existence of this macro, and there is no intention > that it does. ENODATA is not POSIX. It is merely an error code defined > on _some_ systems. Just like EOTHER, ETOOMANYREFS, ENOSTR are error > codes defined on _some_ systems. > > I would get rid of the syntax-check failure in a different way. Oh! Thank you. I'll definitely revert that part.
Re: waitpid and warn_unused_result
Eric Blake wrote: > Any objections to making the gnulib apply the gcc > warn_unused_result attribute to waitpid(), even though glibc does not > (yet) do likewise? After all, if you fail to check for failures on > this syscall (particularly EINTR failures, where you should normally > repeat the waitpid() call), you can end up leaking unreaped child > processes, which is a rather nasty resource leak to debug. That would be welcome. IMHO, we need not wait for glibc to add the warn_unused_result attribute.
Re: [PATCH] maint.mk: don't maintain a second build-aux variable.
[please configure your mail client to stop including both plain and html versions of each message] Gary V. Vaughan wrote: > To what advantage over factoring out the duplication entirely at the > source of the problem with the patch I submitted? Not breaking some project's existing set-up when they next update from gnulib. Alerting the user that they need to change something to make these two values consistent. > Wouldn't that break things for those who customize build_aux? > > Yes, and that's the problem I encountered that inspired me to > send a patch to fix it. It would be better to alert them to the problem. > How about merely ensuring that they're consistent, for now, > i.e., by adding something like this: > > ifneq ($(build_aux),$(_build-aux)) > $(error ...) > endif > > Do you mean: > > ifneq ($(srcdir)/$(_build-aux), $(build_aux)) > $(error ...) > endif > > in GNUmakefile? or in maint.mk? or in cfg.mk? It belongs in maint.mk, since all uses of build_aux are there, and there are more of them than of _build-aux in GNUmakefile: $ cd top && grep -c build_aux GNUmakefile maint.mk GNUmakefile:0 maint.mk:7 Definitely not in cfg.mk, since that is not distributed via gnulib. > Again, what's the advantage of kludging it instead of just making > GNUmakefile and maint.mk use the same variable name for the ... "kludge"? We who work on gnulib try hard not to let changes there cause silent malfunction in client packages. When we must make changes that may break clients, it is only as a last resort, and preferably with automated warning. The minimal requirement is to warn about such a change in NEWS. Actually, I think we can both get what we want. I suggest to adjust your patch so that make is guaranteed to fail with a nice diagnostic for anyone who defines build_aux in cfg.mk.
Re: [PATCH] maint.mk: don't maintain a second build-aux variable.
Gary V. Vaughan wrote: > I was wondering why 'make stable' would always use a stale version unless > I manually updated my .version file first. It turns out that if you use > a non-standard build-aux location, you have to tell GNUmakefile by setting > _build-aux to get the .version dist-hook machinery, and that you also have > to set build_aux in order for maint.mk to work properly. > > Before this patch, my cfg.mk needed: > > build_aux = $(srcdir)/libltdl/build-aux > _build-aux = libltdl/build-aux I would leave them for now, but factor out the duplication: _build-aux = libltdl/build-aux build_aux = $(srcdir)/$(_build-aux) > This patch combines the two so that you I declare my build-aux directory > with just: > > _build-aux = libltdl/build-aux > > Alternatively, we could change maint.mk to read: > > build-aux = $(srcdir)/$(_build-aux) > > ...but that leaves open the possibility of users like me only setting one > or the other variable in cfg.mk and wondering why things almost work, but > not quite. I think this is the cleaner, safer solution. Wouldn't that break things for those who customize build_aux? How about merely ensuring that they're consistent, for now, i.e., by adding something like this: ifneq ($(build_aux),$(_build-aux)) $(error ...) endif
Re: rm -rf calls rmdir() prior to close(), which can fail
Jim Meyering wrote: >> Here is the patch that I expect to push tomorrow: ... > I've fixed/improved the ChangeLog/commit-log: > > Subject: [PATCH] fts: close parent dir FD before returning from > post-traversal fts_read > > The problem: the fts-using "mkdir -p A/B; rm -rf A" would attempt to > unlink A, even though an FD open on A remained. This is suboptimal > (holding a file descriptor open longer than needed), but otherwise not > a problem on Unix-like kernels. However, on Cygwin with certain Novell > file systems, (see http://cygwin.com/ml/cygwin/2011-10/msg00365.html), > that represents a real problem: it causes the removal of A to fail > with e.g., "rm: cannot remove `A': Device or resource busy" > > fts visits each directory twice and keeps a cache (fts_fd_ring) of > directory file descriptors. After completing the final, FTS_DP, > visit of a directory, RESTORE_INITIAL_CWD intended to clear the FD > cache, but then proceeded to add a new FD to it via the subsequent > FCHDIR (which calls cwd_advance_fd and i_ring_push). Before, the > final file descriptor would be closed only via fts_close's call to > fd_ring_clear. Now, it is usually closed earlier, via the final > FTS_DP-returning fts_read call. > * lib/fts.c (restore_initial_cwd): New function, converted from > the macro. Call fd_ring_clear *after* FCHDIR, not before it. > Update callers. > Reported by Franz Sirl via the above URL, with analysis by Eric Blake > in http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/28739 I pushed that, along with the following in coreutils. The gnulib update induced a new (coreutils-specific) syntax-check failure: src/system.h:# define ENODATA (-1) make[3]: *** [sc_prohibit_always-defined_macros] Error 1 because gnulib now defines that symbol, so I have also removed that definition from coreutils: >From f8ae6440eb8f943fd1f040d039753851824512d3 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Mon, 24 Oct 2011 10:27:22 +0200 Subject: [PATCH] rm: update gnulib to get an fts fix for Cygwin+NWFS/NcFsd file systems * NEWS (Bug fixes): Mention it. As far as we know, this fix affects only Cygwin with NWFS or NcFsd file systems. See these: http://git.sv.gnu.org/cgit/gnulib.git/commit/?id=71f13422f3e634 http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/28739 http://cygwin.com/ml/cygwin/2011-10/msg00365.html * src/system.h (ENODATA): Remove fall-back definition, now that gnulib provides one. Caught by the sc_prohibit_always-defined_macros syntax-check rule. Also remove now-irrelevant "Don't use bcopy..." comment. --- NEWS |4 gnulib |2 +- src/system.h | 11 --- 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/NEWS b/NEWS index 4d210b5..b73057a 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,10 @@ GNU coreutils NEWS-*- outline -*- ** Bug fixes + rm -rf DIR would fail with "Device or resource busy" on Cygwin with NWFS + and NcFsd file systems. This did not affect Unix/Linux-based kernels. + [bug introduced in coreutils-7.0, when rm began using fts] + tac no longer fails to handle two or more non-seekable inputs [bug introduced in coreutils-5.3.0] diff --git a/gnulib b/gnulib index 6a4c64c..71f1342 16 --- a/gnulib +++ b/gnulib @@ -1 +1 @@ -Subproject commit 6a4c64ce4a59bd9589e63fb5ee480765d356f8c7 +Subproject commit 71f13422f3e6345933513607255f1f7a7526e937 diff --git a/src/system.h b/src/system.h index 18ac0cc..19421a9 100644 --- a/src/system.h +++ b/src/system.h @@ -74,19 +74,8 @@ you must include before including this file # define makedev(maj, min) mkdev (maj, min) #endif -/* Don't use bcopy! Use memmove if source and destination may overlap, - memcpy otherwise. */ - #include - #include - -/* Some systems don't define this; POSIX mentions it but says it is - obsolete, so gnulib does not provide it either. */ -#ifndef ENODATA -# define ENODATA (-1) -#endif - #include #include #include "version.h" -- 1.7.7.419.g87009
Re: rm -rf calls rmdir() prior to close(), which can fail
Jim Meyering wrote: ... > Here is the patch that I expect to push tomorrow: > > Subject: [PATCH] fts: close parent dir FD before returning from > post-traversal fts_read > > The problem: the fts-using "rm -rf A/B/" would attempt to unlink A, > while a file descriptor open on A remained. This is suboptimal > (holding a file descriptor open longer than needed) on Linux, but > otherwise not a problem. However, on Cygwin with certain file system > types, (see http://cygwin.com/ml/cygwin/2011-10/msg00365.html), that > represents a real problem: it causes the removal of A to fail with > e.g., "rm: cannot remove `A': Device or resource busy" > > fts visits each directory twice and keeps a cache (fts_fd_ring) of > directory file descriptors. After completing the final, FTS_DP, > visit of a directory, RESTORE_INITIAL_CWD intended to clear the FD > cache, but then proceeded to add a new FD to it via the subsequent > FCHDIR (which calls cwd_advance_fd and i_ring_push). Before, the > final file descriptor would be closed only via fts_close's call to > fd_ring_clear. Now, it is usually closed earlier, via the final > FTS_DP-returning fts_read call. > * lib/fts.c (restore_initial_cwd): New function, converted from > the macro. Call fd_ring_clear *after* FCHDIR, not before it. I've fixed/improved the ChangeLog/commit-log: >From 71f13422f3e6345933513607255f1f7a7526e937 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Sun, 23 Oct 2011 22:42:25 +0200 Subject: [PATCH] fts: close parent dir FD before returning from post-traversal fts_read The problem: the fts-using "mkdir -p A/B; rm -rf A" would attempt to unlink A, even though an FD open on A remained. This is suboptimal (holding a file descriptor open longer than needed), but otherwise not a problem on Unix-like kernels. However, on Cygwin with certain Novell file systems, (see http://cygwin.com/ml/cygwin/2011-10/msg00365.html), that represents a real problem: it causes the removal of A to fail with e.g., "rm: cannot remove `A': Device or resource busy" fts visits each directory twice and keeps a cache (fts_fd_ring) of directory file descriptors. After completing the final, FTS_DP, visit of a directory, RESTORE_INITIAL_CWD intended to clear the FD cache, but then proceeded to add a new FD to it via the subsequent FCHDIR (which calls cwd_advance_fd and i_ring_push). Before, the final file descriptor would be closed only via fts_close's call to fd_ring_clear. Now, it is usually closed earlier, via the final FTS_DP-returning fts_read call. * lib/fts.c (restore_initial_cwd): New function, converted from the macro. Call fd_ring_clear *after* FCHDIR, not before it. Update callers. Reported by Franz Sirl via the above URL, with analysis by Eric Blake in http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/28739 --- ChangeLog | 25 + lib/fts.c | 23 +++ 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/ChangeLog b/ChangeLog index 93ee45e..a4ac818 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,28 @@ +2011-10-23 Jim Meyering + + fts: close parent dir FD before returning from post-traversal fts_read + The problem: the fts-using "mkdir -p A/B; rm -rf A" would attempt to + unlink A, even though an FD open on A remained. This is suboptimal + (holding a file descriptor open longer than needed), but otherwise not + a problem on Unix-like kernels. However, on Cygwin with certain Novell + file systems, (see http://cygwin.com/ml/cygwin/2011-10/msg00365.html), + that represents a real problem: it causes the removal of A to fail + with e.g., "rm: cannot remove `A': Device or resource busy" + + fts visits each directory twice and keeps a cache (fts_fd_ring) of + directory file descriptors. After completing the final, FTS_DP, + visit of a directory, RESTORE_INITIAL_CWD intended to clear the FD + cache, but then proceeded to add a new FD to it via the subsequent + FCHDIR (which calls cwd_advance_fd and i_ring_push). Before, the + final file descriptor would be closed only via fts_close's call to + fd_ring_clear. Now, it is usually closed earlier, via the final + FTS_DP-returning fts_read call. + * lib/fts.c (restore_initial_cwd): New function, converted from + the macro. Call fd_ring_clear *after* FCHDIR, not before it. + Update callers. + Reported by Franz Sirl via the above URL, with analysis by Eric Blake + in http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/28739 + 2011-10-23 Gary V. Vaughan Bruno Haible Jim Meyering diff --git a/lib/fts.c b/lib/fts.c index e3829f3..f61a91e 100644 --- a/lib/fts.c +++ b/lib/fts.c @@ -229,11 +229,6 @@ static int fts_safe_changedir (FTS *
Re: rm -rf calls rmdir() prior to close(), which can fail
Eric Blake wrote: > POSIX is clear that attempts to rmdir() a directory that still has > open descriptors may fail. Of course, on Linux, this (rather > limiting) restriction is not present, so we don't notice it; but on > Cygwin, there are certain file systems where this is a real problem, > such as in this thread: > http://cygwin.com/ml/cygwin/2011-10/msg00365.html > > Looking at an strace on Linux reveals the problem (abbreviated to show > highlights here): > > $ mkdir -p a/b > $ strace rm -f a > ... > openat(AT_FDCWD, "a", O_RDONLY|O_NOCTTY|O_NONBLOCK|O_DIRECTORY|O_NOFOLLOW) = 3 > ... > fcntl(3, F_DUPFD, 3)= 4 > ... > close(3)= 0 > ... > openat(4, "b", O_RDONLY|O_NOCTTY|O_NONBLOCK|O_DIRECTORY|O_NOFOLLOW) = 3 > ... > fcntl(3, F_DUPFD, 3)= 5 > ... > close(3)= 0 > close(5)= 0 > unlinkat(4, "b", AT_REMOVEDIR) = 0 > unlinkat(AT_FDCWD, "a", AT_REMOVEDIR) = 0 > close(4)= 0 > > Notice that for subdirectories, we opened the directory, then used dup > to have a handle for use in further *at calls, then do > fdopendir/readdir/closedir on the DIR*, then close the duplicate fd, > all before calling unlinkat (aka rmdir) on that subdirectory. But for > the top-level directory, the dup'd fd (4) is still open when we > attempt the unlinkat. Thanks for the analysis, Eric. That was due to a rather subtle but easy/safe-to-fix bug. While the rm from coreutils-8.14 worked as your strace above shows, the fixed one does this: (note how the close(4) now precedes the removal of "a") $ mkdir -p a/b $ strace -e openat,close,unlinkat ./rm -rf a close(3)= 0 close(3)= 0 openat(AT_FDCWD, "a", O_RDONLY|O_NOCTTY|O_NONBLOCK|O_DIRECTORY|O_NOFOLLOW) = 3 close(3)= 0 openat(4, "b", O_RDONLY|O_NOCTTY|O_NONBLOCK|O_DIRECTORY|O_NOFOLLOW) = 3 close(3)= 0 close(5)= 0 unlinkat(4, "b", AT_REMOVEDIR) = 0 close(4)= 0 unlinkat(AT_FDCWD, "a", AT_REMOVEDIR) = 0 close(0)= 0 close(1) = 0 close(2)= 0 Here is the patch that I expect to push tomorrow: >From a11c49cd72a91c05a272e36ff5d3cd92675cbfb5 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Sun, 23 Oct 2011 22:42:25 +0200 Subject: [PATCH] fts: close parent dir FD before returning from post-traversal fts_read The problem: the fts-using "rm -rf A/B/" would attempt to unlink A, while a file descriptor open on A remained. This is suboptimal (holding a file descriptor open longer than needed) on Linux, but otherwise not a problem. However, on Cygwin with certain file system types, (see http://cygwin.com/ml/cygwin/2011-10/msg00365.html), that represents a real problem: it causes the removal of A to fail with e.g., "rm: cannot remove `A': Device or resource busy" fts visits each directory twice and keeps a cache (fts_fd_ring) of directory file descriptors. After completing the final, FTS_DP, visit of a directory, RESTORE_INITIAL_CWD intended to clear the FD cache, but then proceeded to add a new FD to it via the subsequent FCHDIR (which calls cwd_advance_fd and i_ring_push). Before, the final file descriptor would be closed only via fts_close's call to fd_ring_clear. Now, it is usually closed earlier, via the final FTS_DP-returning fts_read call. * lib/fts.c (restore_initial_cwd): New function, converted from the macro. Call fd_ring_clear *after* FCHDIR, not before it. Update callers. Reported by Franz Sirl via the above URL, with analysis by Eric Blake in http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/28739 --- ChangeLog | 25 +++++ lib/fts.c | 23 +++ 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/ChangeLog b/ChangeLog index 93ee45e..3a2d2cc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,28 @@ +2011-10-23 Jim Meyering + + fts: close parent dir FD before returning from post-traversal fts_read + The problem: the fts-using "rm -rf A/B/" would attempt to unlink A, + while a file descriptor open on A remained. This is suboptimal + (holding a file descriptor open longer than needed) on Linux, but + otherwise not a problem. However, on Cygwin with certain file system + types, (see http://cygwin.com/ml/cygwin/2011-10/msg00365.html), that + represents a real problem: it causes the removal of A to fail with + e.g., "rm: cannot remove `
Re: [PATCH] README-release improvements
Stefano Lattarini wrote: > Hi everybody. Just my two cents about this matter ... > > On Saturday 22 October 2011, Bruno Haible wrote: >> Gary V. Vaughan wrote: >> > >> Running the (potentially) outdated configure, to build a (potentially) >> > >> outdated Makefile, which may very well rerun aclocal, automake, autoconf >> > >> etc just to call the maintainer-clean rule, and then blow it all away >> > >> in the next step with a bootstrap seems odd and torturous to me. >> > > >> > > The above may be a little wasteful. But it is reliable and turn-key. >> > >> > No it isn't. >> >> I'm with Gary on this point. >> >> How about these steps? >> >> make -k maintainer-clean # Clean up previous build artefacts >> ./configure >> make maintainer-clean# Clean up previous build artefacts >> git checkout master >> git pull origin master # don't try (perhaps fail) to pull other branches >> ./bootstrap # rerun autotools explicitly on latest sources >> ./configure # regenerate Makefile explicitly -"- -"- >> > At this point, wouldn't it be simpler (and maybe safer) to clone the git > repository in a new directory, and simply do "./bootstrap && ./configure" > from there? Hi Stefano, That would be simpler, but might end up masking a problem in the long run. I want to be sure that my usual build directory has the latest versions of byproducts produced by tools like bison. With an explicit "make maintainerclean" at least just before each release, I can't accidentally continue to use an old generated file for too long.
Re: [PATCH] README-release improvements
Gary V. Vaughan wrote:
...
> Okay to push?
Yes, but with the following changes applied:
use this instead, per today's discussion:
make -k maintainer-clean || { ./configure && make maintainer-clean; }
and change "incase" to "in case" in the commit log and ChangeLog:
> readme-release: several release instructions improvements.
>
> * README-release: Don't git pull all branches when only master
> is needed for the release process.
> Don't try to run ./configure right after git pull incase files
...
Re: [PATCH] README-release improvements
Bruno Haible wrote:
> Gary V. Vaughan wrote:
>> >> Running the (potentially) outdated configure, to build a (potentially)
>> >> outdated Makefile, which may very well rerun aclocal, automake, autoconf
>> >> etc just to call the maintainer-clean rule, and then blow it all away
>> >> in the next step with a bootstrap seems odd and torturous to me.
>> >
>> > The above may be a little wasteful. But it is reliable and turn-key.
>>
>> No it isn't.
>
> I'm with Gary on this point.
And so am I.
My objection was to the *removal* of the "make maintainer-clean" step.
I guess you didn't see the reply where I said a patch moving it to
precede the checkout+pull would be welcome.
> How about these steps?
>
> make -k maintainer-clean # Clean up previous build artefacts
Adding the -k is good, too.
> ./configure
> make maintainer-clean# Clean up previous build artefacts
What's the point of rerunning configure and maintainer-clean?
I can see doing that if the make -k ... failed, e.g.,
make -k maintainer-clean || { ./configure && make maintainer-clean; }
but not unconditionally.
Some other reason?
> git checkout master
> git pull origin master # don't try (perhaps fail) to pull other branches
> ./bootstrap # rerun autotools explicitly on latest sources
> ./configure # regenerate Makefile explicitly -"- -"-
>
> Doing the "make maintainer-clean" with the old Makefiles is the most
> reliable you can do.
>
> Now this sequence takes a little more time than what you proposed, Jim.
> But it's only needed once before the release. You don't object to the
> time it takes to run "make distcheck" either.
Re: [PATCH] README-release improvements
Gary V. Vaughan wrote: > Hi Jim, > > On 22 Oct 2011, at 03:06, Jim Meyering wrote: >> Gary V. Vaughan wrote: >>> As it stands (without this patch), README-release recommends: >>> >>> git checkout master >>> git pull >>> ./configure >>> make maintainer-clean >>> ./bootstrap >>> >>> Running the (potentially) outdated configure, to build a (potentially) >>> outdated Makefile, which may very well rerun aclocal, automake, autoconf >>> etc just to call the maintainer-clean rule, and then blow it all away >>> in the next step with a bootstrap seems odd and torturous to me. >> >> The above may be a little wasteful. But it is reliable and turn-key. > > No it isn't. Imagine that the bootstrap process has changed upstream, > and that, for example, additional bootstrap steps are needed to fix a > bug in maintainer-clean. The README-release mandated procedure tells > you to run the old maintainer-clean rule after potentially merging with > a new tree that has different requirements. And you don't even rerun > bootstrap first to ensure that you get a Makefile with a matching > maintainer-clean rule. > > It's very easy to think of a multitude of scenarios where it's not only > suboptimal but flat out wrong to recommend a full autoreconf run, and Omitting the "make maintainerclean" step can result in putting stale generated files in a distribution tarball. Running things in the wrong order wastes time, but I see no other ill effect. E.g., if you remove a gnulib module like parse-datetime, with rules that run $(YACC). Then, the post-pull bootstrap'd Makefiles will lack the rule to remove the generated .c file. People rarely remove modules like that, and even when they do, leaving around a stray non-distributed file wouldn't hurt anyone. Of course, it's better not to do that, but unlike omitting "make maintainerclean" altogether, having a stray .c file in your build directory would not have an impact on the distributed tarball. > a second additional configure run just to bring the tree back into a > semi consistent state that will even allow you to run maintainer-clean > at all. > > If you want to clean the cruft out of your working directories before > starting the release process, you should do it before you change branches > and merge changes from upstream, using the Makefile that you already have > that is already in sync with the rest of your tree: > > make maintainer-clean > git checkout master > git pull origin master > ./bootstrap > ./configure Yes, running "make maintainer-clean" is a requirement. A patch with that improvement would be welcome.
Re: [PATCH] README-release improvements
Gary V. Vaughan wrote: ... >>> -* Run ./configure && make maintainer-clean >> >> Why do you want to remove the "make maintainer-clean"? >> I saw no justification, so maybe it's an accident. > > Hmm... half and half. Here's the relevant bit of ChangeLog: > > + Don't try to run ./configure right after git pull incase files > + that influence the bootstrap process have changed, move the > + ./configure step to after running ./bootstrap. That does not justify the removal of "make maintainer-clean". > As it stands (without this patch), README-release recommends: > > git checkout master > git pull > ./configure > make maintainer-clean > ./bootstrap > > Running the (potentially) outdated configure, to build a (potentially) > outdated Makefile, which may very well rerun aclocal, automake, autoconf > etc just to call the maintainer-clean rule, and then blow it all away > in the next step with a bootstrap seems odd and torturous to me. The above may be a little wasteful. But it is reliable and turn-key. If you know you've run "make maintainer-clean" recently, and you know that none of the tools that will regenerate key bits have changed (hard to know, though), then it's ok to skip. It's not worth the risk, to me. I don't make releases frequently enough to worry about a few minutes of CPU time, even if it's only for a minimal thing like this. I want a list of rules that I can follow with a minimum of thinking. Ideally it would be scripted, but we can save that for another day. > Worse, > the next time you try to run make after the final bootstrap step, some > or all of the autotools will have to run again to regenerate the Makefile. > > This patch reorders the steps thus: > > git checkout master > git pull origin master # don't try (perhaps fail) to pull other branches > ./bootstrap # rerun autotools explicitly on latest sources > ./configure # regenerate Makefile explicitly -"- -"- > > If you want to remove any file-droppings, you can use rm or git clean, or > make maintainer-clean as and when it fits the situation your working tree > is in. I didn't want to put an explicit git clean step in for fear of Using git clean here is not an option. And manually running "rm" is obviously not maintainable. Add a new gnulib module and you may easily pull in rules to generate files that only "make maintainer-clean" will remove. If you really dislike burning the extra CPU cycles, patch the procedure for your projects.
Re: [PATCH] README-release improvements
Gary V. Vaughan wrote:
> I made these changes in gnulib-local/top/README-release while making
> a start at leveraging the gnulib release machinery into GNU Libtool,
> but they seem generally applicable too.
Thanks for the suggestions.
However, it's not quite ready.
> Okay to push?
I'm glad you asked first.
Please do that for every iteration with any file I maintain.
In the future, please post unmodified "git format-patch" output,
precisely so that we can apply and test any such patch.
> +* If you are a @PACKAGE@ maintainer, but have not yet registered your
> + gpg public key and (preferred) email address with the FSF, send an
> + email, preferably GPG-signed, to that includes
> + the following:
> +
> +(a) name of package(s) that you are the maintainer for, and your
> +preferred email address.
> +
> +(b) an ASCII armored copy of your GnuPG key, as an attachment.
> + ("gpg --export -a YOUR_KEY_ID > mykey.asc" should give you
> + this.)
> +
> + When you have received acknowledgement of your message, the proper GPG
> + keys will be registered on ftp-upload.gnu.org and only then will you be
> + authorized to upload files to the FSF ftp machines.
All of the above will be useful exactly once per person/package
and then skipped forever after. Also, that same information is
already in the maintainer's guide. Let's not duplicate it.
If anything simply refer to that, e.g.,
https://www.gnu.org/prep/maintain/html_node/Automated-Upload-Registration.html#Automated-Upload-Registration
Though I am leery of adding even a one or two line note at
the very top, when it's something that will be useful so seldom.
> +* If you do not have access to the mailing list administrative interface,
> + approach the list owners for the password.
This sentence is in the same vein. Please omit it.
> Be sure to check the lists
> + (esp. bug-@PACKAGE@) for outstanding bug reports also in the list of
> + pending moderation requests.
IMHO, the above is so obvious that it's not worth including. If you're a
package maintainer and not watching mailing lists then you're not doing
your job. You should never have to hassle with moderation requests.
If you do, then you should follow the instructions here:
http://savannah.gnu.org/maintenance/ListHelperAntiSpam
...
> -* Run ./configure && make maintainer-clean
> -
Why do you want to remove the "make maintainer-clean"?
I saw no justification, so maybe it's an accident.
...
> -* Run bootstrap one last time. This downloads any new translations:
> +* Run "./bootstrap". This downloads any new translations.
Please leave two spaces after each sentence-ending ".".
...
> -build-aux/do-release-commit-and-tag X.Y stable
> +./build-aux/do-release-commit-and-tag X.Y stable
Please drop these "./"-adding changes or include a note in the log saying
which precisely shell motivates the change. There's certainly no "." in
my PATH and the leading "./" is not needed by bash, zsh, dash, ksh, ash,
csh or tcsh.
...
> * Send the announcement email message.
> + Address the email to `@PACKAGE@@gnu.org' and
> + `autotools-annou...@gnu.org' with the Reply-To header set to
> + `bug-@PACKAGE@@gnu.org'. Stable releases should also be announced on
> + `info-...@gnu.org'.
Please remove the above hunk. First, it should not mention autotools.
Though mainly, if the automatically-generated announcement does not include
the headers you want, customize them via cfg.mk, using these variables:
announcement_Cc_
announcement_mail_headers_
Re: a saner bootstrap script
Gary V. Vaughan wrote: ... > On the other hand, if by "incremental" you really mean chunking my rewrite > into > patches that add a function here and there, and disable bits of the old script > when they are no longer called, then I could be persuaded to do that. How > big should the chunks be to make reviewing them managable? Please just try to be patient. And reduce your multi-person on-list ping frequency. Your request is on my radar, and that's all you need.
Re: a saner bootstrap script
Gary V. Vaughan wrote: > On 19 Oct 2011, at 20:13, Jim Meyering wrote: >> Gary V. Vaughan wrote: >>>> Jim, please consider pulling it into coreutils master as a better fix than >>>> twidding Makefiles after the fact in bootstrap.conf. >>>> >>>>> This -Iintl option will disappear with gettext version 0.19 - because >>>>> then "gettextize --intl" will not work any more; it's already deprecated. >>>> >>>> And then, when coreutils moves to gettext-0.19, it's easy to remove the >>>> diff file at the same time. >> >> Your change got me past that bump. >> That's definitely a better way to work around the problem. >> >>> Ping. >> >> I haven't forgotten. > > Okay, thanks. > >> It's taking so long because this task has >> priority lower than almost everything else -- it's primarily >> an optimization, after all. > > Bummer. Being at the back of the queue is no fun, especially when all the > folks > that arrive after you get served first. Hi Gary, Your suggestion of unfair treatment is unjustified. Are you trying to demotivate me/us? No review request of a 1000-line rewrite -- or even a 100-line rewrite -- has taken precedence over yours. If you had proposed incremental changes, as requested, the review barrier would not be so high. > Maybe a return to plan A is better after all: Where I ask if anyone has a > reasonable objection to my committing the saner bootstrap to gnulib (which of Please try to adopt a more accommodating tone. Already you have refused the suggestion to present incremental changes. Now, when review of your monolith doesn't happen as quickly as you'd like, you propose to oust the preexisting, more-widely-used script. Doesn't that seem premature, or even impetuous to you? So far, gnulib development has been remarkably cordial and professional. Let's try to keep it that way. Jim
Re: [PATCH] posix_openpt: new module
I ran gnulib's top-level "make check", and cppi spotted this: >From ca3be51f7beb529726b066d1da34acd338828b24 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Wed, 19 Oct 2011 15:28:46 +0200 Subject: [PATCH] posix_openpt: remove spurious #endif * lib/posix_openpt.c (posix_openpt): Remove spurious #endif. --- ChangeLog |5 + lib/posix_openpt.c |2 -- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 478a7f2..fc42c53 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2011-10-19 Jim Meyering + + posix_openpt: remove spurious #endif + * lib/posix_openpt.c (posix_openpt): Remove spurious #endif. + 2011-10-19 Gary V. Vaughan maint.mk: Respect $(build_aux) in web-manual rule. diff --git a/lib/posix_openpt.c b/lib/posix_openpt.c index 3b7d4cf..19cd0b6 100644 --- a/lib/posix_openpt.c +++ b/lib/posix_openpt.c @@ -46,5 +46,3 @@ posix_openpt (int flags) return master; } - -#endif -- 1.7.7.30.g22b6ea.dirty
Re: [PATCH] Fix coreutils -Iintl vs gnulib gettext [WAS Re: a saner bootstrap script]
Gary V. Vaughan wrote: >> Jim, please consider pulling it into coreutils master as a better fix than >> twidding Makefiles after the fact in bootstrap.conf. >> >>> This -Iintl option will disappear with gettext version 0.19 - because >>> then "gettextize --intl" will not work any more; it's already deprecated. >> >> And then, when coreutils moves to gettext-0.19, it's easy to remove the >> diff file at the same time. Your change got me past that bump. That's definitely a better way to work around the problem. > Ping. I haven't forgotten. It's taking so long because this task has priority lower than almost everything else -- it's primarily an optimization, after all.
Re: a saner bootstrap script
Gary V. Vaughan wrote:
> Hi Pádraig, Jim,
>
> Is there anything else I can do to help you incorporate this, and the
> matching bootstrap.conf I wrote for you into coreutils now that the
> release is out?
Hi Gary,
Thanks for persevering. I have just tried it.
It looks like your coreutils working directory must have contained
an intl/ directory, but most people don't have that. Since I don't
have one, I get lots of these when compiling in lib/:
cc1: error: ../intl: No such file or directory [-Werror]
Merely creating the directory gets past that, and the build
did complete, but obviously I don't want to have to do that.
I used AM_GNU_GETTEXT([external]... after all.
Not surprisingly, the part of the current bootstrap script that
takes care of that transformation is absent from your version:
- if test $file = Makefile.am && test "X$gnulib_mk" != XMakefile.am; then
-copied=$copied${sep}$gnulib_mk; sep=$nl
-remove_intl='/^[^#].*\/intl/s/^/#/;'"s!$bt_regex/!!g"
-sed "$remove_intl" $1/$dir/$file |
-cmp - $dir/$gnulib_mk > /dev/null || {
- echo "$me: Copying $1/$dir/$file to $dir/$gnulib_mk ..." &&
- rm -f $dir/$gnulib_mk &&
- sed "$remove_intl" $1/$dir/$file >$dir/$gnulib_mk &&
- gnulib_mk_hook $dir/$gnulib_mk
-}
- elif { test "${2+set}" = set && test -r $2/$dir/$file; } ||
- version_controlled_file $dir $file; then
-echo "$me: $dir/$file overrides $1/$dir/$file"
- else
-copied=$copied$sep$file; sep=$nl
-cp_mark_as_generated $1/$dir/$file $dir/$file
- fi || exit
Please make at least a token effort to minimize differences in
bootstrap.conf. That makes it a lot easier to compare to the original.
Here is a new version with fewer gratuitous white space changes and
that avoids changing the copyright header.
==
# Bootstrap configuration.
# Copyright (C) 2006-2011 Free Software Foundation, Inc.
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
# You should have received a copy of the GNU General Public License
# along with this program. If not, see
Re: test-renameat does not cleanup after itself
Tom G. Christensen wrote: > Running make distcheck on CentOS 5 dies with: > ERROR: files left in build directory after distclean: > ./gltests/test-renameat.too > make[1]: *** [distcleancheck] Error 1 Thanks for the report. That's nearly identical to the one I fixed for linkat on Oct 1. Here's a fix: >From 775119f83d459107178933d4baa8886e91b19a9c Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Mon, 10 Oct 2011 08:49:37 +0200 Subject: [PATCH] test-renameat: don't leave behind a temporary file * tests/test-renameat.c (main): Don't forget to remove a temporary file. ERROR: files left in build directory after distclean: ./gltests/test-renameat.too make[1]: *** [distcleancheck] Error 1 Reported by Tom G. Christensen. --- ChangeLog |9 + tests/test-renameat.c |1 + 2 files changed, 10 insertions(+), 0 deletions(-) diff --git a/ChangeLog b/ChangeLog index 44f1e18..69ed191 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +2011-10-10 Jim Meyering + + test-renameat: don't leave behind a temporary file + * tests/test-renameat.c (main): Don't forget to remove a temporary file. + ERROR: files left in build directory after distclean: + ./gltests/test-renameat.too + make[1]: *** [distcleancheck] Error 1 + Reported by Tom G. Christensen. + 2011-10-09 Bruno Haible rint: Determine RINT_LIBM correctly on AIX 7. diff --git a/tests/test-renameat.c b/tests/test-renameat.c index 9ca1787..9b67da0 100644 --- a/tests/test-renameat.c +++ b/tests/test-renameat.c @@ -83,6 +83,7 @@ main (void) ASSERT (renameat (AT_FDCWD, BASE "oo", 99, "bar") == -1); ASSERT (errno == EBADF); } + ASSERT (unlink (BASE "oo") == 0); /* Test basic rename functionality, using current directory. */ result = test_rename (do_rename, false); -- 1.7.7.rc0.362.g5a14
Re: new snapshot available: coreutils-8.13.29-43a9
Jim Meyering wrote: > Bruno Haible wrote: >> On MacOS X 10.5, the following gnulib tests fail to link: > ... >> This is apparently due to the recent implementation change of getcwd(). >> This patch should fix it: >> >> 2011-10-08 Bruno Haible >> >> Tests: Avoid link failures w.r.t. libintl. >> * modules/faccessat-tests (Makefile.am): Link test-faccessat against >> $(LIBINTL). >> * modules/fchdir-tests (Makefile.am): Link test-fchdir against >> $(LIBINTL). >> * modules/getcwd-lgpl-tests (Makefile.am): Link test-getcwd-lgpl >> against $(LIBINTL). >> * modules/getcwd-tests (Makefile.am): Link test-getcwd against >> $(LIBINTL). >> * modules/openat-tests (Makefile.am): Link test-fchmodat against >> $(LIBINTL). >> * modules/stat-tests (Makefile.am): Link test-stat against $(LIBINTL). > > Thanks for the speedy testing and fix! This should fix it: >From 3fb0f4aaf2d858dc668dd786286c47e5967eed1b Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Sun, 9 Oct 2011 18:51:53 +0200 Subject: [PATCH] build: update gnulib to latest to fix MacOS X 10.5 test link failure Details here: http://thread.gmane.org/gmane.comp.gnu.coreutils.general/1726/focus=1743 --- gnulib |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/gnulib b/gnulib index d813b68..6a4c64c 16 --- a/gnulib +++ b/gnulib @@ -1 +1 @@ -Subproject commit d813b688732c3a0da947f91cbb19cb78a627209e +Subproject commit 6a4c64ce4a59bd9589e63fb5ee480765d356f8c7 -- 1.7.7.rc0.362.g5a14
Re: new snapshot available: coreutils-8.13.29-43a9
Bruno Haible wrote: > On MacOS X 10.5, the following gnulib tests fail to link: ... > This is apparently due to the recent implementation change of getcwd(). > This patch should fix it: > > 2011-10-08 Bruno Haible > > Tests: Avoid link failures w.r.t. libintl. > * modules/faccessat-tests (Makefile.am): Link test-faccessat against > $(LIBINTL). > * modules/fchdir-tests (Makefile.am): Link test-fchdir against > $(LIBINTL). > * modules/getcwd-lgpl-tests (Makefile.am): Link test-getcwd-lgpl > against $(LIBINTL). > * modules/getcwd-tests (Makefile.am): Link test-getcwd against > $(LIBINTL). > * modules/openat-tests (Makefile.am): Link test-fchmodat against > $(LIBINTL). > * modules/stat-tests (Makefile.am): Link test-stat against $(LIBINTL). Thanks for the speedy testing and fix!
Re: [PATCH] file-has-acl: revert unintended change in behavior of ls -L
Bruno Haible wrote: > Jim Meyering wrote: >> I propose to push Kamil's fix (mainly to have a record of it, >> in case we need it later), but then to immediately revert it >> along with the file-has-acl.c change that started this. >> That seems to be the right thing to do, going forward, >> since the kernel folks have recently reverted the behavior >> that motivated the earlier file-has-acl.c patch. >> Any objections? > > Fine with me. > >>(see https://bugzilla.redhat.com/show_bug.cgi?id=720325#c24) > > Good to see that the kernel people are considering the issue seriously. > Thanks, Kamil, for having pulled Ian Kent into the discussion. Thanks for the feedback, Bruno. I've pushed that to gnulib.
Re: [PATCH] file-has-acl: revert unintended change in behavior of ls -L
race condition). */
+return acl_extended_file_nofollow (name);
+}
+
+ /* fallback for symlinks and old versions of libacl */
+ return acl_extended_file (name);
+}
+
+
/* Return 1 if NAME has a nontrivial access control list, 0 if NAME
only has no or a base access control list, and -1 (setting errno)
on error. SB must be set to the stat buffer of NAME, obtained
@@ -454,20 +482,12 @@ file_has_acl (char const *name, struct stat const *sb)
/* Linux, FreeBSD, MacOS X, IRIX, Tru64 */
int ret;
- if (HAVE_ACL_EXTENDED_FILE || HAVE_ACL_EXTENDED_FILE_NOFOLLOW) /* Linux
*/
+ if (HAVE_ACL_EXTENDED_FILE) /* Linux */
{
-# if HAVE_ACL_EXTENDED_FILE_NOFOLLOW
- /* acl_extended_file_nofollow() uses lgetxattr() in order to prevent
- unnecessary mounts, but it returns the same result as we already
- know that NAME is not a symbolic link at this point (modulo the
- TOCTTOU race condition). */
- ret = acl_extended_file_nofollow (name);
-# else
/* On Linux, acl_extended_file is an optimized function: It only
makes two calls to getxattr(), one for ACL_TYPE_ACCESS, one for
ACL_TYPE_DEFAULT. */
- ret = acl_extended_file (name);
-# endif
+ ret = acl_extended_file_wrap (name);
}
else /* FreeBSD, MacOS X, IRIX, Tru64 */
{
--
1.7.7.rc0.362.g5a14
>From d813b688732c3a0da947f91cbb19cb78a627209e Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Wed, 5 Oct 2011 15:06:49 +0200
Subject: [PATCH 2/2] file-has-acl: revert both recent changes, 80af92af and
95f7c57f
* lib/file-has-acl.c: While the 2011-10-03 change does fix the
ls -lL regression introduced in coreutils-8.12, it does so at the
cost of an additional stat call in the common case. Besides, now
that the kernel change that prompted commit 95f7c57f has been reverted
(see https://bugzilla.redhat.com/show_bug.cgi?id=720325#c24)
we have no use for commit 95f7c57f, "file-has-acl: use
acl_extended_file_nofollow if available".
---
ChangeLog | 11 +++
lib/acl-internal.h |6 --
lib/file-has-acl.c | 30 +-
m4/acl.m4 |2 +-
4 files changed, 13 insertions(+), 36 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 6715d96..8d66d56 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2011-10-05 Jim Meyering
+
+ file-has-acl: revert both recent changes, 80af92af and 95f7c57f
+ * lib/file-has-acl.c: While the 2011-10-03 change does fix the
+ ls -lL regression introduced in coreutils-8.12, it does so at the
+ cost of an additional stat call in the common case. Besides, now
+ that the kernel change that prompted commit 95f7c57f has been reverted
+ (see https://bugzilla.redhat.com/show_bug.cgi?id=720325#c24)
+ we have no use for commit 95f7c57f, "file-has-acl: use
+ acl_extended_file_nofollow if available".
+
2011-10-03 Kamil Dudka
file-has-acl: revert unintended change in behavior of ls -L
diff --git a/lib/acl-internal.h b/lib/acl-internal.h
index 4f7166e..7a105c8 100644
--- a/lib/acl-internal.h
+++ b/lib/acl-internal.h
@@ -133,12 +133,6 @@ rpl_acl_set_fd (int fd, acl_t acl)
# endif
/* Linux-specific */
-# ifndef HAVE_ACL_EXTENDED_FILE_NOFOLLOW
-# define HAVE_ACL_EXTENDED_FILE_NOFOLLOW false
-# define acl_extended_file_nofollow(name) (-1)
-# endif
-
-/* Linux-specific */
# ifndef HAVE_ACL_FROM_MODE
# define HAVE_ACL_FROM_MODE false
# define acl_from_mode(mode) (NULL)
diff --git a/lib/file-has-acl.c b/lib/file-has-acl.c
index 532cafc..4ff2808 100644
--- a/lib/file-has-acl.c
+++ b/lib/file-has-acl.c
@@ -437,34 +437,6 @@ acl_nontrivial (int count, struct acl *entries)
#endif
-/* acl_extended_file() tests whether a file has an ACL. But it can trigger
- unnecessary autofs mounts. In newer versions of libacl, a function
- acl_extended_file_nofollow() is available that uses lgetxattr() and
- therefore does not have this problem. It is equivalent to
- acl_extended_file(), except on symbolic links. */
-
-static int
-acl_extended_file_wrap (char const *name)
-{
- if ( ! HAVE_ACL_EXTENDED_FILE)
-return -1;
-
- if (HAVE_ACL_EXTENDED_FILE_NOFOLLOW)
-{
- struct stat sb;
- if (! lstat (name, &sb) && ! S_ISLNK (sb.st_mode))
-/* acl_extended_file_nofollow() uses lgetxattr() in order to
- prevent unnecessary mounts. It returns the same result as
- acl_extended_file() since we already know that NAME is not a
- symbolic link at this point (modulo the TOCTTOU race condition). */
-return acl_extended_file_nofollow (name);
-}
-
- /* fallback for symlinks and old versions of libacl */
- return acl_extended_file (name);
-}
-
-
/* Return 1 if NAME has a nontrivial access control list, 0 if NAME
only has no or a base
Re: a saner bootstrap script
Gary V. Vaughan wrote: > Hi Jim, > > On 4 Oct 2011, at 17:09, Jim Meyering wrote: >> Gary V. Vaughan wrote: >>> Sorry I didn't notice your reply sooner. >>> >>> On 22 Sep 2011, at 23:38, Jim Meyering wrote: >>>> Gary V. Vaughan wrote: >>>>>>> * g...@github.com:gvvaughan/GNU-coreutils.git in gary/bootstrap >>>>>>> https://github.com/gvvaughan/GNU-coreutils/commits/gary/bootstrap >>>> >>>> I'll try to find time for this next week. >>> >>> Thanks. How did you get on? >> >> Hi Gary, >> >> I started looking, but found that your bootstrap.conf is >> months old, and thus not usable with the current sources. > > Indeed. Although the idea was to prove that my bootstrap works well by > writing a series of bootstrap.conf for a selection of GNU projects with > different bootstrap requirements... and much less to provide bootstrap.conf's > for the projects I chose for that demonstration. And I wasn't really expecting > to wait 14 months for it to be adopted into gnulib, and didn't anticipate the > ongoing development in those projects would make my demo bootstrap.confs > obsolescent :( > >> Do you feel like rebasing it? > > Sure, although it will take me a few days to find the time - is that because > you're planning to adopt my bootstrap script into coreutils master > unilaterally? > If not then you can easily checkout my coreutils snapshot from github to > verify > that it is perfectly sane. > > Really, I'm trying to help gnulib to adopt the script first and foremost... > although the last few Zile releases have been using it, and I'll be making > a Libtool release in the next week or so that uses it irrespective of gnulib > adoption. And I expect I'll also put an M4 alpha out with that script too > before long. So, if you want to unilaterally take the script into coreutils > too, then I'll be very happy to provide any assistance I can with updating > the coreutils bootstrap.conf I wrote all those months ago. I'm trying it solely because you've invested so much in it and asked so many times. I'm certainly not chomping at the bit for a new version. However, I confess that I was disappointed by your rejection of some of the style-related suggestions made by Stefano, and have to say that if I do use it, the copy I use in coreutils, grep, gzip, patch, parted, etc. will inherit most, if not all, of his suggestions. >>> I just tried to check that everything in my coreutils bootstrap.conf still >>> works correctly with coreutils master, but unfortunately coreutils bootstrap >>> now requires that I install the latest autotools -- including >>> gettext-0.18.1.1 >>> which doesn't compile on Mac OS 10.7.1 with the latest Apple supplied gcc >>> (4.2.1 LLVM). And without that, I can no longer bootstrap coreutils on my >>> Mac :-( >> >> If it's a gettext problem, report it and it should be fixed very quickly. >> Otherwise, just adjust the AM_GNU_GETTEXT_VERSION([0.18.1]) line >> in configure.ac to accommodate whatever version of gettext you have. >> You should be ok if it's 0.17.x. > > If an older version of gettext is sufficient, then can you please require > that version instead? Gettext is a large complex package that is quite a Using an older version is sufficient to ensure that your script works with coreutils. I don't see why everyone should accept an older version just because a build-glitch affects one type of system. The work-around is trivial. > challenge to compile on some of the architectures we support at times. > > At least as far as Mac OS 10.7 is concerned, I tracked the problem down to > a bug in the gnulib non-release that was used to bootstrap gettext-0.18.1.1, > which has since been fixed in gnulib. Again, did you report it? I guess Bruno may read this, but still. It's been over 100 commits since 0.18.1.1. Maybe the report of a build problem like that will encourage him to make a new release. > Rather than trying to rebootstrap > gettext to pick up the fix, and have to worry about other problems that > might bring, I hand-applied the patch to gettext configure, and was able to > build a new enough gettext. > > I haven't had time yet to pick up the coreutils bootstrap.conf update, but > I'll probably be able to get to it by the end of the week. If you're in a > hurry, then I think you might find writing your own updated bootstrap.conf > would be instructive in the vast improvements I think the new bootstrap I've > written brings to the table - and maybe help build enough confidence in it > that you'd like to help me adopt it into upstream gnulib? If I go with it, it will eventually gain at least 10 new client packages. However, I don't have a lot of time to invest in the transition, so anything you can do to make it easier may go a long way.
Re: a saner bootstrap script
Gary V. Vaughan wrote: > Sorry I didn't notice your reply sooner. > > On 22 Sep 2011, at 23:38, Jim Meyering wrote: >> Gary V. Vaughan wrote: >>>>> * g...@github.com:gvvaughan/GNU-coreutils.git in gary/bootstrap >>>>> https://github.com/gvvaughan/GNU-coreutils/commits/gary/bootstrap >> >> I'll try to find time for this next week. > > Thanks. How did you get on? Hi Gary, I started looking, but found that your bootstrap.conf is months old, and thus not usable with the current sources. Do you feel like rebasing it? > I just tried to check that everything in my coreutils bootstrap.conf still > works correctly with coreutils master, but unfortunately coreutils bootstrap > now requires that I install the latest autotools -- including gettext-0.18.1.1 > which doesn't compile on Mac OS 10.7.1 with the latest Apple supplied gcc > (4.2.1 LLVM). And without that, I can no longer bootstrap coreutils on my > Mac :-( If it's a gettext problem, report it and it should be fixed very quickly. Otherwise, just adjust the AM_GNU_GETTEXT_VERSION([0.18.1]) line in configure.ac to accommodate whatever version of gettext you have. You should be ok if it's 0.17.x. >> I glanced through the first few lines of your script and found >> >> FGREP=fgrep >> EGREP=egrep >> ... >> >> Please use the more standard grep -F and grep -E instead. > > I wasn't aware that it was more portable to expect a vendor grep that egrep and fgrep have been deprecated for years. POSIX stopped requiring them back in 1003.1-2001. Since they have not been required for POSIX-conformance for 10 years... > supports -E and -F than to expect a vendor egrep and fgrep binary. None- > the-less, I applied your suggested change to the master copy of bootstrap > in Zile.
Re: [PATCH] file-has-acl: revert unintended change in behavior of ls -L
Kamil Dudka wrote:
> On Mon October 3 2011 15:02:20 Bruno Haible wrote:
>> The function name should explain the semantics of the function. The fact
>> that it's a wrapper around acl_extended_file is something one can see by
>> reading the code.
>>
>> Maybe call it acl_extended_file_optimized?
>
> Sounds good.
>
>> > +/* acl_extended_file_nofollow() uses lgetxattr() in order to prevent
>> > + unnecessary mounts, but it returns the same result as we already
>> > + know that NAME is not a symbolic link at this point (modulo the
>> > + TOCTTOU race condition). */
>>
>> I have a hard time understanding this comment.
>> - Why the "but"? The "same result" as what?
>
> Fixed. I am not a native speaker.
>
>> - What is the "TOCTTOU race condition"?
>
> TOC = checking whether the file is a symlink
> TOU = checking whether the file has ACLs
>
>> If it's important, please add a FIXME and an explanation.
>
> No, it does not feel important to me, at least not for ls.
>
>> How about this comment, right after the inner #if: 1. Describe the problem.
>> 2. Describe the solution.
>>
>> /* acl_extended_file() tests whether a file has an ACL. But it can
>> trigger unnecessary autofs mounts. In newer versions of libacl, a
>> function acl_extended_file_nofollow() is available that uses lgetxattr()
>> and therefore does not have this problem. It is equivalent to
>> acl_extended_file(), except on symbolic links. */
>
> Comment replaced. Thanks for the suggestion.
Kamil,
I've merged your two comment changes, kept my commit-log and ChangeLog
and rebased past Bruno's latest change. This is what I expect to push:
>From 603b1e3b16894cac183952b0b0fe379749a3aebe Mon Sep 17 00:00:00 2001
From: Kamil Dudka
Date: Mon, 3 Oct 2011 12:17:22 +0200
Subject: [PATCH] file-has-acl: revert unintended change in behavior of ls -L
* lib/file-has-acl.c (acl_extended_file_wrap): New function,
derived from...
(file_has_acl): ...code here. Call it.
This problem was introduced with 2011-07-22 commit 95f7c57f,
"file-has-acl: use acl_extended_file_nofollow if available".
See http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/28538
---
ChangeLog | 10 ++
lib/file-has-acl.c | 40 ++--
2 files changed, 40 insertions(+), 10 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 7f9e391..4965785 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2011-10-03 Kamil Dudka
+
+ file-has-acl: revert unintended change in behavior of ls -L
+ * lib/file-has-acl.c (acl_extended_file_wrap): New function,
+ derived from...
+ (file_has_acl): ...code here. Call it.
+ This problem was introduced with 2011-07-22 commit 95f7c57f,
+ "file-has-acl: use acl_extended_file_nofollow if available".
+ See http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/28538
+
2011-10-03 Bruno Haible
nonblocking tests: Fix test failure on OpenBSD/SPARC64.
diff --git a/lib/file-has-acl.c b/lib/file-has-acl.c
index 9a5e249..532cafc 100644
--- a/lib/file-has-acl.c
+++ b/lib/file-has-acl.c
@@ -437,6 +437,34 @@ acl_nontrivial (int count, struct acl *entries)
#endif
+/* acl_extended_file() tests whether a file has an ACL. But it can trigger
+ unnecessary autofs mounts. In newer versions of libacl, a function
+ acl_extended_file_nofollow() is available that uses lgetxattr() and
+ therefore does not have this problem. It is equivalent to
+ acl_extended_file(), except on symbolic links. */
+
+static int
+acl_extended_file_wrap (char const *name)
+{
+ if ( ! HAVE_ACL_EXTENDED_FILE)
+return -1;
+
+ if (HAVE_ACL_EXTENDED_FILE_NOFOLLOW)
+{
+ struct stat sb;
+ if (! lstat (name, &sb) && ! S_ISLNK (sb.st_mode))
+/* acl_extended_file_nofollow() uses lgetxattr() in order to
+ prevent unnecessary mounts. It returns the same result as
+ acl_extended_file() since we already know that NAME is not a
+ symbolic link at this point (modulo the TOCTTOU race condition). */
+return acl_extended_file_nofollow (name);
+}
+
+ /* fallback for symlinks and old versions of libacl */
+ return acl_extended_file (name);
+}
+
+
/* Return 1 if NAME has a nontrivial access control list, 0 if NAME
only has no or a base access control list, and -1 (setting errno)
on error. SB must be set to the stat buffer of NAME, obtained
@@ -454,20 +482,12 @@ file_has_acl (char const *name, struct stat const *sb)
/* Linux, FreeBSD, MacOS X, IRIX, Tru64 */
int ret;
- if (HAVE_ACL_EXTENDED_FILE || HAVE_ACL_EXTENDED_FILE_NOFOLLOW) /* Linux
*/
+ if (HAVE_ACL_EXTENDED_FILE) /* Linux */
{
-# if HAVE_ACL_EXTENDED_FILE_NOFOLLOW
- /* acl_extended_file_nofollow() uses lgetxattr() in order to prevent
- unnecessary mounts, but it returns the same result as we already
- know that NAME is not a symbolic link at this point (modulo the
-
Re: [PATCH] file-has-acl: revert unintended change in behavior of ls -L
Kamil Dudka wrote:
> The commit 95f7c57 introduced an unintended change in behavior of ls -L.
> I am attaching a patch that restores the old behavior. Thanks in advance
> for considering the patch!
Thanks again, Kamil,
Here's a version of your patch that I find more readable:
- prefer if (CONDITION) over #if CONDITION, when possible
- document the code more in comments, less in the commit log
Is this ok with you?
>From 628f1d14424c8356d5b1cfce690bbe544c048177 Mon Sep 17 00:00:00 2001
From: Kamil Dudka
Date: Mon, 3 Oct 2011 12:17:22 +0200
Subject: [PATCH] file-has-acl: revert unintended change in behavior of ls -L
* lib/file-has-acl.c (acl_extended_file_wrap): New function,
derived from...
(file_has_acl): ...code here. Call it.
This problem was introduced with 2011-07-22 commit 95f7c57f,
"file-has-acl: use acl_extended_file_nofollow if available".
See http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/28538
---
ChangeLog | 10 ++
lib/file-has-acl.c | 38 --
2 files changed, 38 insertions(+), 10 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index a6d363a..140f5e1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2011-10-03 Kamil Dudka
+
+ file-has-acl: revert unintended change in behavior of ls -L
+ * lib/file-has-acl.c (acl_extended_file_wrap): New function,
+ derived from...
+ (file_has_acl): ...code here. Call it.
+ This problem was introduced with 2011-07-22 commit 95f7c57f,
+ "file-has-acl: use acl_extended_file_nofollow if available".
+ See http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/28538
+
2011-10-01 Jim Meyering
maint.mk: adjust a release-related rule not to require use of gzip
diff --git a/lib/file-has-acl.c b/lib/file-has-acl.c
index 1b7e392..db733ac 100644
--- a/lib/file-has-acl.c
+++ b/lib/file-has-acl.c
@@ -437,6 +437,32 @@ acl_nontrivial (int count, struct acl *entries)
#endif
+/* Call acl_extended_file_nofollow when it is available and when
+ NAME is a symlink. Otherwise, merely call acl_extended_file.
+ When acl_extended_file is not available, always return -1. */
+
+static int
+acl_extended_file_wrap (char const *name)
+{
+ if ( ! HAVE_ACL_EXTENDED_FILE)
+return -1;
+
+ if (HAVE_ACL_EXTENDED_FILE_NOFOLLOW)
+{
+ struct stat sb;
+ if (! lstat (name, &sb) && ! S_ISLNK (sb.st_mode))
+/* acl_extended_file_nofollow() uses lgetxattr() in order to prevent
+ unnecessary mounts, but it returns the same result as we already
+ know that NAME is not a symbolic link at this point (modulo the
+ TOCTTOU race condition). */
+return acl_extended_file_nofollow (name);
+}
+
+ /* fallback for symlinks and old versions of libacl */
+ return acl_extended_file (name);
+}
+
+
/* Return 1 if NAME has a nontrivial access control list, 0 if NAME
only has no or a base access control list, and -1 (setting errno)
on error. SB must be set to the stat buffer of FILE. */
@@ -453,20 +479,12 @@ file_has_acl (char const *name, struct stat const *sb)
/* Linux, FreeBSD, MacOS X, IRIX, Tru64 */
int ret;
- if (HAVE_ACL_EXTENDED_FILE || HAVE_ACL_EXTENDED_FILE_NOFOLLOW) /* Linux
*/
+ if (HAVE_ACL_EXTENDED_FILE) /* Linux */
{
-# if HAVE_ACL_EXTENDED_FILE_NOFOLLOW
- /* acl_extended_file_nofollow() uses lgetxattr() in order to prevent
- unnecessary mounts, but it returns the same result as we already
- know that NAME is not a symbolic link at this point (modulo the
- TOCTTOU race condition). */
- ret = acl_extended_file_nofollow (name);
-# else
/* On Linux, acl_extended_file is an optimized function: It only
makes two calls to getxattr(), one for ACL_TYPE_ACCESS, one for
ACL_TYPE_DEFAULT. */
- ret = acl_extended_file (name);
-# endif
+ ret = acl_extended_file_wrap (name);
}
else /* FreeBSD, MacOS X, IRIX, Tru64 */
{
--
1.7.7.rc0.362.g5a14
Re: [PATCH] file-has-acl: revert unintended change in behavior of ls -L
Kamil Dudka wrote:
> On Mon October 3 2011 13:09:21 Jim Meyering wrote:
>> Kamil Dudka wrote:
>> > On Mon October 3 2011 12:45:01 Jim Meyering wrote:
>> >> Can you describe how to make "ls -L" misbehave without this patch?
>> >
>> > if you have a symlink to a file with ACL, 'ls -Ll' does not print the '+'
>> > at end of the column with permission bits.
>>
>> Thanks. I expect to add something like this:
>>
>> $ touch k; setfacl -m user:${USER}:r k; ln -s k s; env ls -Log s
>> -rw-r-. 1 0 Oct 3 13:07 s
>>
>> That "." is wrong. It should be "+".
>
> I am having problems finding a good way to detect that ls is capable
> of detecting ACLs. AFAIK there is no such macro in config.h. The
> preprocessor games in lib/file-has-acl.c are overly complicated.
>
> What about the following scenario?
>
> 1. try ls -l directly on a regular file with ACL, check if it prints '+'.
> 2. if it succeeds, try the same on a regular file and a symbolic link
> with/without -L
Good point. This should do it:
touch k; setfacl -m user:${USER}:r k; ln -s k s
set _ $(ls -Log s); shift; link=$1
set _ $(ls -og k); shift; reg=$1
test "$link" = "$reg" || fail=1
>From 6b9751f0ffe276bc4fb9d30e38cc59ad92e7ca0e Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Mon, 3 Oct 2011 13:49:47 +0200
Subject: [PATCH] tests: add a test to exercise today's ls-lL-vs-ACL bug
* tests/ls/slink-acl: New file.
* tests/Makefile.am (TESTS): Add it.
* tests/init.cfg (require_setfacl_): New function.
See http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/28538
---
tests/Makefile.am |1 +
tests/init.cfg |6 ++
tests/ls/slink-acl | 33 +
3 files changed, 40 insertions(+), 0 deletions(-)
create mode 100755 tests/ls/slink-acl
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 2cf409a..9c9a1b8 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -431,6 +431,7 @@ TESTS = \
ls/readdir-mountpoint-inode \
ls/recursive \
ls/rt-1 \
+ ls/slink-acl \
ls/stat-dtype\
ls/stat-failed \
ls/stat-free-color \
diff --git a/tests/init.cfg b/tests/init.cfg
index f6eb651..04abe4f 100644
--- a/tests/init.cfg
+++ b/tests/init.cfg
@@ -130,6 +130,12 @@ require_strace_()
skip_ 'strace -qe "'"$1"'" does not work'
}
+require_setfacl_()
+{
+ setfacl -m user::rwx . \
+|| skip_ "setfacl does not work on the current file system"
+}
+
# Require a controlling input `terminal'.
require_controlling_input_terminal_()
{
diff --git a/tests/ls/slink-acl b/tests/ls/slink-acl
new file mode 100755
index 000..8594642
--- /dev/null
+++ b/tests/ls/slink-acl
@@ -0,0 +1,33 @@
+#!/bin/sh
+# verify that ls -lL works when applied to a symlink to an ACL'd file
+
+# Copyright (C) 2011 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../src
+print_ver_ ls
+
+require_setacl_
+
+touch k || framework_failure_
+setfacl -m user:${USER}:r k || framework_failure_
+ln -s k s || framework_failure_
+
+set _ $(ls -Log s); shift; link=$1
+set _ $(ls -og k); shift; reg=$1
+
+test "$link" = "$reg" || fail=1
+
+Exit $fail
--
1.7.7.rc0.362.g5a14
Re: [PATCH] file-has-acl: revert unintended change in behavior of ls -L
Kamil Dudka wrote:
> On Mon October 3 2011 12:45:01 Jim Meyering wrote:
>> Can you describe how to make "ls -L" misbehave without this patch?
>
> if you have a symlink to a file with ACL, 'ls -Ll' does not print the '+'
> at end of the column with permission bits.
Thanks. I expect to add something like this:
$ touch k; setfacl -m user:${USER}:r k; ln -s k s; env ls -Log s
-rw-r-. 1 0 Oct 3 13:07 s
That "." is wrong. It should be "+".
>> I.e., if there isn't already a test in coreutils to exercise this,
>> I'd like to add one.
>
> Ok, will have a look if there is a similar test exercising ACLs in coreutils.
>
>> Also, if you can reference a bugzilla number, that would be nice.
>> Looks like it's this one:
>>
>> https://bugzilla.redhat.com/720325
>
> This is the bugzilla mentioned in the original commit, which introduced the
> bug. There is no separate bugzilla ID for the change in behavior that this
> patch reverts.
Re: [PATCH] file-has-acl: revert unintended change in behavior of ls -L
Kamil Dudka wrote: > The commit 95f7c57 introduced an unintended change in behavior of ls -L. > I am attaching a patch that restores the old behavior. Thanks in advance > for considering the patch! > > Kamil > > From 75836c03cb21d616591b11164b626556d9f26152 Mon Sep 17 00:00:00 2001 > From: Kamil Dudka > Date: Mon, 3 Oct 2011 12:17:22 +0200 > Subject: [PATCH] file-has-acl: revert unintended change in behavior of ls -L > > * lib/file-has-acl.c (acl_extended_file_wrap): A wrapper around > acl_extended_file () that allows to call acl_extended_file_nofollow () > only if the function is available and the file is not a symbolic link. > (file_has_acl): Remove code that caused problems. Call > acl_extended_file_wrap (). Hi Kamil, Thank you for the patch. Can you describe how to make "ls -L" misbehave without this patch? I.e., if there isn't already a test in coreutils to exercise this, I'd like to add one. Also, if you can reference a bugzilla number, that would be nice. Looks like it's this one: https://bugzilla.redhat.com/720325
[PATCH] maint.mk: adjust a release-related rule not to require use of gzip
In preparing to stop building gzip-compressed tarballs in a few
projects I maintain, this change comes in handy:
>From 244794a7887f13d9cdb91fed96932cc479905b96 Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Sat, 1 Oct 2011 17:33:42 +0200
Subject: [PATCH] maint.mk: adjust a release-related rule not to require use
of gzip
* top/maint.mk (writable-files): Don't hard-code use of .tar.gz.
Instead, check each file in $(DIST_ARCHIVES). This is better for
projects that build only .tar.xz files. Also fix an erroneous test.
---
ChangeLog|5 +
top/maint.mk | 18 +++---
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 7ad4703..a6d363a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
2011-10-01 Jim Meyering
+ maint.mk: adjust a release-related rule not to require use of gzip
+ * top/maint.mk (writable-files): Don't hard-code use of .tar.gz.
+ Instead, check each file in $(DIST_ARCHIVES). This is better for
+ projects that build only .tar.xz files. Also fix an erroneous test.
+
test-linkat: don't leave behind a temporary file
* tests/test-linkat.c (main): Don't forget to remove a temporary file.
Otherwise, coreutils' "make distcheck" would fail with this:
diff --git a/top/maint.mk b/top/maint.mk
index 0137df1..51f617b 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -1079,16 +1079,20 @@ sc_makefile_path_separator_check:
halt=$(msg) \
$(_sc_search_regexp)
-# Check that `make alpha' will not fail at the end of the process.
+# Check that `make alpha' will not fail at the end of the process,
+# i.e., when pkg-M.N.tar.xz already exists (either in "." or in ../release)
+# and is read-only.
writable-files:
- if test -d $(release_archive_dir); then :; else \
- for file in $(distdir).tar.gz \
- $(release_archive_dir)/$(distdir).tar.gz; do \
- test -e $$file || continue; \
- test -w $$file \
- || { echo ERROR: $$file is not writable; fail=1; }; \
+ if test -d $(release_archive_dir); then \
+ for file in $(DIST_ARCHIVES); do \
+ for p in ./ $(release_archive_dir)/; do \
+ test -e $$p$$file || continue;\
+ test -w $$p$$file \
+ || { echo ERROR: $$p$$file is not writable; fail=1; }; \
+ done; \
done; \
test "$$fail" && exit 1 || : ;\
+ else :; \
fi
v_etc_file = $(gnulib_dir)/lib/version-etc.c
--
1.7.7.rc0.362.g5a14
[PATCH] test-linkat: don't leave behind a temporary file
FYI, >From 24fae8f857a1ab285dc936e697f1e10138e9 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Sat, 1 Oct 2011 15:06:43 +0200 Subject: [PATCH] test-linkat: don't leave behind a temporary file * tests/test-linkat.c (main): Don't forget to remove a temporary file. Otherwise, coreutils' "make distcheck" would fail with this: Only in /c/cu/tests/torture/coreutils/test/\ coreutils-8.13.22-d5caf.old/gnulib-tests: test-linkat.too make[2]: *** [my-distcheck] Error 1 --- ChangeLog |7 +++ tests/test-linkat.c |1 + 2 files changed, 8 insertions(+), 0 deletions(-) diff --git a/ChangeLog b/ChangeLog index 779e00d..7ad4703 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,12 @@ 2011-10-01 Jim Meyering + test-linkat: don't leave behind a temporary file + * tests/test-linkat.c (main): Don't forget to remove a temporary file. + Otherwise, coreutils' "make distcheck" would fail with this: + Only in /c/cu/tests/torture/coreutils/test/\ + coreutils-8.13.22-d5caf.old/gnulib-tests: test-linkat.too + make[2]: *** [my-distcheck] Error 1 + float, math: add omitted file * lib/itold.c: Add file, required for yesterday's float change. diff --git a/tests/test-linkat.c b/tests/test-linkat.c index 40b8371..6da82b0 100644 --- a/tests/test-linkat.c +++ b/tests/test-linkat.c @@ -117,6 +117,7 @@ main (void) ASSERT (linkat (AT_FDCWD, BASE "oo", 99, "bar", 0) == -1); ASSERT (errno == EBADF); } + ASSERT (unlink (BASE "oo") == 0); /* Test basic link functionality, without mentioning symlinks. */ result = test_link (do_link, true); -- 1.7.7.rc0.362.g5a14
Re: float, math: Fix 'int' to 'long double' conversion on Linux/SPARC64
Bruno Haible wrote: > Jim Meyering wrote: >> I've taken the liberty of adding the above file. > > Oops, I forgot a "git add" command. Thanks for the fix. If you always write ChangeLog entries before committing, using "vc-dwim --commit" to perform the commit would prevent precisely this problem (among others). I.e., it would have detected the inconsistency (file listed in ChangeLog, yet not "git add"ed) and would have warned you.
Re: float, math: Fix 'int' to 'long double' conversion on Linux/SPARC64
Bruno Haible wrote:
> It started as a test failure on Linux/SPARC64:
>
> test-logl.c:42: assertion failed
> FAIL: test-logl
>
> I tried to fix it by activating the gnulib replacement code for logl(). But
> the bug persisted.
>
> Debugging it in detail, it turned out to be a bug in the 'int' to 'long
> double'
> conversion. The glibc provided function, that is invoked by the GCC generated
> code, actually performs an 'unsigned int' to 'long double' conversion.
>
> I've reported it at
> <http://sources.redhat.com/bugzilla/show_bug.cgi?id=13240>.
> Here's the workaround for gnulib.
>
>
> 2011-09-30 Bruno Haible
>
> float, math: Fix 'int' to 'long double' conversion on Linux/SPARC64.
> * m4/float_h.m4 (gl_FLOAT_H): Test conversion from 'int' to
> 'long double'. Set REPLACE_ITOLD.
> * lib/float.in.h (_Qp_itoq, _gl_float_fix_itold): New declarations.
> * lib/math.in.h (_Qp_itoq, _gl_math_fix_itold): New declarations.
> * lib/itold.c: New file.
> * modules/float (Files): Add lib/itold.c.
> (configure.ac): When REPLACE_ITOLD is 1, arrange to compile itold.c.
> (Makefile.am): Substitute REPLACE_ITOLD.
> * modules/math (Depends-on): Add float.
> (Makefile.am): Substitute REPLACE_ITOLD.
> * doc/posix-headers/float.texi: Mention problem on Linux/SPARC64.
> * doc/posix-headers/math.texi: Likewise.
> * doc/posix-functions/logl.texi: Likewise.
>
> = lib/itold.c
> =
> /* Replacement for 'int' to 'long double' conversion routine.
>Copyright (C) 2011 Free Software Foundation, Inc.
>Written by Bruno Haible , 2011.
>
>This program is free software: you can redistribute it and/or modify
>it under the terms of the GNU General Public License as published by
>the Free Software Foundation; either version 3 of the License, or
>(at your option) any later version.
>
>This program is distributed in the hope that it will be useful,
>but WITHOUT ANY WARRANTY; without even the implied warranty of
>MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>GNU General Public License for more details.
>
>You should have received a copy of the GNU General Public License
>along with this program. If not, see <http://www.gnu.org/licenses/>. */
>
> #include
>
> /* Specification. */
> #include
>
> void
> _Qp_itoq (long double *result, int a)
> {
> /* Convert from 'int' to 'double', then from 'double' to 'long double'. */
> *result = (double) a;
> }
Hi Bruno,
Thanks for all of this work.
I've taken the liberty of adding the above file.
Without it, any use of gnulib with affected modules fails like this:
gnulib/gnulib-tool: *** file /h/j/w/co/cu/gnulib/lib/itold.c not found
gnulib/gnulib-tool: *** Stop.
E.g.,
http://hydra.nixos.org/jobset/gnu/gzip-master
>From 0041d0b4ff6c590ea87c21bdeabc22d00133c184 Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Sat, 1 Oct 2011 14:34:09 +0200
Subject: [PATCH] float, math: add omitted file
* lib/itold.c: Add file, required for yesterday's float change.
---
ChangeLog |5 +
lib/itold.c | 28
2 files changed, 33 insertions(+), 0 deletions(-)
create mode 100644 lib/itold.c
diff --git a/ChangeLog b/ChangeLog
index d60071b..779e00d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2011-10-01 Jim Meyering
+
+ float, math: add omitted file
+ * lib/itold.c: Add file, required for yesterday's float change.
+
2011-10-01 Bruno Haible
isinf: Fix for OpenBSD/x86.
diff --git a/lib/itold.c b/lib/itold.c
new file mode 100644
index 000..0236f33
--- /dev/null
+++ b/lib/itold.c
@@ -0,0 +1,28 @@
+/* Replacement for 'int' to 'long double' conversion routine.
+ Copyright (C) 2011 Free Software Foundation, Inc.
+ Written by Bruno Haible , 2011.
+
+ This program is free software: you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include
+
+/* Specification. */
+#include
+
+void
+_Qp_itoq (long double *result, int a)
+{
+ /* Convert from 'int' to 'double', then from 'double' to 'long double'. */
+ *result = (double) a;
+}
--
1.7.7.rc0.362.g5a14
Re: msvc-inval: more options
Bruno Haible wrote: > Jim Meyering wrote: >> > -#if MSVC_INVALID_PARAMETER_HANDLING == DEFAULT_HANDLING >> > +#if HAVE_MSVC_INVALID_PARAMETER_HANDLER \ >> > +&& MSVC_INVALID_PARAMETER_HANDLING == DEFAULT_HANDLING >> >gl_msvc_inval_ensure_handler (); >> > #endif >> >> That is a detail specific to MSVC. Why impose the readability and >> maintenance hassle of requiring such an expression on all callers? > > It's not for all callers. It's only for specific tests. I could introduce > a macro > > MSVC_INIT_INVALID_PARAMETER_HANDLER_FOR_TESTS_WITH_FUNCTIONS_WITHOUT_GNULIB_WORKAROUND() > but that would be so special-purpose that I really wonder whether it's worth > adding to msvc-inval.h. I see now. If there will only ever be these four guards, then I agree that it's not worth the trouble. Thanks for explaining.
Re: msvc-inval: more options
Bruno Haible wrote:
>> I noticed that the latest from gnulib's tests gets some new warnings
>> when compiled on Fedora 15:
>>
>> test-fread.c: In function 'main':
>> test-fread.c:43:3: error: implicit declaration of function
>> gl_msvc_inval_ensure_handler'
>> [-Werror=implicit-function-declaration]
>
> Oops, yes, I made a mistake. Fixed below.
>
>> Wouldn't it be better to make the commonly-used-in-application-code
>> gl_msvc_inval_ensure_handler ();
>> a no-op when possible, so that applications don't need to guard
>> uses of it with #if directives like those above?
>
> This would make sense for the cases
> !HAVE_MSVC_INVALID_PARAMETER_HANDLER
> HAVE_MSVC_INVALID_PARAMETER_HANDLER && MSVC_INVALID_PARAMETER_HANDLING ==
> DEFAULT_HANDLING
> But in the case
> HAVE_MSVC_INVALID_PARAMETER_HANDLER && MSVC_INVALID_PARAMETER_HANDLING ==
> HAIRY_LIBRARY_HANDLING
> the handler exists but has, on its own, not the desired effect. Therefore
> I think it's better to be clear about which conditions do benefit from the
> code.
>
> 2011-09-28 Bruno Haible
>
> fgetc, fputc, fread, fwrite tests: Fix link error.
> * tests/test-fgetc.c (main): Don't invoke gl_msvc_inval_ensure_handler
> on non-MSVC platforms.
> * tests/test-fputc.c (main): Likewise.
> * tests/test-fread.c (main): Likewise.
> * tests/test-fwrite.c (main): Likewise.
> Reported by Jim Meyering.
Hi Bruno,
Thank you for the quick fix.
With that, coreutils-updated-to-use-latest-from-gnulib
now passes its build/check tests on Fedora 15 and rawhide.
> --- tests/test-fgetc.c.orig Wed Sep 28 10:22:49 2011
> +++ tests/test-fgetc.cWed Sep 28 10:22:30 2011
> @@ -35,9 +35,10 @@
> {
>const char *filename = "test-fgetc.txt";
>
> - /* We don't have an fread() function that installs an invalid parameter
> + /* We don't have an fgetc() function that installs an invalid parameter
> handler so far. So install that handler here, explicitly. */
> -#if MSVC_INVALID_PARAMETER_HANDLING == DEFAULT_HANDLING
> +#if HAVE_MSVC_INVALID_PARAMETER_HANDLER \
> +&& MSVC_INVALID_PARAMETER_HANDLING == DEFAULT_HANDLING
>gl_msvc_inval_ensure_handler ();
> #endif
That is a detail specific to MSVC. Why impose the readability and
maintenance hassle of requiring such an expression on all callers?
Wouldn't you prefer to make any MSVC-related code as inconspicuous
as possible? Not to mention avoiding the duplication of an #if
expression like that...
Re: msvc-inval: more options
Bruno Haible wrote:
>> FAIL: test-fgetc.exe
>> FAIL: test-fputc.exe
>> FAIL: test-fread.exe
>> FAIL: test-fwrite.exe
>>
>> All these fail on a stream whose file descriptor has been closed. Most
>> libraries won't do such a thing. Therefore I don't find it worth to fix
>> this. Just document it.
>
> But two of the tests still fail, and the other two succeed only accidentally.
> This patch makes the 4 tests pass:
Hi Bruno,
I noticed that the latest from gnulib's tests gets some new warnings
when compiled on Fedora 15:
test-fread.c: In function 'main':
test-fread.c:43:3: error: implicit declaration of function
'gl_msvc_inval_ensure_handler' [-Werror=implicit-function-declaration]
cc1: all warnings being treated as errors
test-fputc.c: In function 'main':
test-fputc.c:43:3: error: implicit declaration of function
'gl_msvc_inval_ensure_handler' [-Werror=implicit-function-declaration]
cc1: all warnings being treated as errors
test-fwrite.c: In function 'main':
test-fwrite.c:43:3: error: implicit declaration of function
'gl_msvc_inval_ensure_handler' [-Werror=implicit-function-declaration]
cc1: all warnings being treated as errors
make[3]: *** [test-fputc.o] Error 1
make[3]: *** Waiting for unfinished jobs
make[3]: *** [test-fread.o] Error 1
make[3]: *** [test-fwrite.o] Error 1
CC test-strings.o
test-fgetc.c: In function 'main':
test-fgetc.c:43:3: error: implicit declaration of function
'gl_msvc_inval_ensure_handler' [-Werror=implicit-function-declaration]
cc1: all warnings being treated as errors
That's because the guard in each test-*.c file is weaker
than the guards in msvc-inval.h. Thus, the use is exposed
while the declaration is not.
>From msvc-inval.h:
#define DEFAULT_HANDLING 0
#define HAIRY_LIBRARY_HANDLING 1
#define SANE_LIBRARY_HANDLING 2
#if HAVE_MSVC_INVALID_PARAMETER_HANDLER \
&& !(MSVC_INVALID_PARAMETER_HANDLING == SANE_LIBRARY_HANDLING)
/* A native Windows platform with the "invalid parameter handler" concept,
and either DEFAULT_HANDLING or HAIRY_LIBRARY_HANDLING. */
# if MSVC_INVALID_PARAMETER_HANDLING == DEFAULT_HANDLING
/* Default handling. */
# ifdef __cplusplus
extern "C" {
# endif
/* Ensure that the invalid parameter handler in installed that just returns.
Because we assume no other part of the program installs a different
invalid parameter handler, this solution is multithread-safe. */
extern void gl_msvc_inval_ensure_handler (void);
# ifdef __cplusplus
}
# endif
>From test-fread.c:
#if MSVC_INVALID_PARAMETER_HANDLING == DEFAULT_HANDLING
gl_msvc_inval_ensure_handler ();
#endif
To kludge around it, I built with this:
make check CFLAGS=-DMSVC_INVALID_PARAMETER_HANDLING=99
Wouldn't it be better to make the commonly-used-in-application-code
gl_msvc_inval_ensure_handler ();
a no-op when possible, so that applications don't need to guard
uses of it with #if directives like those above?
Re: gengetopt, anyone?
Reuben Thomas wrote: > I've just been playing with gengetopt. There's a lot to like: it gives > me (potentially multi-language) options handling for one line per > option (plus any extra text I want) in a simple .ggo file, and half a > dozen lines of code (four of which really should be redundant as they > test the help and version flags and call the relevant display > routine). All the necessary variables to signal which options are set > and their values are defined for me. > > The only thing really not to like is that for my simple six-option > program it generates 565 lines of C. My program is only 140 lines, > representing a roughly 500-line net addition when I use gengetopt. I'm > not inclined to worry that much, but it does seem a lot. > > There's also a slew of minor issues, mostly cosmetic, and all easy to > fix, for which I've filed a dizaine of bug reports and offered to fix > myself. > > It should not go unremarked that gengetopt is big on gnulib (though > one of the bugs I filed is that its manual overenthusiastically > documents how to add gnulib to a project!). > > Has anyone else any experience with it? It would seem to have > applications in, for example, coreutils, though its use would > obviously be a fair amount of work and the quantity of code it > generates might be more of a concern. Hi Reuben, "a fair amount"? British understatement, surely ;-) I suspect that you'll find many exceptions. If we were starting coreutils development now, I'd be much more inclined to use it, but it seems like the minimal benefit from that sort of large-scale factorization would not be worth the effort. [not even considering code size issues]
[PATCH] test-futimens: avoid a warning from gcc -Wshadow
I tried the latest from gnulib via coreutils, and encountered a new failure:
In file included from test-fdutimensat.c:35:0:
test-futimens.h: In function 'test_futimens':
test-futimens.h:95:9: error: declaration of 'fd' shadows a previous local \
[-Werror=shadow]
test-futimens.h:29:7: error: shadowed declaration is here [-Werror=shadow]
cc1: all warnings being treated as errors
I fixed it like this:
>From 90520caf3cfe735b83237a11034b7aa0c3bb05b9 Mon Sep 17 00:00:00 2001
From: Jim Meyering
Date: Sat, 24 Sep 2011 21:02:15 +0200
Subject: [PATCH] test-futimens: avoid a warning from gcc -Wshadow
* tests/test-futimens.h (test_futimens): Rename inner local, s/fd/fd0/
to avoid a shadowing warning.
---
ChangeLog |6 ++
tests/test-futimens.h |8
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 9def263..f154617 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2011-09-24 Jim Meyering
+
+ test-futimens: avoid a warning from gcc -Wshadow
+ * tests/test-futimens.h (test_futimens): Rename inner local, s/fd/fd0/
+ to avoid a shadowing warning.
+
2011-09-24 Bruno Haible
fdopen: Support for MSVC 9.
diff --git a/tests/test-futimens.h b/tests/test-futimens.h
index 2cd7c01..9c9b1bb 100644
--- a/tests/test-futimens.h
+++ b/tests/test-futimens.h
@@ -90,11 +90,11 @@ test_futimens (int (*func) (int, struct timespec const *),
ASSERT (errno == EBADF);
}
{
-int fd = dup (0);
-ASSERT (0 <= fd);
-ASSERT (close (fd) == 0);
+int fd0 = dup (0);
+ASSERT (0 <= fd0);
+ASSERT (close (fd0) == 0);
errno = 0;
-ASSERT (func (fd, NULL) == -1);
+ASSERT (func (fd0, NULL) == -1);
ASSERT (errno == EBADF);
}
{
--
1.7.7.rc0.362.g5a14
Re: C99, declaration after statement
Bruno Haible wrote:
> Hi Jim,
>
>> > About this change of fts.c from 2011-08-18.
>> > The declaration of max_entries in line 1339 comes after a statement. Not
>> > valid C99.
>>
>> s/C99/C89/
>
> Oops, yes. Not valid C89.
>
>> C89 is now more than 12 years old
>
> You meant s/C89/C99/ ?
No, I meant s/12/22/, to emphasize that C89 is now *22* years old.
>> I see so much more value in using C99's decl-after-statement feature
>
> Sure decl-after-statement is comfortable, But even without it, you can have a
> good code structure, by using braces (compound statements).
That is rarely a viable alternative.
One of the main advantages of decl-after-statement is improved
readability/maintainability. Requiring added braces just to delimit
the reduced scope is often enough to render the code *less*
readable/maintainable than the original. The reader will ask
themselves, "Why on earth did the author add braces here?"
Few will guess that we're trying to accommodate a standard that
is 22 years old.
>> accommodating the oh-so-few relevant compilers that still lack
>> support that I have a hard time justifying gnulib's current policy.
>
> The compilers that don't support decl-after-statement that I know of are:
>
> - gcc 2.95 (still used in some distros, AFAIK).
>
> - IRIX 6.5 cc
> ("A declaration cannot appear after an executable statement in a block.")
>
> - HP-UX 10 cc
> ("Unexpected symbol: "int".")
>
> - Solaris 2.6 cc
> ("syntax error before or at: int")
I hold that the above are all museum-piece compilers.
You may still use them for portability testing,
but very few "real" users do.
> - MSVC 9.0, also known as MS Visual Studio 2008, released in 2008
> ("error C2143"). See also
>
>
>
> IRIX is among gnulib's usual targets, and MSVC is being added now.
Portability is a worthy goal, but when such old compilers tie us
to an obsolete standard like c89, it is time to reexamine priorities.
I find it ironic that you are willing to let a flaw in MSVC influence
gnulib's coding conventions.
>> Do you still object to allowing declarations after statements in gnulib?
>
> Yes, I think if the gnulib user has the benefit of not needing to
> worry about this
> issue, it's a benefit for everyone. It's useful if we can say that all of
> gnulib
> is working with C89.
>
>> If so, can you estimate when you will relax that stance?
>
> It depends on what machines are considered "reasonable portability targets"
> and on the compiler vendors. I guess IRIX will be dropped within 5 years,
> but MSVC support is likely to be relevant for ca. 8 years or more.
Sorry, but that is too long to wait. IMHO, even 4 years would
be too long. We need another solution. See below.
>> Here is an alternative to imposing a blanket policy:
>>
>> Allow/encourage use of C99, but heed requests for conversion to C89,
>> if there are any.
>
> I can't agree to this. IMO the comfort for the programmer who contributes to
> gnulib is not worth the hassles to people who want to compile the package on
> IRIX or with MSVC.
I will then do what I did for a few years with coreutils:
Manually maintain a C99-to-C89 patch for each of the few .c files
that deserve the effort, like fts.c.
Then, people who require C89 sources can apply the patch manually.
Or, who knows... maybe gnulib-tool will be extended to apply the
patch for them, say when given a --c89 option.
Thus, I have the freedom to use the newer feature, yet without
impeding those who insist on using deficient compilers.
Another file for which this would be useful is grep's dfa.[ch].
It is not currently in gnulib, but if we do migrate it, I would
not want to be constrained.
Re: [PATCH 8/8] fts: do not exhaust memory when processing million-entry directories
Bruno Haible wrote: > Hi Jim, > > About this change of fts.c from 2011-08-18. > The declaration of max_entries in line 1339 comes after a statement. Not > valid C99. s/C99/C89/ > gnulib's conventions are still to not require a C99 compiler. Hi Bruno, I see so much more value in using C99's decl-after-statement feature than in accommodating the oh-so-few relevant compilers that still lack support that I have a hard time justifying gnulib's current policy. Do you still object to allowing declarations after statements in gnulib? If so, can you estimate when you will relax that stance? If you still think it's a problem, I don't mind adjusting fts.c to accommodate, but I would like to know how far it is to the light at the end of this tunnel. Here is an alternative to imposing a blanket policy: Allow/encourage use of C99, but heed requests for conversion to C89, if there are any. There will be none via any package I maintain, since they all already use the decl-after-statement feature elsewhere. If no one else is using the fts module, there will be no problem with this particular case. C89 is now more than 12 years old, and wide support for decl-after-statement is years older than that. Let it die ;-)
Re: a saner bootstrap script [Was Re: I fixed this once already]
Gary V. Vaughan wrote: >>> * g...@github.com:gvvaughan/GNU-coreutils.git in gary/bootstrap >>> https://github.com/gvvaughan/GNU-coreutils/commits/gary/bootstrap I'll try to find time for this next week. I glanced through the first few lines of your script and found FGREP=fgrep EGREP=egrep ... Please use the more standard grep -F and grep -E instead.
Re: modernize ftruncate module
Bruno Haible wrote: > Hi Jim, > > Here's a proposal to modernize the 'ftruncate' module. It's triggered by link > errors that I got on MSVC 9, because the 'ftruncate' module, while present in > gnulib, is marked as 'obsolete' and therefore not included by default. > This classification was based on the knowledge that all modern Unix platforms > have ftruncate(), mingw too, and MSVC was not yet a portability target. > > But now that MSVC is worth considering (because we have a standard 'compile' > script for it in GNU, and there is demand from Octave), the actions that make > sense are: > - drop the old code for SVR2 platforms, > - keep the code for native Windows platforms, > - un-deprecate the module. > > Additional improvements: > - Include , for the declaration of chsize(). > - Depend on 'largefile', for automatic use of ftruncate64. > > > 2011-09-21 Bruno Haible > > ftruncate: Un-deprecate, concentrate on Win32 support. > * modules/ftruncate (Status, Notice): Remove sections. > (Depends-on): Add largefile. > * m4/ftruncate.m4 (gl_FUNC_FTRUNCATE): Drop failure message on non-mingw > platforms. > * lib/ftruncate.c: Remove code for the older platforms. For Win32, > include > . > * modules/perror-tests (Depends-on): Add ftruncate. > * doc/posix-functions/ftruncate.texi: Mention the MSVC problem and the > 'ftruncate' module. That sounds fine.
