Re: gnulib-tool: Improve handling of multiple --local-dir options

[Pavel Raiskup]

On Tuesday, February 19, 2019 7:18:57 PM CET Bruno Haible wrote:
> Hi Pavel,
> 
> > > makes the IFS handling a bit more robust.
> 
> What I meant is that
> 
>save_IFS="$IFS"
>for fpf_dir in $fpf_dirs
>do
>  IFS="$save_IFS"
>  [Some more code]
>done
>IFS="$save_IFS"
> 
> is more robust than
> 
>save_IFS="$IFS"
>for fpf_dir in $fpf_dirs
>do
>  [Some more code]
>done
>IFS="$save_IFS"
> 
> because the [Some more code] may invoke word splitting, and as a programmer
> I would expect this word splitting to be performed against the default IFS.

Makes sense, thanks.  I was only confused by the var="$quoted_var" style,
which I was before discouraged from.

Pavel

relocatable-prog: Use $ORIGIN trick on more platforms

[Bruno Haible]

Paul Smith noted on gnu-prog-discuss that other systems than glibc have support
for $ORIGIN in rpath. This patch changes the 'relocatable-prog' module to
make use of this feature, thus removing the need for a "wrapper"/"trampoline"
executable on these platforms.

Tested on
  - FreeBSD 11,
  - NetBSD 7, 8,
  - OpenBSD 6,
  - Solaris 9, 10, 11,
  - Haiku.


2019-02-19  Bruno Haible  

relocatable-prog: Use $ORIGIN trick on more platforms.
* m4/relocatable.m4 (gl_RELOCATABLE_BODY): Use $ORIGIN trick also on
FreeBSD >= 7.3, DragonFly >= 3.0, NetBSD >= 8.0, OpenBSD >= 5.4,
Solaris >= 10, Haiku. But don't use it on Android.
* build-aux/reloc-ldflags: Allow the use of the $ORIGIN trick also on
Hurd, FreeBSD, DragonFly, NetBSD, OpenBSD, Solaris, Haiku.

diff --git a/m4/relocatable.m4 b/m4/relocatable.m4
index c55f7b4..0044477 100644
--- a/m4/relocatable.m4
+++ b/m4/relocatable.m4
@@ -1,4 +1,4 @@
-# relocatable.m4 serial 19
+# relocatable.m4 serial 20
 dnl Copyright (C) 2003, 2005-2007, 2009-2019 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -40,10 +40,34 @@ AC_DEFUN([gl_RELOCATABLE_BODY],
 enable_rpath=no
 AC_CHECK_HEADERS([mach-o/dyld.h])
 AC_CHECK_FUNCS([_NSGetExecutablePath])
+changequote(,)dnl
 case "$host_os" in
   mingw*) is_noop=yes ;;
+  # For the platforms that support $ORIGIN, see
+  # .
+  # glibc systems, Linux with musl libc: yes. Android: no.
+  # Hurd: no 
.
+  linux*-android*) ;;
+  gnu*) ;;
   linux* | kfreebsd*) use_elf_origin_trick=yes ;;
+  # FreeBSD >= 7.3, DragonFly >= 3.0: yes.
+  freebsd | freebsd[1-7] | freebsd[1-6].* | freebsd7.[0-2]) ;;
+  dragonfly | dragonfly[1-2] | dragonfly[1-2].*) ;;
+  freebsd* | dragonfly*) use_elf_origin_trick=yes ;;
+  # NetBSD >= 8.0: yes.
+  netbsd | netbsd[1-7] | netbsd[1-7].*) ;;
+  netbsdelf | netbsdelf[1-7] | netbsdelf[1-7].*) ;;
+  netbsd*) use_elf_origin_trick=yes ;;
+  # OpenBSD >= 5.4: yes.
+  openbsd | openbsd[1-5] | openbsd[1-4].* | openbsd5.[0-3]) ;;
+  openbsd*) use_elf_origin_trick=yes ;;
+  # Solaris >= 10: yes.
+  solaris | solaris2.[1-9] | solaris2.[1-9].*) ;;
+  solaris*) use_elf_origin_trick=yes ;;
+  # Haiku: yes.
+  haiku*) use_elf_origin_trick=yes ;;
 esac
