-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
builtin.inc: fix build with -Woverlength-strings #2979
Conversation
I need to add |
9eaa7b5
to
75d0c92
Compare
I think I need to pass more flags to make it build on non-x86 systems. |
75d0c92
to
903d3e3
Compare
903d3e3
to
b8d276a
Compare
I fixed the |
d052f52
to
d68438c
Compare
d68438c
to
fc81a5b
Compare
Huh strange failure |
I fixed the windows builds, now I will try to remove |
Remembered now that we only run and tests on amd64 https://github.com/jqlang/jq/blob/master/.github/workflows/ci.yml#L168-L172 but it does not correlate with the failured ci jobs hmm. Thinking if gen_builtins is built as cross target binary somehow? |
It works without |
9782323
to
02c8e5b
Compare
02c8e5b
to
5618213
Compare
35c170d
to
70d155a
Compare
49f60fc
to
760bfe8
Compare
🤔 |
I think CIs are all running on amd64, and just cross compiling for a given architecture |
|
We should use an host compiler to build |
Hmm define a |
7480327
to
189586c
Compare
C99 only allows string literals with length up to 4095 characters. This patch introduces a gen_jq_builtins generator program that generates the jq_builtins array in builtin.c as an array literal instead of a string literal to make compilation not fail when building with -Woverlength-strings. Add -Woverlength-strings to CFLAGS to verify that the fix works. Also introduce HOSTCC autoconf variable to get a C compiler to build gen_jq_builtin on the host when cross-compiling. Fixes jqlang#1481
3496186
to
ca71287
Compare
#include <stringapiset.h> | ||
#include <wchar.h> | ||
|
||
#define DEFINE_MAIN(ARGC, ARGV) \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't notice this code before. Is this used when building native for windows, not via emulation layers like msys2 or cygwin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, when you compile with -municode
on windows, wmain()
is called instead of main()
as entry point with wchar_t*
strings instead of char*
strings; then that code in wmain()
converts the wchar_t*
strings to char*
strings encoding with utf-8.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume it is probably an hack to make the program get the correct utf-8 encoding on unicode characters you pass through cmd.exe
because windows encodings are weird.
For example, recently, I added a test to curl
that verifies that utf-8 bytes get correctly encoded to JSON; and I had to modify my test to get the utf-8 string from a file instead of from command line arguemnts because the windows CI was not passing the right bytes to curl
. :/
curl/curl#12434
I didn't anticipate that adding a generator program would be so complex. |
C99 only allows string literals with length up to 4095 characters.
This patch introduces a
gen_jq_builtins
generator program that generates thejq_builtins
array in builtin.c as an array literal instead of a string literal to make compilation not fail when building with-Woverlength-strings
.Also add
-Woverlength-strings
toCFLAGS
to verify that the fix works.Fixes #1481