Feature: tools: make crm_{resource,rule,simulation} accept "free form" date/time format #1756

jnpkrn · 2019-04-23T23:27:26Z

Considerations are, in all honesty, not that straightforward for
anything but the new tools, since they are entirely new interfaces,
and anything else (iso8601 tool?) might provoke backward-incompatible
dealings with older tools (libraries and data-based interfaces are
explicitly exempted for obvious reasons). New library function is
in order regardless.

Example:

env LANG=en_US CIB_file=cts/scheduler/date-1.xml ./tools/crm_rule -c
-r rule.auto-1 -d "$(LANG=en_US date -d "13 years ago")"

jnpkrn · 2019-04-24T07:47:03Z

Alternative is to use downright what coreutils' date uses to allow
for all the supported expressiveness ncl. tomorrow etc.):

https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob_plain;f=modules/parse-datetime

The more I think about that the more I like it!

kgaillot · 2019-04-24T19:22:29Z

lib/common/iso8601.c

+    /* _DATE_FMT is what coreutils' date uses internally */
+            || strptime(date_time, nl_langinfo(_DATE_FMT), &tm_spec) != NULL
+#endif
+    ) {


strptime() doesn't accept a NULL date_time, so handle that first

I see, it happened accidentally, the order was flipped origially
(so the NULL check was handled as a side effect of that), but I
had to change that because of overly verbose complaints when it
didn't match the required strict format and didn't feel like
fighting with the system at the moment.

I now see that Gnulib needs the same treatment, good that you
spotted that, thanks.

kgaillot · 2019-04-24T19:33:39Z

What advantage does the approach in this PR have over using date to convert to ISO 8601? E.g.

CIB_file=cts/scheduler/date-1.xml ./tools/crm_rule -c -r rule.auto-1 -d "$(date --iso-8601 -d "13 years ago")"

Re: parse-datetime, see also g_date_set_parse() -- not sure what would be involved in making it usable for our purposes, but it would avoid more bundled code

jnpkrn · 2019-04-24T21:33:50Z

As mentioned on the list, ISO format is not user friendly.
Moreover, the exact standard is rather proprietary, which kind of
highlights the unfriendliness...

Coreutil's date is not everywhere.

Had a look at g_date_set_parse and it's rather poor compared to
Gnulib. Doesn't support simple relative terms, for instance.

Anyway, I believe I have just the last roadblock to tackle with
Gnulib (after quite some I'm already behind), so stay tuned.

jnpkrn · 2019-04-25T07:57:09Z

Moreover, as you may see, g_date_set_parse deals just with days
granularity, which is just a half of the functionality we require.

If you are looking at dropping some code, it's not very clear
why lib/common/iso8601.c came into existence in the first place
since g_date_time_new_from_iso8601 appears to do what we need.
Hence, most of lib/common/iso8601.c would be in the front line
of the code to say good bye to for good :-)

jnpkrn · 2019-04-25T08:14:55Z

Btw. good that our "control characters" check (323330b) indeed
spotted ^L (\f, form feed):

looking for presence of control characters...
lib/gnu/parse-datetime.c:2893:^L
lib/gnu/parse-datetime.y:1170:^L

with, e.g. (as observed in a proper editor):

2891 };
2892 
2893 ^L
2894 
2895 /* Convert a time zone expressed as HH:MM into an integer count of

Indeed, these characters are nowhere to be seen in the web view of GH,
eroding further the usefulness of any such browse-by reviews, so good
that it was spotted. We may decide to exempt \f at any time, e.g.,
when it is deemd good for code folding, but for now, the solution would
be to get rid of that occcurrence, I think.

kgaillot · 2019-04-25T14:08:40Z

Moreover, as you may see, g_date_set_parse deals just with days
granularity, which is just a half of the functionality we require.

If you are looking at dropping some code, it's not very clear
why lib/common/iso8601.c came into existence in the first place
since g_date_time_new_from_iso8601 appears to do what we need.
Hence, most of lib/common/iso8601.c would be in the front line
of the code to say good bye to for good :-)

Ah, I see: all of the GDateTime functions became available in glib 2.26.0 (2011-01-23), and we currently require only 2.16.0. GDateTime supports microseconds as well. I'll put it on the roadmap to investigate switching at the next big API overhaul.

jnpkrn · 2019-04-25T14:27:03Z

On 25/04/19 07:08 -0700, Ken Gaillot wrote: > Moreover, as you may see, `g_date_set_parse` deals just with days > granularity, which is just a half of the functionality we require. > > If you are looking at dropping some code, it's not very clear > why `lib/common/iso8601.c` came into existence in the first place > since `g_date_time_new_from_iso8601` appears to do what we need. > Hence, most of `lib/common/iso8601.c` would be in the front line > of the code to say good bye to for good :-) Ah, I see: all of the GDateTime functions became available in glib 2.26.0 (2011-01-23), and we currently require only 2.16.0. GDateTime supports microseconds as well. I'll put it on the roadmap to investigate switching at the next big API overhaul.

We are not meeting on the subject, still. GDateTime has no such best-effort parsing function, so what you talk about is entirely orthogonal unless I miss something.

…

-- Jan (Poki)

kgaillot · 2019-04-25T14:36:39Z

If you are looking at dropping some code, it's not very clear
why lib/common/iso8601.c came into existence in the first place since
g_date_time_new_from_iso8601 appears to do what we need.

^^^ = GDateTime

Ah, I see: all of the GDateTime functions became available in glib 2.26.0 (2011-01-23),
and we currently require only 2.16.0. GDateTime supports microseconds as well. I'll put
it on the roadmap to investigate switching at the next big API overhaul.

We are not meeting on the subject, still. GDateTime has no such best-effort parsing function, so what you talk about is entirely orthogonal unless I miss something.

jnpkrn · 2019-04-25T14:49:47Z

On 25/04/19 07:36 -0700, Ken Gaillot wrote: >>> If you are looking at dropping some code, it's not very clear >>> why `lib/common/iso8601.c` came into existence in the first place since >>> `g_date_time_new_from_iso8601` appears to do what we need. ^^^ = GDateTime >> Ah, I see: all of the GDateTime functions became available in glib 2.26.0 (2011-01-23), >> and we currently require only 2.16.0. GDateTime supports microseconds as well. I'll put >> it on the roadmap to investigate switching at the next big API overhaul. > We are not meeting on the subject, still. GDateTime has no such > best-effort parsing function, so what you talk about is entirely > orthogonal unless I miss something.

Oh, I see. Let's keep that strictly separate, since it won't help us with the intention at hand. POSIX-mandated structs are always the least common denominator assuring interoperatibility regardless of which tooling (hand-made wheel/existing implementation) is used.

…

-- Jan (Poki)

jnpkrn · 2019-04-25T20:07:01Z

Refreshed the snowballed mono-commit if anyone wants to play with it,
it slowly gets finalized, once it happens, will be cut down to rather
self-contained steps.

jnpkrn · 2019-04-25T20:17:40Z

Note that along our already traditional backfiring, nonsensical burden
like occupying CFLAGS (normally user-reserved) unprecedently keeps
complicating the progress.

Perhaps build-system overhaul is in order (see also
#1701 (comment))

jnpkrn · 2019-05-02T20:53:22Z

Cut that into better manageable pieces, though not sure if it'll
compile at this form:

there would have to be an implicit rule for YACC files to C
(EDIT: likely there isn't, we need to add recipe for sure)
some gnulib original code changed slightly in the interim

But now, it at least feels more flexible regarding modifications.

jnpkrn · 2019-05-07T14:37:42Z

Hmm, so the strategy of giving native (broken) code a chance first,
and only then to conditionally fall back to parse-datetime module
is not working out either (that's why the CI breaks now).

jnpkrn · 2019-05-07T15:56:28Z

It turned out that we were "lucky" enough that initial 'T' from
today's free form date specification (Tue 07 May 2019 ...)
triggered another bug in pacemaker's implementation of parsing.
Put a soft fix (only applied when there's an actual fallback
~ tolerant == true) in place.

jnpkrn · 2019-05-07T19:06:28Z

Uff, it wasn't working on FreeBSD, owing to the fact I built the
build-time integration on pre-existing ill-advised approach -- put
some more love to integration with Gnulib in generally, hopefully
it will help.

EDIT: yep, current approach works with FreeBSD as well \o/

That's by the means of a new, internal do_parse_date wrapper that takes whether to be "tolerance" as an additional flag, which is then propagated over the control paths to wherever it may be have any effect, currently boiling down just to time_check for which a likewise internal-only counterpart do_time_check is added. And regarding "tolerance" itself, it is meant to be applied in tool (as opposed to daemon) mode only, to prevent spurious complaints in case there's, e.g., a more generic parser queued as a fallback (something to arrive within this patch series), so any problems encountered during this first shot do not matter at all. There refactorings are not pretty, although, it's expected the overall (and at places buggy) date/time parse/convert/format module will eventually get ditched for redundancy with GDateTime of glib2 (https://developer.gnome.org/glib/stable/glib-GDateTime.html) hence shall be "tolerated" on their own. Along the way, fix a const-correctness (crm_time_check) and ineffecient not-empty-string check ([do_]parse_date). Also, make the "tolerant" mode actually stop early at crm_time_parse, since that's exactly what's needed so as to give the fallback parser a chance at all (rather than to live with totally mis-parsed value in such case -- case in point, initial 'T' in "Tue 07 May" would previously be recognized as a date-time separator per ISO 8601).

This is a forerunner of a rather fat annex of new modules to borrow from gnulib, so we'd rather start with cutting of what we expressly don't require. Note that md5.c code used to be consumed through a custom recipes in lib/common/Makefile.am, rather than deferring to Gnulib's own recipes, which is now changed as well.

As a consequence of: https://lists.gnu.org/archive/html/bug-gnulib/2016-10/msg00045.html we cannot consume parse-datetime module right away since we are strict on LGPLv2+ license boundaries. The solution for us is hence to obtain at least the the so-licenced prerequisite modules by the standard means of gnulib-tool as already established, and to grab its actual constituting source (parse-datetime.[hy]) directly from checkout. For the missing (non-LGPLv2+'d) prerequisite modules, we establish reasonable surrogates (that's why those files pop up in te stats): * strftime (strftime.[ch]) - strftime.h surrogate header that defers directly to libc for strftime() * timespec (timespec.[ch]) - transitive dependency via gettime module, for which we inject gettime() wrapper over clock_gettime directly in parse-datetime.h - just an empty timespec.h is tracked then instead * xalloc (xalloc.[ch]) - xalloc.h surrogate header that defers to g_malloc/g_memdup Finally, it's to be noted that the situation appeared to be untractable like this after following monumental (in terms of seemingly untwistable dependency on non-LGPLv2+'d module time_r) update to parse-datime: https://lists.gnu.org/archive/html/bug-gnulib/2017-01/msg00133.html so consuming the state of the source right prior to that change seemed a pragmatic and good enough solution, modulo: * a trivial quick fix we replay artifically with a sed one-linet: https://lists.gnu.org/archive/html/bug-gnulib/2017-04/msg00028.html * similarly applied removal of a form-feed character from parse-datetime.y that would raise an alarm in Travis CI (MAINT=1) character sanity checking arrangement otherwise

Continuing on the previous integration work, we now add pcmk__time_new_versatile() as a (private) clone of its libcrmcommon's sibling crm_time_new() -- the new function offers the same interface, but accepts arbitrary free forms expressing date/time specification that parse-datetime code of gnulib (added with the previous commit) is able to handle. Further integration to the actual tools is forthcoming at this point. We now hence also add an optional build dependency on bison (yacc could also be workable, but needs a sed one-liner around union definition, IIRC).

Building upon our previous integration work, we can now finally swap crm_time_new() in crm_{rule,simulate} tools for pcmk__time_new_versatile() where applicable, so as to support arbitrary free forms expressing date/time specification on such eligible platforms -- this is quite a leap regarding the friendliness towards users, since now they get what they may be used to with coreutils' date(1) implementation. Also add a few tests to exercise this new input possibilities, incl. fixing some thinkos in the tests themselves (at least for rules).

In addition to pre-existing interface to specify the lifespan of the newly created constraints as "period from now" (using -u), we proceed to add a similar interface we've just added to crm_{rule,simulate}, that is, ability to specify such end point in absolute terms (until date/time X, using "--lifetime-until X"), and likewise, this specification can be of arbitrary free form parseable with pcmk__time_new_versatile() on systems where supported. Also add a few tests to exercise this new input possibilities.

In particular, unified with crm_{resource,rule,simulate} that underwent such changes along teaching them to understand date/time free forms (which is not appropriate in the context of this tool, it seems -- that can be reconsidered later, though). Also fix a handful of typos and cosmetic discrepancies, incl. suggestions of actually ISO 8601 non-conforming date/time formats.

Now that all compiled CLI tools supporting date/time inputs are adjusted to support that, and since it's rather platfom specific, care to advertise the support by standard means.

jnpkrn · 2019-05-09T12:58:33Z

Realized that X_time_new_versatile shall rather be marked as very
internal ... that symbol won't be present where the new feature is not
compiled in, and only within-the-same-build binaries can rely on that
information to be consistent for them, meaning that only these internal
consumers shall be allowed to use it to begin with.

jnpkrn · 2019-06-10T20:20:50Z

Note: this will also, as a side-effect, fix
https://bugs.clusterlabs.org/show_bug.cgi?id=5390

Instead of md5.c being built through a custom recipe in lib/common/Makefile.am, use gnulib's own recipes. This was originally written by Jan Pokorný as part of ClusterLabs#1756 but I've made a couple changes. * The gnulib updating to pick up the various other Makefile.am and m4 changes is going to happen in a separate commit. * We cannot drop 00gnulib.m4. This file appears to accumulate fixes that will eventually make their way into autoconf. It grows and shrinks over time. At the moment, it contains fixes for references to gl_COMPILER_CLANG that we'll need.

Now that we are using gnulib's own build stuff, including libtool, we need to update various macros to reflect those changes. These changes would also be done by running "make gnulib-update", but that would also update all the other gnulib stuff which seems like too much for the moment. It's easy enough to apply by hand. This was originally written by Jan Pokorný as part of ClusterLabs#1756.

Instead of md5.c being built through a custom recipe in lib/common/Makefile.am, use gnulib's own recipes. This was originally written by Jan Pokorný as part of ClusterLabs#1756 but I've made a couple changes. * The gnulib updating to pick up the various other Makefile.am and m4 changes is going to happen in a separate commit. * We cannot drop 00gnulib.m4. This file appears to accumulate fixes that will eventually make their way into autoconf. It grows and shrinks over time. At the moment, it contains fixes for references to gl_COMPILER_CLANG that we'll need. * Keep the /lib/gnu/libgnu.a ignore line in the "Formerly built" section. * Don't do the separate dependency for maint/gnulib/.git. We are always only running gnulib-update manually at release time, so there's no need for the extra rule complication.

Now that we are using gnulib's own build stuff, including libtool, we need to update various macros to reflect those changes. These changes would also be done by running "make gnulib-update", but that would also update all the other gnulib stuff which seems like too much for the moment. It's easy enough to apply by hand. This was originally written by Jan Pokorný as part of ClusterLabs#1756.

Instead of md5.c being built through a custom recipe in lib/common/Makefile.am, use gnulib's own recipes. This was originally written by Jan Pokorný as part of ClusterLabs#1756 but I've made a couple changes. * The gnulib updating to pick up the various other Makefile.am and m4 changes is going to happen in a separate commit. * We cannot drop 00gnulib.m4. This file appears to accumulate fixes that will eventually make their way into autoconf. It grows and shrinks over time. At the moment, it contains fixes for references to gl_COMPILER_CLANG that we'll need. * Keep the /lib/gnu/libgnu.a ignore line in the "Formerly built" section. * Don't do the separate dependency for maint/gnulib/.git. We are always only running gnulib-update manually at release time, so there's no need for the extra rule complication.

Now that we are using gnulib's own build stuff, including libtool, we need to update various macros to reflect those changes. These changes would also be done by running "make gnulib-update", but that would also update all the other gnulib stuff which seems like too much for the moment. It's easy enough to apply by hand. This was originally written by Jan Pokorný as part of ClusterLabs#1756.

jnpkrn force-pushed the crm_rule-lang-aware branch 2 times, most recently from d65d4b6 to 4069632 Compare April 24, 2019 07:34

kgaillot reviewed Apr 24, 2019

View reviewed changes

jnpkrn force-pushed the crm_rule-lang-aware branch from 4069632 to 3f33485 Compare April 24, 2019 23:31

jnpkrn changed the title ~~Feature: tools: make crm_rule accept default date/time format for LANG~~ Feature: tools: make crm_rule accept "free form" date/time format Apr 25, 2019

jnpkrn mentioned this pull request Apr 25, 2019

[WIP] Med: libcrmcommon/iso8601: fix train of thinkos regarding time offsets #1759

Closed

jnpkrn force-pushed the crm_rule-lang-aware branch from 3f33485 to 507ef9c Compare April 25, 2019 20:05

jnpkrn force-pushed the crm_rule-lang-aware branch 9 times, most recently from ed36f88 to 40c5edb Compare May 2, 2019 20:51

jnpkrn force-pushed the crm_rule-lang-aware branch 2 times, most recently from 89fb6f0 to e2b937a Compare May 3, 2019 13:30

jnpkrn force-pushed the crm_rule-lang-aware branch from ee0a2b3 to 2164549 Compare May 7, 2019 13:29

jnpkrn force-pushed the crm_rule-lang-aware branch from 2164549 to c91bf14 Compare May 7, 2019 15:48

jnpkrn force-pushed the crm_rule-lang-aware branch 2 times, most recently from d42551e to 738cc17 Compare May 7, 2019 18:59

jnpkrn force-pushed the crm_rule-lang-aware branch 3 times, most recently from 08b95b0 to bb01161 Compare May 8, 2019 18:41

jnpkrn added 10 commits May 9, 2019 14:52

Build: .gitignore: continue slightly in classification of byproducts

e9cb1c1

Build: make generated man pages respect embedded "See also" section

34679bf

Build: configure: announce support of "free form" date/time spec support

e8040ca

Now that all compiled CLI tools supporting date/time inputs are adjusted to support that, and since it's rather platfom specific, care to advertise the support by standard means.

jnpkrn force-pushed the crm_rule-lang-aware branch from bb01161 to e8040ca Compare May 9, 2019 13:03

kgaillot closed this Apr 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: tools: make crm_{resource,rule,simulation} accept "free form" date/time format #1756

Feature: tools: make crm_{resource,rule,simulation} accept "free form" date/time format #1756

jnpkrn commented Apr 23, 2019

jnpkrn commented Apr 24, 2019

kgaillot Apr 24, 2019

jnpkrn Apr 24, 2019 •

edited

kgaillot commented Apr 24, 2019

jnpkrn commented Apr 24, 2019

jnpkrn commented Apr 25, 2019

jnpkrn commented Apr 25, 2019 •

edited

kgaillot commented Apr 25, 2019

jnpkrn commented Apr 25, 2019 via email

kgaillot commented Apr 25, 2019

jnpkrn commented Apr 25, 2019 via email

jnpkrn commented Apr 25, 2019

jnpkrn commented Apr 25, 2019

jnpkrn commented May 2, 2019 •

edited

jnpkrn commented May 7, 2019

jnpkrn commented May 7, 2019

jnpkrn commented May 7, 2019 •

edited

jnpkrn commented May 9, 2019

jnpkrn commented Jun 10, 2019

Feature: tools: make crm_{resource,rule,simulation} accept "free form" date/time format #1756

Feature: tools: make crm_{resource,rule,simulation} accept "free form" date/time format #1756

Conversation

jnpkrn commented Apr 23, 2019

jnpkrn commented Apr 24, 2019

kgaillot Apr 24, 2019

Choose a reason for hiding this comment

jnpkrn Apr 24, 2019 • edited

Choose a reason for hiding this comment

kgaillot commented Apr 24, 2019

jnpkrn commented Apr 24, 2019

jnpkrn commented Apr 25, 2019

jnpkrn commented Apr 25, 2019 • edited

kgaillot commented Apr 25, 2019

jnpkrn commented Apr 25, 2019 via email

kgaillot commented Apr 25, 2019

jnpkrn commented Apr 25, 2019 via email

jnpkrn commented Apr 25, 2019

jnpkrn commented Apr 25, 2019

jnpkrn commented May 2, 2019 • edited

jnpkrn commented May 7, 2019

jnpkrn commented May 7, 2019

jnpkrn commented May 7, 2019 • edited

jnpkrn commented May 9, 2019

jnpkrn commented Jun 10, 2019

jnpkrn Apr 24, 2019 •

edited

jnpkrn commented Apr 25, 2019 •

edited

jnpkrn commented May 2, 2019 •

edited

jnpkrn commented May 7, 2019 •

edited