rename strftime module

Discussion:

Bruno Haible

2017-04-30 14:53:27 UTC

Hi,

The name of the strftime module is misleading:
1) It suggests that it defines the function 'strftime', but in fact it
defines 'nstrftime'. GNU findutils fell into this pit: their bootstrap.conf
requests the gnulib 'strftime' module but the coded doesn't use nstrftime.
2) When we need to apply workarounds to the strftime() function, it is
awkward to not be able to use the module name 'strftime' for it.

I'd suggest to rename 'strftime' to 'strftime-gnu' or 'nstrftime', and mark
the 'strftime' module obsolete for two years, then after two years remove it.
So that all users of this module have enough time to notice the change.

Bruno

Pádraig Brady

2017-04-30 17:22:01 UTC

Permalink

Post by Bruno Haible
Hi,
1) It suggests that it defines the function 'strftime', but in fact it
defines 'nstrftime'. GNU findutils fell into this pit: their bootstrap.conf
requests the gnulib 'strftime' module but the coded doesn't use nstrftime.
2) When we need to apply workarounds to the strftime() function, it is
awkward to not be able to use the module name 'strftime' for it.
I'd suggest to rename 'strftime' to 'strftime-gnu' or 'nstrftime', and mark
the 'strftime' module obsolete for two years, then after two years remove it.
So that all users of this module have enough time to notice the change.

Sounds sensible to me.
I was initially confused with this module also

Paul Eggert

2017-04-30 20:52:37 UTC

Permalink

Post by Bruno Haible
I'd suggest to rename 'strftime' to 'strftime-gnu' or 'nstrftime', and mark
the 'strftime' module obsolete for two years, then after two years remove it.
So that all users of this module have enough time to notice the change.

Makes sense to me.

Jim Meyering

2017-05-01 00:16:45 UTC

Permalink

Good plan. Thank you.

Bernhard Voelker

2017-05-02 20:54:40 UTC

Permalink

Post by Bruno Haible
1) It suggests that it defines the function 'strftime', but in fact it
defines 'nstrftime'. GNU findutils fell into this pit: their bootstrap.conf
requests the gnulib 'strftime' module but the coded doesn't use nstrftime.

What exactly do you mean by "fell into ..."?
I still don't see 'nstrftime' anywhere in findutils.git nor do
I remember this topic on the findutils mailing list in the last
two years. Do you see it fixed somewhere?
Do you have a reference for me?

Thanks & have a nice day,
Berny

Bruno Haible

2017-05-02 22:30:14 UTC

Permalink

Post by Bernhard Voelker

What exactly do you mean by "fell into ..."?

I mean that the bootstrap.conf requests the module 'strftime', whose benefit
is that it supports GNU extensions in the format string, but it does not get used.

'nstrftime' is meant to lift this limitation [1]:
"Some of these formats might not be available on all systems, due to
differences in the C strftime function between systems."

How to reproduce:

