Conversation
| } | ||
| #endif | ||
| #if defined(__sun) | ||
| tzset(); |
There was a problem hiding this comment.
Possible to skip above tzset and call it once here?
|
Hey, glad to share the love of jq :) Looks good to me, but i lack autotools knowledge to review those parts. Hopefully some other maintainer can shim in. |
| else | ||
| dt1=$(LC_ALL=$l date +'%a %d %b %Y at %H:%M:%S') | ||
| dt2=$(LC_ALL=$l jq -nr 'now | strflocaltime("%a %d %b %Y at %H:%M:%S")') | ||
| dt2=$(LC_ALL=$l $JQ -nr 'now | strflocaltime("%a %d %b %Y at %H:%M:%S")') |
There was a problem hiding this comment.
I'd like to cherry-pick this change in separate PR.
|
In my honest opinion as one of the active maintainers, we can't officially support the architecture that none of the maintainers is familiar with. Adding a job to the CI workflow could be a pain for us, because it blocks everything when it's broken. But fixing the code and build scripts for wider portability is acceptable. Just like I included #3277. We don't ensure the permanent support for the unfamiliar (for us) architecture in the future releases, but if distributors have trouble while building the package, fixing the code in the upstream (us!) is better than distributors patching for their packages. |
|
Hi, thanks for looking into this PR! I do understand your concern with alien OS. Fair enough :) What next? I think that all the patches are non-intrusive and safe for other architectures. If you want me to just remove the the github actions and merge the two |
Yes, please. |
Solaris comes with own make implementation which is does not work with
GNU tools well. Running configure ends with this error:
config.status: error: Something went wrong bootstrapping makefile fragments
for automatic dependency tracking. If GNU make was not used, consider
re-running the configure script with MAKE="gmake" (or whatever is
necessary). You can also try re-running configure with the
'--disable-dependency-tracking' option to at least be able to build
the package (albeit without support for automatic dependency tracking).
|
Removed github actions, but kept calling twice the |
| for colors in '/' '[30' '30m' '30:31m:32' '30:*:31' 'invalid'; do | ||
| JQ_COLORS=$colors \ | ||
| export JQ_COLORS=$colors | ||
| $JQ -Ccn '[{"a":true,"b":false},"abc",123,null]' > $d/color 2>$d/warning |
There was a problem hiding this comment.
Sorry to nit pick, unindent this line.
There was a problem hiding this comment.
Well, what's the intention of this change? Why the other tests (Set non-default colors, complex input) works?
There was a problem hiding this comment.
I do not mind nitpicking. At least you have a chance of doing thing properly and most importantly only once :)
If I remove this chagne the test fails:
+ 1> /tmp/jqA4UUyb/color 2> /tmp/jqA4UUyb/warning
+ cmp /tmp/jqA4UUyb/color /tmp/jqA4UUyb/expect
+ cat /tmp/jqA4UUyb/warning
+ JQ_COLORS=/
Failed to set $JQ_COLORS
+ cat /tmp/jqA4UUyb/expect_warning
Failed to set $JQ_COLORS
+ cmp /tmp/jqA4UUyb/warning /tmp/jqA4UUyb/expect_warning
/tmp/jqA4UUyb/warning /tmp/jqA4UUyb/expect_warning differ: char 1, line 1
It's a little bit difficult to unpack. I have added a cat to show contents of the file. The test here captures both stdout and stderr in separate files. The stderr contains:
+ JQ_COLORS=/
Failed to set $JQ_COLORS
The expected output is:
Failed to set $JQ_COLORS
It does not happen in any other tests because stderr is not caught or the form VAR=VALUE command is not used. This is /usr/bin/sh peculiarity here.
$ exec bash
$ set -x
$ AAA=BBB /bin/true 2> x
$ cat x
$ exec /usr/bin/sh
$ set -x
$ AAA=BBB /bin/true 2> x
$ cat x
+ AAA=BBB
There was a problem hiding this comment.
What shell is /usr/bin/sh? ksh93?
There was a problem hiding this comment.
Yeah, I see the difference.
$ ksh93 -c 'set -x; AAA=BBB true 2> /tmp/t; cat /tmp/t'
+ true
+ 2> /tmp/t
+ cat /tmp/t
+ AAA=BBB
$ bash -c 'set -x; AAA=BBB true 2> /tmp/t; cat /tmp/t'
+ AAA=BBB
+ true
+ cat /tmp/t
$ dash -c 'set -x; AAA=BBB true 2> /tmp/t; cat /tmp/t'
+ AAA=BBB true
+ cat /tmp/t- run tools from /usr/gnu - disable color testing on Solaris - change the way JQ_COLORS is exported to the process First two poinst are self-explanatory. As for JQ_COLORS, the script originally used this form: JQ_COLORS=blah $JQ ... 2>...warning It runs $JQ and compares stderr with expected value. On Solaris the 'warning' file contains also the variable being set. $ cat warning + JQ_COLORS=/ Failed to set $JQ_COLORS Exporting it in advance makes the tests succeed even on Solaris.
On Solaris, strptime clears the tm structure before parsing. This breaks the
sentinel logic in builtin.c, which then incorrectly assumes that strptime set
tm_wday and tm_yday. Defining _STRPTIME_DONTZERO disables that behavior.
As described in strptime(3C):
o If _STRPTIME_DONTZERO is not defined, the tm struct is ze-
roed on entry and strptime() updates the fields of the tm
struct associated with the specifiers in the format string.
o If _STRPTIME_DONTZERO is defined, strptime() does not zero
the tm struct on entry. Additionally, for some specifiers,
strptime() will use some values in the input tm struct to
recalculate the date and re-assign the appropriate members
of the tm struct.
Solaris prefers the global timezone similarly to Apple. But in addition needs a 'tzset' call in order to recognise that the environment was changed. Without this change shtests fails with: + TZ=Europe/Paris + r='2024-11-14 23:35:41 +0100 CET' + [ '2024-11-14 23:35:41 +0100 CET' '!=' '2024-11-14 23:35:41 +0000 UTC' ] + [ '2024-11-14 23:35:41 +0100 CET' '!=' '2024-11-14 23:35:41 +0000 GMT' ] + echo 'Incorrectly formatted universal time' Incorrectly formatted universal time
Oh sorry now i see, didn't understand this was part of temp changing TZ to UTC |
|
Thanks for your patience :) |
I love jq :) So while updating the version we ship in Solaris I have tried to modify stuff to make everything work out of the box. Just
I also went ahead and created Solaris test action should you be interested in having it too. The changes are not extensive and I have tried to comment them nicely in each commit. Hopefully that's enough.
Thanks for jq!