+changequote([,])dnl
 if test $is_noop = yes; then
   RELOCATABLE_LDFLAGS=:
   AC_SUBST([RELOCATABLE_LDFLAGS])
diff --git a/build-aux/reloc-ldflags b/build-aux/reloc-ldflags
index 4f2b10d..3aed330 100755
--- a/build-aux/reloc-ldflags
+++ b/build-aux/reloc-ldflags
@@ -54,7 +54,12 @@ case "$installdir" in
 esac
 
 case "$host_os" in
-  linux* | kfreebsd*)
+  linux* | gnu* | kfreebsd* | \
+  freebsd* | dragonfly* | \
+  netbsd* | \
+  openbsd* | \
+  solaris* | \
+  haiku*)
 rpath=
 save_IFS="$IFS"; IFS=":"
 for dir in $library_path_value; do
@@ -89,7 +94,14 @@ case "$host_os" in
 IFS="$save_IFS"
 # Output it.
 if test -n "$rpath"; then
-  echo "-Wl,-rpath,$rpath"
+  case "$host_os" in
+# At least some versions of FreeBSD, DragonFly, and OpenBSD need the
+# linker option "-z origin". See .
+freebsd* | dragonfly* | openbsd*)
+  echo "-Wl,-z,origin -Wl,-rpath,$rpath" ;;
+*)
+  echo "-Wl,-rpath,$rpath" ;;
+  esac
 fi
 ;;
   *)

progreloc: improve platform specific code

[Bruno Haible]

This set of patches improve the platform specific code in progreloc.c.
Most BSD systems and Solaris nowadays have a way to retrieve the name
of the current executable, in a way similar to /proc/self/exe on Linux.


2019-02-19  Bruno Haible  

progreloc: Simplify code for Android.
* lib/progreloc.c (executable_fd): Don't define on Android.
(maybe_executable, find_executable): Don't use executable_fd on Android.

2019-02-19  Bruno Haible  

progreloc: Speed up executable lookup on various platforms.
* lib/progreloc.c: Include .
(safe_read, full_read): New functions.
(find_executable): On GNU/kFreeBSD, FreeBSD, DragonFly, NetBSD, Solaris,
prefer the information from the /proc file system to a PATH search.

>From cd46bf0ca5083162f3ac564ebbdeb6371085df45 Mon Sep 17 00:00:00 2001
From: Bruno Haible 
Date: Tue, 19 Feb 2019 21:38:53 +0100
Subject: [PATCH 1/2] progreloc: Simplify code for Android.

* lib/progreloc.c (executable_fd): Don't define on Android.
(maybe_executable, find_executable): Don't use executable_fd on Android.
---
 ChangeLog   |  6 ++
 lib/progreloc.c | 15 ---
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 64e9acf..9cb7ac8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2019-02-19  Bruno Haible  
+
+	progreloc: Simplify code for Android.
+	* lib/progreloc.c (executable_fd): Don't define on Android.
+	(maybe_executable, find_executable): Don't use executable_fd on Android.
+
 2019-02-15  Bruno Haible  
 
 	gnulib-tool: Support --import with just a few tests, not --with-tests.
diff --git a/lib/progreloc.c b/lib/progreloc.c
index 1c4db0c..0cd335e 100644
--- a/lib/progreloc.c
+++ b/lib/progreloc.c
@@ -102,7 +102,7 @@ extern char * canonicalize_file_name (const char *name);
 
 #if ENABLE_RELOCATABLE
 
