Skip to content
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

CI: build examples for additional code verification #7922

Closed
wants to merge 2 commits into from

Conversation

mback2k
Copy link
Member

@mback2k mback2k commented Oct 29, 2021

Some CIs already build them, let's do it on more of them.

Follow up to #7690
Replaces #7591

@mback2k mback2k added the CI Continuous Integration label Oct 29, 2021
@mback2k mback2k self-assigned this Oct 29, 2021
@mback2k
Copy link
Member Author

mback2k commented Nov 1, 2021

Is Circle CI actually running for PRs? I can't find it in the listing below or am I just blind? @bagder

@mback2k
Copy link
Member Author

mback2k commented Nov 1, 2021

Quoting @jay from #7591 (comment) for further discussion:

do you have any idea why the examples using fprintf cannot compile on the failed Azure DevOps builds?

It looks like warnings are treated as errors but I don't know why it's an error for just the examples. I found this post from you many years ago, and maybe we can get around it with -Wno-pedantic-ms-format

Thanks. I just checked and found that -Wno-pedantic-ms-format is actually being used to build the examples. The error message is also a little different this time:

2021-10-29T20:45:35.3132082Z depbase=`echo anyauthput.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
2021-10-29T20:45:35.3137364Z i686-w64-mingw32-gcc -DHAVE_CONFIG_H   -I../../include -DCURL_NO_OLDIES  -isystem C:/msys64/mingw32/include -isystem C:/msys64/mingw32/include -isystem C:/msys64/mingw32/include  -Werror-implicit-function-declaration -g -O0 -pedantic -Wall -W -Wpointer-arith -Wwrite-strings -Wunused -Wshadow -Winline -Wnested-externs -Wmissing-declarations -Wmissing-prototypes -Wno-long-long -Wbad-function-cast -Wfloat-equal -Wno-multichar -Wsign-compare -Wundef -Wno-format-nonliteral -Wendif-labels -Wstrict-prototypes -Wdeclaration-after-statement -Wold-style-definition -Wstrict-aliasing=3 -Wcast-align -Wtype-limits -Wold-style-declaration -Wmissing-parameter-type -Wempty-body -Wclobbered -Wignored-qualifiers -Wconversion -Wno-sign-conversion -Wvla -ftree-vrp -Wno-pedantic-ms-format -Wdouble-promotion -Wformat=2 -Warray-bounds=2 -Wshift-negative-value -Wshift-overflow=2 -Wnull-dereference -fdelete-null-pointer-checks -Wduplicated-cond -Wunused-const-variable -Wduplicated-branches -Wrestrict -Walloc-zero -Wformat-overflow=2 -Wformat-truncation=2 -Wimplicit-fallthrough=4 -Wno-system-headers -Wenum-conversion -Werror -pedantic-errors -MT anyauthput.o -MD -MP -MF $depbase.Tpo -c -o anyauthput.o anyauthput.c &&\
2021-10-29T20:45:35.3148223Z mv -f $depbase.Tpo $depbase.Po
2021-10-29T20:45:36.3328869Z anyauthput.c: In function 'read_callback':
2021-10-29T20:45:36.3329906Z anyauthput.c:93:19: error: ISO C does not support the 'I' printf flag [-Werror=format=]
2021-10-29T20:45:36.3330688Z    93 |   fprintf(stderr, "*** We read %" CURL_FORMAT_CURL_OFF_T
2021-10-29T20:45:36.3331692Z       |                   ^~~~~~~~~~~~~~~
2021-10-29T20:45:36.3332535Z anyauthput.c:93:19: error: format '%d' expects argument of type 'int', but argument 3 has type 'curl_off_t' {aka 'long long int'} [-Werror=format=]
2021-10-29T20:45:36.3334204Z In file included from ../../include/curl/curl.h:35,
2021-10-29T20:45:36.3334847Z                  from anyauthput.c:32:
2021-10-29T20:45:36.3335536Z ../../include/curl/system.h:199:42: note: format string is defined here
2021-10-29T20:45:36.3336240Z   199 | #  define CURL_FORMAT_CURL_OFF_T     "I64d"
2021-10-29T20:45:36.3949522Z cc1.exe: all warnings being treated as errors
2021-10-29T20:45:36.4188318Z make[2]: *** [Makefile:1813: anyauthput.o] Error 1
2021-10-29T20:45:36.4189142Z make[2]: *** Waiting for unfinished jobs....
2021-10-29T20:45:36.6744255Z make[2]: Leaving directory '/c/__w/1/s/docs/examples'
2021-10-29T20:45:36.6756852Z make[1]: *** [Makefile:1928: check-am] Error 2
2021-10-29T20:45:36.6759935Z make[1]: Leaving directory '/c/__w/1/s/docs/examples'
2021-10-29T20:45:36.6848015Z make: *** [Makefile:1741: examples] Error 2
2021-10-29T20:45:36.7881488Z ##[error]Cmd.exe exited with code '2'.
2021-10-29T20:45:36.8831887Z ##[section]Finishing: compile

So the error message mentions '%d' , but then points to a definition of CURL_FORMAT_CURL_OFF_T as "I64d".

I think something is off with how the typedef's are done if used from the examples. This looks like a mismatch to me.

@mback2k mback2k requested a review from jay November 1, 2021 11:33
@mback2k
Copy link
Member Author

mback2k commented Nov 1, 2021

Another interesting find: the error message does not appear in the Cirrus Windows builds which are using the same Docker images as the Azure Windows builds, therefore use the identical toolchain. The only difference is that the Cirrus Windows builds are release and not debug builds.

@jay
Copy link
Member

jay commented Nov 1, 2021

This looks like #6079, which was solved by ignoring -Wformat. If it's a bug in gcc 10 then one of us should report it. I don't have mingw-w64 gcc 10 but I just tried mingw-w64 gcc 9 and I don't get the error unless I exclude -Wno-pedantic-ms-format.

mback2k added a commit to mback2k/curl that referenced this pull request Nov 1, 2021
Some CIs already build them, let's do it on more of them.

Ignore -Wformat while using GCC on mingw to compile examples.

Ref: curl#6079
Ref: curl#6082
Assisted-by: Jay Satiro

Follow up to curl#7690
Replaces curl#7591
Closes curl#7922
@mback2k
Copy link
Member Author

mback2k commented Nov 1, 2021

Thanks for digging this up @jay. Let's see what happens if I put the same workaround into the examples, although I am still wondering why the Cirrus CI builds are not affected without it.

It also looks like this is mostly affecting external code, because curl internal code mostly uses the curlx_* versions of printf*. For example tool_writeout.c uses curlx as a workaround while tool_filetime.c uses @bagder's pragma-based workaround. I guess a follow up PR could be to make tool_filetime.c also use curlx instead, right?

@mback2k mback2k requested a review from bagder November 1, 2021 20:50
@mback2k
Copy link
Member Author

mback2k commented Nov 1, 2021

Regarding the follow up PR mentioned above. I just created a draft one, since the pragma is now obsolete due to #6535.

mback2k added a commit to mback2k/curl that referenced this pull request Nov 5, 2021
Some CIs already build them, let's do it on more of them.

Ignore -Wformat while using GCC on mingw to compile examples.

Ref: curl#6079
Ref: curl#6082
Assisted-by: Jay Satiro

Follow up to curl#7690
Replaces curl#7591
Closes curl#7922
@mback2k mback2k force-pushed the ci-build-examples branch 2 times, most recently from 0c429a8 to f784edc Compare November 5, 2021 20:51
mback2k added a commit to mback2k/curl that referenced this pull request Nov 5, 2021
Some CIs already build them, let's do it on more of them.

Ignore -Wformat while using GCC on mingw to compile examples.

Ref: curl#6079
Ref: curl#6082
Assisted-by: Jay Satiro

Follow up to curl#7690
Replaces curl#7591
Closes curl#7922
mback2k added a commit to mback2k/curl that referenced this pull request Nov 6, 2021
Some CIs already build them, let's do it on more of them.

Ignore -Wformat while using GCC on mingw to compile examples.

Ref: curl#6079
Ref: curl#6082
Assisted-by: Jay Satiro

Follow up to curl#7690
Replaces curl#7591
Closes curl#7922
mback2k added a commit to mback2k/curl that referenced this pull request Nov 6, 2021
Some CIs already build them, let's do it on more of them.

Ignore -Wformat while using GCC on mingw to compile examples.

Ref: curl#6079
Ref: curl#6082
Assisted-by: Jay Satiro

Follow up to curl#7690
Replaces curl#7591
Closes curl#7922
mback2k added a commit to mback2k/curl that referenced this pull request Nov 7, 2021
Some CIs already build them, let's do it on more of them.

Ignore -Wformat while using GCC on mingw to compile examples.
Improve size_t to long conversion in imap-append.c example.

Ref: curl#6079
Ref: curl#6082
Assisted-by: Jay Satiro

Follow up to curl#7690
Replaces curl#7591
Closes curl#7922
mback2k added a commit to mback2k/curl that referenced this pull request Nov 8, 2021
Some CIs already build them, let's do it on more of them.

Ignore -Wformat while using GCC on mingw to compile examples.
Improve size_t to long conversion in imap-append.c example.

Ref: curl#6079
Ref: curl#6082
Assisted-by: Jay Satiro

Follow up to curl#7690
Replaces curl#7591
Closes curl#7922
@mback2k
Copy link
Member Author

mback2k commented Nov 8, 2021

@bagder I don't have the time now, but you may wanna investigate this error that shows up in AppVeyor for master and this PR:

3>c:\projects\curl\src\tool_operate.c(889) : error C2220: warning treated as error - no 'object' file generated
3>c:\projects\curl\src\tool_operate.c(889) : warning C4701: potentially uninitialized local variable 'per' use

bagder added a commit that referenced this pull request Nov 8, 2021
tool_operate.c(889) : warning C4701: potentially uninitialized local
variable 'per' use

Follow-up to cc71d35
Reported-by: Marc Hörsken
Bug: #7922 (comment)
bagder added a commit that referenced this pull request Nov 8, 2021
tool_operate.c(889) : warning C4701: potentially uninitialized local
variable 'per' use

Follow-up to cc71d35
Reported-by: Marc Hörsken
Bug: #7922 (comment)
bagder added a commit that referenced this pull request Nov 8, 2021
tool_operate.c(889) : warning C4701: potentially uninitialized local
variable 'per' use

Follow-up to cc71d35
Reported-by: Marc Hörsken
Bug: #7922 (comment)
Closes #7971
mback2k added a commit to mback2k/curl that referenced this pull request Nov 9, 2021
Some CIs already build them, let's do it on more of them.

Ignore -Wformat while using GCC on mingw to compile examples.
Improve size_t to long conversion in imap-append.c example.

Ref: curl#6079
Ref: curl#6082
Assisted-by: Jay Satiro

Follow up to curl#7690
Replaces curl#7591
Closes curl#7922
@mback2k mback2k marked this pull request as ready for review November 10, 2021 21:33
@mback2k
Copy link
Member Author

mback2k commented Nov 10, 2021

@bagder please confirm with another approval if you are fine with adding the pragma to the relevant examples, thanks.

Copy link
Member

@bagder bagder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the added pragma lines are unfortunate since the examples are meant to contain as little "extra fluff" as possible. I assume you haven't figured out any other way to work around the problems you get without them? Can we remove/change the printf lines instead?

mback2k added a commit to mback2k/curl that referenced this pull request Dec 11, 2021
Some CIs already build them, let's do it on more of them.

Ignore -Wformat while using GCC on mingw to compile examples.
Improve size_t to long conversion in imap-append.c example.

Ref: curl#6079
Ref: curl#6082
Assisted-by: Jay Satiro

Follow up to curl#7690
Replaces curl#7591
Closes curl#7922
@mback2k
Copy link
Member Author

mback2k commented Dec 11, 2021

@bagder I just tried to update the relevant printf lines by using unsigned long instead of curl_off_t.

@mback2k mback2k requested a review from bagder December 11, 2021 16:12
Some CIs already build them, let's do it on more of them.

Ignore -Wformat while using GCC on mingw to compile examples.
Improve size_t to long conversion in imap-append.c example.

Ref: curl#6079
Ref: curl#6082
Assisted-by: Jay Satiro

Follow up to curl#7690
Replaces curl#7591
Closes curl#7922
@mback2k
Copy link
Member Author

mback2k commented Dec 13, 2021

Rebased again after @bagder simplified progressfunc.c with e75bc3a.

mback2k added a commit that referenced this pull request Dec 14, 2021
Avoid CURL_FORMAT_CURL_OFF_T by using unsigned long instead.
Improve size_t to long conversion in imap-append.c example.

Ref: #6079
Ref: #6082
Assisted-by: Jay Satiro
Reviewed-by: Daniel Stenberg

Preparation of #7922
@mback2k mback2k closed this in f03cc1b Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration hacktoberfest-accepted
Development

Successfully merging this pull request may close these issues.

None yet

3 participants