Checkout findutils git.
$ ./bootstrap
$ ./configure
$ make
$ nm gl/lib/libgnulib.a | grep ' T ' | grep strftime
0000000000001b80 T nstrftime
$ nm find/print.o | grep strftime
U strftime
$ nm lib/listfile.o | grep strftime
U strftime
$ nm locate/locate.o | grep strftime
U strftime
$ LD_DEBUG=bindings find/find . -prune -printf '%TA\n' 2>&1 | grep strftime
3928: binding file find/find [0] to /lib/x86_64-linux-gnu/libc.so.6 [0]: normal symbol `strftime' [GLIBC_2.2.5]

As you can see, it uses 'strftime' from glibc. It does not use 'nstrftime'.

Bruno

[1] https://www.gnu.org/software/findutils/manual/html_node/find_html/Time-Formats.html

Bernhard Voelker

2017-05-03 05:53:30 UTC

Permalink

Post by Bruno Haible

Post by Bernhard Voelker
What exactly do you mean by "fell into ..."?

I mean that the bootstrap.conf requests the module 'strftime', whose benefit
is that it supports GNU extensions in the format string, but it does not get used.
"Some of these formats might not be available on all systems, due to
differences in the C strftime function between systems."

...

Post by Bruno Haible
As you can see, it uses 'strftime' from glibc. It does not use 'nstrftime'.

Okay, thanks for clarifying that this is still an issue in find(1).
I understood the "fell into" that this was already fixed somewhere
in the past but obviously it wasn't.
Just out of curiosity: how did you find out?

Thanks & have a nice day,
Berny

Bruno Haible

2017-07-23 23:28:31 UTC

Permalink

Hi,

This proposal from
<https://lists.gnu.org/archive/html/bug-gnulib/2017-04/msg00167.html>

So I'm implementing it.

2017-07-23 Bruno Haible <***@clisp.org>

Rename module 'strftime' to 'nstrftime'.
* m4/nstrftime.m4: Renamed from m4/strftime.m4.
* lib/nstrftime.c: Renamed from lib/strftime.c.
* modules/nstrftime: Renamed from modules/strftime.
(Files, Makefile.am): Update.
* tests/test-nstrftime.c: Renamed from tests/test-strftime.c.
Fix comment.
* modules/nstrftime-tests: Renamed from modules/strftime-tests.
(Files, Makefile.am): Update.
* modules/strftime: New file, an obsolete indirection.
* doc/posix-functions/strftime.texi: Update reference.
* config/srclist.txt: Update info.

diff --git a/modules/nstrftime b/modules/nstrftime
index b54f44e..b559b5e 100644
--- a/modules/nstrftime
+++ b/modules/nstrftime
@@ -3,9 +3,9 @@ nstrftime() function: convert date and time to string, with GNU extensions.

Files:
lib/strftime.h
-lib/strftime.c
+lib/nstrftime.c
m4/tm_gmtoff.m4
-m4/strftime.m4
+m4/nstrftime.m4

Depends-on:
extensions
@@ -16,7 +16,7 @@ configure.ac:
gl_FUNC_GNU_STRFTIME

Makefile.am:
-lib_SOURCES += strftime.c
+lib_SOURCES += nstrftime.c

Include:
"strftime.h"
diff --git a/modules/nstrftime-tests b/modules/nstrftime-tests
index 17f7001..708b510 100644
--- a/modules/nstrftime-tests
+++ b/modules/nstrftime-tests
@@ -1,5 +1,5 @@
Files:
-tests/test-strftime.c
+tests/test-nstrftime.c
tests/macros.h

Depends-on:
@@ -8,5 +8,5 @@ strerror
configure.ac:

Makefile.am:
-TESTS += test-strftime
-check_PROGRAMS += test-strftime
+TESTS += test-nstrftime
+check_PROGRAMS += test-nstrftime
diff --git a/tests/test-nstrftime.c b/tests/test-nstrftime.c
index 102e168..85af8a6 100644
--- a/tests/test-nstrftime.c
+++ b/tests/test-nstrftime.c
@@ -1,4 +1,4 @@
-/* Test that posixtime works as required.
+/* Test that nstrftime works as required.
Copyright (C) 2011-2017 Free Software Foundation, Inc.

This program is free software: you can redistribute it and/or modify
diff --git a/config/srclist.txt b/config/srclist.txt
index ae24d4a..bf6e4a2 100644
--- a/config/srclist.txt
+++ b/config/srclist.txt
@@ -233,7 +233,7 @@ $GETTEXT gettext-runtime/po/remove-potcdate.sin build-aux/po release
#$LIBCSRC sysdeps/unix/bsd/unlockpt.c lib gpl
#$LIBCSRC sysdeps/unix/dirfd.c lib gpl
#$LIBCSRC sysdeps/unix/grantpt.c lib gpl
-#$LIBCSRC sysdeps/unix/rmdir.c lib gpl
+#$LIBCSRC sysdeps/unix/rmdir.c lib gpl (nstrftime.c in gnulib)
#$LIBCSRC time/strftime.c lib gpl
# These are close, but we are using the gettext versions.
#$LIBCSRC misc/mkdtemp.c lib gpl
diff --git a/doc/posix-functions/strftime.texi b/doc/posix-functions/strftime.texi
index dd52720..40e391a 100644
--- a/doc/posix-functions/strftime.texi
+++ b/doc/posix-functions/strftime.texi
@@ -24,5 +24,5 @@ Native Windows platforms (mingw, MSVC) support only a subset of time
zones supported by GNU or specified by POSIX. @xref{tzset}.
@end itemize

-Extension: Gnulib offers a module @samp{strftime} that provides an
+Extension: Gnulib offers a module @samp{nstrftime} that provides an
@code{nstrftime} function with various GNU extensions.

============================= modules/strftime =============================
Description:
nstrftime() function: convert date and time to string, with GNU extensions.

Status:
obsolete

Notice:
This module is obsolete. Use module 'nstrftime' instead.

Files:

Depends-on:
nstrftime

configure.ac:

Makefile.am:

Include:
"strftime.h"

License:
LGPL

Maintainer:
all

Paul Eggert

2017-07-24 04:53:45 UTC

Permalink

Post by Bruno Haible
So I'm implementing it.

Thanks. Should there be an entry for this in gnulib/NEWS?

Bruno Haible

2017-07-24 08:35:41 UTC

Permalink

Post by Paul Eggert
Thanks. Should there be an entry for this in gnulib/NEWS?

Indeed. You're right. Pushed:

diff --git a/ChangeLog b/ChangeLog
index 6b81952..43401d2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -12,6 +12,7 @@
* modules/strftime: New file, an obsolete indirection.
* doc/posix-functions/strftime.texi: Update reference.
* config/srclist.txt: Update info.
+ * NEWS: Mention the change.

2017-07-21 Tim Rühsen <***@gmx.de>

diff --git a/NEWS b/NEWS
index b75ca01..8fb2c54 100644
--- a/NEWS
+++ b/NEWS
@@ -42,6 +42,8 @@ User visible incompatible changes

Date Modules Changes

+2017-07-23 strftime This module is renamed to 'nstrftime'.
+
2017-05-19 closeout close_stdout longer closes stderr when addresses
are being sanitized, as the sanitizer outputs to
stderr afterwards.