-#if defined __linux__ || defined __ANDROID__ || defined __CYGWIN__
+#if defined __linux__ || defined __CYGWIN__
 /* File descriptor of the executable.
(Only used to verify that we find the correct executable.)  */
 static int executable_fd = -1;
@@ -118,7 +118,7 @@ maybe_executable (const char *filename)
 return false;
 #endif
 
-#if defined __linux__ || defined __ANDROID__ || defined __CYGWIN__
+#if defined __linux__ || defined __CYGWIN__
   if (executable_fd >= 0)
 {
   /* If we already have an executable_fd, check that filename points to
@@ -180,7 +180,7 @@ find_executable (const char *argv0)
 
   return xstrdup (location);
 #else /* Unix */
-# if defined __linux__ || defined __ANDROID__
+# if defined __linux__
   /* The executable is accessible as /proc//exe.  In newer Linux
  versions, also as /proc/self/exe.  Linux >= 2.1 provides a symlink
  to the true pathname; older Linux versions give only device and ino,
@@ -205,6 +205,15 @@ find_executable (const char *argv0)
 }
   }
 # endif
+# if defined __ANDROID__
+  {
+char *link;
+
+link = xreadlink ("/proc/self/exe");
+if (link != NULL)
+  return link;
+  }
+# endif
 # ifdef __CYGWIN__
   /* The executable is accessible as /proc//exe, at least in
  Cygwin >= 1.5.  */
-- 
2.7.4

>From 18f4d4133eae7d8ac228ed2986748c52825f3d0e Mon Sep 17 00:00:00 2001
From: Bruno Haible 
Date: Tue, 19 Feb 2019 21:42:54 +0100
Subject: [PATCH 2/2] progreloc: Speed up executable lookup on various
 platforms.

* lib/progreloc.c: Include .
(safe_read, full_read): New functions.
(find_executable): On GNU/kFreeBSD, FreeBSD, DragonFly, NetBSD, Solaris,
prefer the information from the /proc file system to a PATH search.
---
 ChangeLog   |  8 +
 lib/progreloc.c | 94 +++--
 2 files changed, 100 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 9cb7ac8..949dbae 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2019-02-19  Bruno Haible  
 
+	progreloc: Speed up executable lookup on various platforms.
+	* lib/progreloc.c: Include .
+	(safe_read, full_read): New functions.
+	(find_executable): On GNU/kFreeBSD, FreeBSD, DragonFly, NetBSD, Solaris,
+	prefer the information from the /proc file system to a PATH search.
+
+2019-02-19  Bruno Haible  
+
 	progreloc: Simplify code for Android.
 	* lib/progreloc.c (executable_fd): Don't define on Android.
 	(maybe_executable, find_executable): Don't use executable_fd on Android.
diff --git a/lib/progreloc.c b/lib/progreloc.c
index 0cd335e..b3eb503 100644
--- a/lib/progreloc.c
+++ b/lib/progreloc.c
@@ -22,6 +22,7 @@
 /* Specification.  */
 #include "progname.h"
 
+#include 
 #include 
 #include 
 #include 
@@ -102,6 +103,48 @@ extern char * canonicalize_file_name (const char *name);
 
 #if ENABLE_RELOCATABLE
 
+#ifdef __sun
+
+/* Helper function, from gnulib module 'safe-read'.  */
+static size_t
+safe_read (int fd, void *buf, size_t count)
+{
+  for (;;)
+{
+  ssize_t result = read (fd, buf, count);
+
+  if (0 <= result || errno != EINTR)
+return 

Re: shell variable references - coding style

[Jim Meyering]

On Tue, Feb 19, 2019 at 10:02 AM Bruno Haible  wrote:
>
> Hi Pavel,
>
> > > [...]
> > > This patch fixes both issues, and makes the IFS handling a bit more 
> > > robust.
> > > [...]
> >
> > > -case $_fpf_arg in
> > > +case "$_fpf_arg" in
> > > [...]
> > > -  fpf_dirs=$1 ; shift
> > > -  fpf_cb=$1 ; shift
> > > +  fpf_dirs="$1"; shift
> > > +  fpf_cb="$1"; shift
> > > [...]
> >
> > ... and so on, I don't think it is more robust.  At least according to
> > Autoconf's Shellology [1] it should be actually better to write it the
> > other way around (without additional quotes).
>
> I do think it is more robust, because
>   * The number one mistake in shell scripts (measured by frequency of
> occurrence) is to reference variables without double quotes when word
> splitting is in fact undesired.
>   * Simple rules are easier to follow by programmers, resulting in fewer bugs.
>   * The rule
>   "Always double-quote shell variable references, except if you DO
>want word-splitting."
> is simpler than
>   "Always double-quote shell variable references, except if you DO
>want word-splitting OR in the right-hand side of assignments OR
>as argument of 'case' statements."
>
> The text that you quote says two different things:
>   * Backquotes inside double-quotes are hairy.
> There is a simple rule to avoid them: When you have a backquote
> expression, always first assign its result to a variable. Then use
> the variable (with double-quotes, usually).
>   * Bash 4.1 has a bug when you WANT word splitting.
> These two things don't make my style rule
>   "Always double-quote shell variable references, except if you DO
>want word-splitting."
> less robust.
>
> > FTR, Gary Vaughan has wrote a syntax checker rules for protecting us from
> > adding such statements into libtool codebase.
>
> Opinions regarding coding style differ. Gary is entitled to his opinion, as
> much as I am entitled to mine.
>
> Bruno
>
> > [1] https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/
> > Shell-Substitutions.html

I too prefer to avoid double quotes after assignment-"=": less syntax
feels slightly more maintainable, here.
These days I rely heavily on shellcheck. I find it to be very useful,
and at least by default it doesn't complain about such usage.

Re: gnulib-tool: Improve handling of multiple --local-dir options

[Bruno Haible]

Hi Pavel,

> > makes the IFS handling a bit more robust.

What I meant is that

   save_IFS="$IFS"
   for fpf_dir in $fpf_dirs
   do
 IFS="$save_IFS"
 [Some more code]
   done
   IFS="$save_IFS"

is more robust than

   save_IFS="$IFS"
   for fpf_dir in $fpf_dirs
   do
 [Some more code]
   done
   IFS="$save_IFS"

because the [Some more code] may invoke word splitting, and as a programmer
I would expect this word splitting to be performed against the default IFS.

Bruno

Re: shell variable references - coding style

[Bruno Haible]

Hi Pavel,

> > [...]
> > This patch fixes both issues, and makes the IFS handling a bit more robust.
> > [...]
> 
> > -case $_fpf_arg in
> > +case "$_fpf_arg" in
> > [...]
> > -  fpf_dirs=$1 ; shift
> > -  fpf_cb=$1 ; shift
> > +  fpf_dirs="$1"; shift
> > +  fpf_cb="$1"; shift
> > [...]
> 
> ... and so on, I don't think it is more robust.  At least according to
> Autoconf's Shellology [1] it should be actually better to write it the
> other way around (without additional quotes).

I do think it is more robust, because
  * The number one mistake in shell scripts (measured by frequency of
occurrence) is to reference variables without double quotes when word
splitting is in fact undesired.
  * Simple rules are easier to follow by programmers, resulting in fewer bugs.
  * The rule
  "Always double-quote shell variable references, except if you DO
   want word-splitting."
is simpler than
  "Always double-quote shell variable references, except if you DO
   want word-splitting OR in the right-hand side of assignments OR
   as argument of 'case' statements."

The text that you quote says two different things:
  * Backquotes inside double-quotes are hairy.
There is a simple rule to avoid them: When you have a backquote
expression, always first assign its result to a variable. Then use
the variable (with double-quotes, usually).
  * Bash 4.1 has a bug when you WANT word splitting.
These two things don't make my style rule
  "Always double-quote shell variable references, except if you DO
   want word-splitting."
less robust.

> FTR, Gary Vaughan has wrote a syntax checker rules for protecting us from
> adding such statements into libtool codebase.

Opinions regarding coding style differ. Gary is entitled to his opinion, as
much as I am entitled to mine.

Bruno

> [1] https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/
> Shell-Substitutions.html

Re: Gnulib's alloca.h used even when there is a system header

[Eli Zaretskii]

> From: Bruno Haible 
> Cc: Eli Zaretskii , Paul Eggert 
> Date: Tue, 19 Feb 2019 00:32:24 +0100
> 
> Hi Eli,
> 
> >  ./alloca.h:40:18: error: inlining failed in call to always_inline 
> > '__builtin_alloca': function not considered for inlining
> >   #  define alloca __builtin_alloca
> >^
> 
> GCC never inlines a function that calls alloca() or __builtin_alloca().
> The reason is that if you call this function in a loop, then without
> inlining it will consume a bounded amount of stack whereas with inlining
> it might cause a stack overflow.
> 
> The mingw people have declared their new alloca() function as "always
> inline", and GCC is decent enough to point us to the dilemma that it
> cannot resolve.

The GCC diagnostics is somewhat cryptic, so it's hard for me to be
sure what it means.  My original interpretation was that due to
Gnulib's #define, what GCC sees is this:

  extern inline __attribute__((__always_inline__))
void *__builtin_alloca( size_t __n ){ return __builtin_alloca( __n ); }

which AFAIU cannot possibly compile.

As for not ever inlining, please see a toy function I compiled and its
disassembly at the end of this message.  Does this confirm what you
say above?  I'm not sure.

> The upshot is: you can't define an alloca() function this way.
> Either define it as a macro - which is what the gnulib override does,
> if it gets activated -, or define it as a function in assembly-language.
> 
> Since you say that this is a fresh bug from mingw, it's probably best
> that it gets reported to the mingw people. Then we don't need a Gnulib
> workaround.

CC'ing Keith, who maintains the MinGW runtime.

Regardless of whether there is or isn't a bug in MinGW, I urge you to
reconsider whether unconditionally overriding a system header in such
a way is a good idea, at least for platforms that use GCC.  I think
this is a fragile arrangement, even if it seems to work.

Here's the toy source code I used:

#include 
#include 

char *
make_string (int c, size_t n)
{
  char *local_string = alloca (n + 1);
  int i;

  for (i = 0; i < n; i++)
local_string[i] = c;
  local_string[n] = '\0';
  return strdup (local_string);
}

And here's the disassembly of the object file generated by MinGW GCC:


al.o: file format pe-i386


Disassembly of section .text:

 <_make_string>:
#include 
#include 

char *
make_string (int c, size_t n)
{
   0:   55  push   %ebp
   1:   89 e5   mov%esp,%ebp
   3:   83 ec 28sub$0x28,%esp
  char *local_string = alloca (n + 1);
   6:   8b 45 0cmov0xc(%ebp),%eax
   9:   83 c0 01add$0x1,%eax
   c:   89 45 ecmov%eax,-0x14(%ebp)

#if defined _GNU_SOURCE || ! defined _NO_OLDNAMES
/* This is the GNU standard API; it is also compatible with Microsoft's
 * original, but now deprecated, naming convention.
 */
__CRT_ALIAS void *alloca( size_t __n ){ return __builtin_alloca( __n ); }
   f:   8b 45 ecmov-0x14(%ebp),%eax
  12:   8d 50 0flea0xf(%eax),%edx
  15:   b8 10 00 00 00  mov$0x10,%eax
  1a:   83 e8 01sub$0x1,%eax
  1d:   01 d0   add%edx,%eax
  1f:   b9 10 00 00 00  mov$0x10,%ecx
  24:   ba 00 00 00 00  mov$0x0,%edx
  29:   f7 f1   div%ecx
  2b:   6b c0 10imul   $0x10,%eax,%eax
  2e:   e8 00 00 00 00  call   33 <_make_string+0x33>
  33:   29 c4   sub%eax,%esp
  35:   8d 44 24 04 lea0x4(%esp),%eax
  39:   83 c0 0fadd$0xf,%eax
  3c:   c1 e8 04shr$0x4,%eax
  3f:   c1 e0 04shl$0x4,%eax
  42:   89 45 f0mov%eax,-0x10(%ebp)
  int i;

  for (i = 0; i < n; i++)
  45:   c7 45 f4 00 00 00 00movl   $0x0,-0xc(%ebp)
  4c:   eb 11   jmp5f <_make_string+0x5f>
local_string[i] = c;
  4e:   8b 55 f4mov-0xc(%ebp),%edx
  51:   8b 45 f0mov-0x10(%ebp),%eax
  54:   01 d0   add%edx,%eax
  56:   8b 55 08mov0x8(%ebp),%edx
  59:   88 10   mov%dl,(%eax)
make_string (int c, size_t n)
{
  char *local_string = alloca (n + 1);
  int i;

  for (i = 0; i < n; i++)
  5b:   83 45 f4 01 addl   $0x1,-0xc(%ebp)
  5f:   8b 45 f4mov-0xc(%ebp),%eax
  62:   39 45 0ccmp%eax,0xc(%ebp)
  65:   77 e7   ja 4e <_make_string+0x4e>
local_string[i] = c;
  local_string[n] = '\0';
  67:   8b 55 f0mov-0x10(%ebp),%edx
  6a:   8b 45 0cmov0xc(%ebp),%eax
  6d:   01 d0   add%edx,%eax
  6f:   c6 00 00movb   $0x0,(%eax)
  return strdup (local_string);
  72:   8b 45 f0mov-0x10(%ebp),%eax
  75:   89 04 24mov

Re: gnulib-tool: Improve handling of multiple --local-dir options

[Pavel Raiskup]

Thanks for working on this, Bruno!  Only nits..

On Thursday, February 14, 2019 8:53:33 PM CET Bruno Haible wrote:
> [...]
> This patch fixes both issues, and makes the IFS handling a bit more robust.
> [...]

> -case $_fpf_arg in
> +case "$_fpf_arg" in
> [...]
> -  fpf_dirs=$1 ; shift
> -  fpf_cb=$1 ; shift
> +  fpf_dirs="$1"; shift
> +  fpf_cb="$1"; shift
> [...]

... and so on, I don't think it is more robust.  At least according to
Autoconf's Shellology [1] it should be actually better to write it the
other way around (without additional quotes).

FTR, Gary Vaughan has wrote a syntax checker rules for protecting us from
adding such statements into libtool codebase.

[1] https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/
Shell-Substitutions.html

Pavel

[PATCH] ISO 8601 basic format and decimal hours, minutes

[Alex Eulenberg]

1. Accept dates in the ISO 8601 basic date and time format. This
is a format allowing the expression of point in time to the resolution
of a second with only alphanumeric characters -- no spaces, no
punctuation -- suitable for use in fields that cannot contain spaces,
hyphens, or colons.

2. Accept decimal fractions of time units other than seconds in
accordance with ISO 8601. Currently only decimal fractions of a
second are accepted. With the patch, the decimal separator may be
used for the expression of fractions of a minute (in both basic and
extended formats) as well as fractions of an hour.

The ISO 8601 basic date and time format consists of digits representing
the date, followed by the letter "T", followed by a string of digits
(optionally including a dot or comma as decimal separator) representing
the time of day, optionally followed by a zone offset or "Z" for
zero offset (UTC).

The patch will allow parse-datetime to accept strings of the following forms:

MMDDThhmmss[Z]: 20190102T030405, 20180102T030405Z, 20180102T110405-0800
MMDDThhmm[Z]: 20190121T0304, 20180102T0304Z, 20180102T1104-0800
MMDDThh[Z]:  20190121T03, 20180102T03Z, 20180102T11-0800

Note a variant of this format, where the separator is " " instead of "T",
is already accepted by parse-datetime but only when the time component
is either "hh" or "hhmm" (no seconds). With the patch, these forms are still
accepted, and a time component of form "hhmmss" is accepted as well.

MMDD hhmmss[Z]: 20190102 030405, 20190102 030405Z, 20190102 030405 Z,

The optional decimal part is interpreted as a fraction of a second,
a minute, or an hour, corresponding to what the previous two digits
represent:

20180102T210729.068302473 = 2018-01-02T21:07:29,068302473
19800615T1230.25  = 1980-06-15T12:30:15
1980-06-15T12:30,25   =  "
19800615T12.25= 1980-06-15T12:15:00
1980-06-15T12:00.875  = 1980-06-15T12:00:52,5
19660620T06,6 = 1966-06-20T06:40:00

Dates with two-digit year specifiers (YYMMDD) are also accepted following
existing parse-datetime rules: when YY is 69-99, the year is 1969-1999;
when YY is 00-68, the year is 2000-2068), so:

180102T0304   = 2018-01-02T03:04
991231T2300   = 1999-12-31T23:00
190120T030405 = 2019-01-20T03:04:05

Note use of two-digit year specifiers in both basic and extended formats
is allowed in the second edition of ISO 8601 (published in 2000), but it
has been removed as of the third (2004) edition.

Forms no longer accepted as a result of the patch:

1. The current version of parse-datetime already accepts input in ISO
8061 basic format with the " " separator to a limited extent.  It works
with time strings of the form "hh" and "hhmm" but not "hhmmss".
Furthermore, when zone offset is added (e.g. -05:00 or +03, but not zone
names such as Z, UTC, or EST), the results are sometimes accepted, but
are often incorrect. In particular, the time digits are always
interpreted as number of hours, so four digits would be intepreted as
"" not "hhmm".

With the patch, basic format with date-time separator " " can be used
with times of form "hhmmss" (as well as "hh", "hhmm", and "hhmmss.n+") but
the use of zone offset here is uniformly rejected as invalid:

input  current output  output with patch
"20180102 00-03"   2018-01-02T03:00+00 +   (rejected)
"20180102 04-03"   2018-01-02T07:00+00 +   (rejected)
"20180102 0004-03" 2018-01-02T07:00+00 *   (rejected)
"20180102 0004+00" 2018-01-02T04:00+00 *   (rejected)
"20180102 0004Z"   2018-01-02T00:04+00 ++  (rejected)
"20180102 0040-05" (rejected) **   (rejected)
"20180102 0040 EST"2018-01-02T05:40+00 +++ (rejected)

+ correct with two-digit time (hh) and zone offset.
* incorrect with zone offset: 0004 interpreted as 4 h (leading zeroes ignored)
++ correct with zone name: 0004 interpreted as 00:04
** rejected with zone offset: 0040 interpreted as 40 h (invalid hour of day)
+++ correct with zone name: 0040 interpreted as 00:40

With the patch all corresponding expressions with the "T"
separator are accepted and correctly interpreted :

input  current output  output with patch
"20180102T00-03"   (rejected)  2018-01-02T03:00+00
"20180102T04-03"   (rejected)  2018-01-02T07:00+00
"20180102T0004-03" (rejected)  2018-01-02T03:04+00
"20180102T0004+00" (rejected)  2018-01-02T00:04+00
"20180102T0004Z"   (rejected)  2018-01-02T00:04+00
"20180102T0040-05" (rejected)  2018-01-02T05:40+00
"20180102T0040 EST"(rejected)  2018-01-02T05:40+00

2. For inputs of form "(YY)YYMMDD'T'hhmm" the current version of
parse-datetime interprets the "T" as the single-letter military time zone
identifier equivalent to zone offset