Merge old jqlang/jq master to the latest

[owenthereal]

After the transfer from stedolan/jq to jqlang/jq, some commits are missing that divert from the older master of stedolan/jq. This is to pull them in: #2594 (comment)

[owenthereal]

cc: @stedolan @nicowilliams

[stedolan]

Something seems to be wrong with the github action definitions here: the windows one failed with an error and the unix ones have hung.

[itchyny]

The action workflow does not trigger because the runner ubuntu-18.04 is unavailable since 2022-12-01 April 2023 (ref). Let's use ubuntu-22.04, or ubuntu-latest.

[owenthereal]

@stedolan @itchyny @nicowilliams I got the Linux builds going with no problems. But the macos build failed with https://github.com/jqlang/jq/actions/runs/5114162555/jobs/9194126677#step:6:3160. Do you have any insight into what goes wrong? The Linux clang builds ran okay, though.

[wader]

That is a bit strange, error seems to be:

posix.c:94:3: error: implicit declaration of function 'onig_end' is invalid in C99 [-Werror,-Wimplicit-function-declaration]

There is an issue related to it #2381 which seems to indicate it has been fixed in newer version of oniguruma. But why did the linux build not complain about it? i would have guessed linux (with gcc?) would complain and fail also on -Wimplicit-function-declaration?

[wader]

Linux build also complains:

posix.c:94:3: warning: implicit declaration of function 'onig_end' is invalid in C99 [-Wimplicit-function-declaration]

But does not use -Werror for some reason. Both linux and macos build uses clang btw.

[owenthereal]

@wader:

There is an issue related to it #2381 which seems to indicate it has been fixed in newer version of oniguruma

Thank you! It's an issue with oniguruma's missing onig_end function in the header! I pinned oniguruma to the suggested revision 5a24a49d710a9e3bb8ff11d12e1eae5a9f9df40c as indicated in #2381, and the macos builds are now passing! Weirdly, the Linux clang build is warning while the macos clang build is erroring...

[wader]

@owenthereal Some addition info: i can reproduce on macOS 13.3.1 with clang 14.0.3 (clang-1403.0.22.14.1) and the error happens during make check in one of the oniguruma tests.

But i agree that update is probably the best solution, still confused why macOS clang uses -Werror and Linux does not.

[wader]

A patch like this seems to fix the \r issue but it would be nice to know why it worked before on windows CI?

diff --git a/src/lexer.l b/src/lexer.l
index 6b9164b..28d281e 100644
--- a/src/lexer.l
+++ b/src/lexer.l
@@ -123,7 +123,7 @@ struct lexer_param;
 ([a-zA-Z_][a-zA-Z_0-9]*::)*[a-zA-Z_][a-zA-Z_0-9]*  { yylval->literal = jv_string(yytext); return IDENT;}
 \.[a-zA-Z_][a-zA-Z_0-9]*  { yylval->literal = jv_string(yytext+1); return FIELD;}

-[ \n\t]+  {}
+[ \r\n\t]+  {}

 . { return INVALID_CHARACTER; }

(could possibly also include \f\v which seems to be what gojq do)

[wader]

It would be great if someone could have a look at the pow/log2 issue, i haven't started yet

[itchyny]

I don't be surprised log2(pow2(x)) is not exactly equal to x on computer due to floating-point number precision. We can just round the result or assert the absolute difference is small enough.

[wader]

I don't be surprised log2(pow2(x)) is not exactly equal to x on computer due to floating-point number precision. We can just round the result or assert the absolute difference is small enough.

Good idea, let's do that. Maybe a mystery for later to figure what differs from the old build env.

@owenthereal sounds good to you?

[wader]

@itchyny any ideas about the \r issue? just use gitattributes for now or should we fix something? as \n and \r\n do work (would be weird otherwise, but just \r don't) maybe this is limited to the jq --run-tests parser code somehow, haven't checked yet.

[itchyny]

Using gitattributes until fixing the lexer sounds good. I think the defaults of core.autocrlf are different between AppVeyor and actions/checkout.

[owenthereal]

I don't be surprised log2(pow2(x)) is not exactly equal to x on computer due to floating-point number precision. We can just round the result or assert the absolute difference is small enough.

Sounds good to me. Does it require some changes to the test and assertion?

Using gitattributes until fixing the lexer sounds good. I think the defaults of core.autocrlf are different between AppVeyor and actions/checkout.

I found actions/checkout#135. I followed the recommendation to turn on LG line endings for all text: 93465d5

[wader]

Sounds good to me. Does it require some changes to the test and assertion?

Yeap something like this should work i think:

diff --git a/tests/jq.test b/tests/jq.test
index 2d5c36b..9aa435a 100644
--- a/tests/jq.test
+++ b/tests/jq.test
@@ -1565,7 +1565,7 @@ try (1%.) catch .
 jq: error: Division by zero? at <top-level>, line 1:

 # Basic numbers tests: integers, powers of two
-[range(-52;52;1)] as $powers | [$powers[]|pow(2;.)|log2] == $powers
+[range(-52;52;1)] as $powers | [$powers[]|pow(2;.)|log2|round] == $powers
 null
 true

You can run the tests with jq --run-tests < tests/jq.test

[owenthereal]

Weeee....CI is passing finally! Thank you all for the help! Would you happen to have any more feedback? If not, let's merge. At the moment, the CI only compiles & runs tests. A follow-up PR is needed to publish releases to GH Releases.

[wader]

No worries! happy to help and i learned some windows stuff :) Yeap let's merge! the things i'm don't know/are worried about is publish and signing windows binaries (are they built "correctly" etc?).

[wader]

🥳