CI: Check compiler warnings.

[junaruga]

This PR fixes #626.

Add checks to make CI fail by compiler warnings in the rake compile.

If the skip-warnings (default: false, as an undefined variable is evaluated as false in the if syntax) is true in specific matrix cases, the cases skip the checks. If you want to skip new compiler warnings coming from external changes such as upgraded compiler or OpenSSL versions in the specific matrix cases, you can set the skip-warnings: true for the cases.

Note that this issue #628 was detected in the process of this work.

[rhenium]

This already revealed problems that currently exist in master. From your comment at #615 (comment):

• ubuntu-22.04 2.6: There are warnings. But these are not new by this PR.
  ../../../../ext/openssl/ossl_ssl.c: In function ‘ossl_start_ssl’: ../../../../ext/openssl/ossl_ssl.c:1761:5: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] 1761 | VALUE io = rb_attr_get(self, id_i_io);

C99 syntax was not allowed before Ruby 2.7. Since Ruby 2.6 is EOL now, I think we can drop support for Ruby 2.6 in the master branch. (The code generating this warning is in master only.)

• ubuntu-22.04 truffleruby-head: There is a warning. But it is not new by this PR.
  ../../../../ext/openssl/ossl_pkey.c:342:22: warning: implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1 [-Wsingle-bit-bitfield-constant-conversion] arg->interrupted = 1; ^ ~ 1 warning generated.

This needs to be fixed.

[rhenium]

Can you rebase this on top of the current master? I believe all warnings have been fixed now.

[junaruga]

Can you rebase this on top of the current master? I believe all warnings have been fixed now.

Yes! Thanks for your great work fixing all the warnings! That's really great work! I am rebasing this PR now.

Note I am trying to change the current syntax fail_by_warn: true (The default is false not to check the warnings) to skip-warn-checks: true (The default is false to check the warnings).

The syntax name is inspired from the skip-checks: true (default: false)
https://docs.github.com/en/actions/managing-workflow-runs/skipping-workflow-runs

[junaruga]

I rebased the PR again with updating the commit message too.

Below is the commit message.

---

CI: Check compiler warnings.

Add checks to make CI fail by compiler warnings in the rake compile.

If the skip-warnings (default: false, as an undefined variable is evaluated
as false in the if syntax) is true in specific matrix cases, the cases
skip the checks. If you want to skip new compiler warnings coming from external
changes such as upgraded compiler or OpenSSL versions in the specific matrix
cases, you can set the skip-warnings: true for the cases.

---

I tested it on my forked repository.

If the skip-warnings: true is added in a case, it is skipped. The CI result on my forked repository is here.

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 30bda38..ae9769c 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -85,7 +85,7 @@ jobs:
           - libressl-3.8.0 # Development release
         fips-enabled: [ false ]
         include:
-          - { os: ubuntu-latest, ruby: "3.0", openssl: openssl-3.0.9, fips-enabled: true, append-configure: 'enable-fips', name-extra: 'fips' }
+          - { os: ubuntu-latest, ruby: "3.0", openssl: openssl-3.0.9, fips-enabled: true, append-configure: 'enable-fips', name-extra: 'fips', skip-warnings: true }
     steps:
       - name: repo checkout
         uses: actions/checkout@v3

[junaruga]

You can see the -Werror is appended in the compiler (gcc) command lines.

https://github.com/ruby/openssl/actions/runs/5198494841/jobs/9374685609?pr=631#step:12:96

such as

gcc <..snip..> -Wall -Wextra -Wdeprecated-declarations <..snip..>  -fPIC -Werror  -o openssl_missing.o -c ../../../../ext/openssl/openssl_missing.c

[rhenium]

If the skip-warnings (default: false, as an undefined variable is evaluated
as false in the if syntax) is true in specific matrix cases, the cases
skip the checks.

This seems perfectly sensible to me!

All 40 checks passed without warnings. Let's merge this.

[junaruga]

Let's try it. Thanks for reviewing the PR and checking the CI results!