Skip to content

Conversation

@junaruga
Copy link
Member

This PR is to print the compiling commands in the process of the rake compile, and enables us to detect the compiler warnings like the issue #620 in a PR process. This is a workaround until a formal way to enable the verbose mode in runtime of Ruby will be implemented. You can see ruby/setup-ruby#505 (comment) for the proposal.

Here is the result on my forked repository.

Ubuntu Ruby 3.2
https://github.com/junaruga/openssl/actions/runs/5049251346/jobs/9058658930#step:6:81

gcc -I. -I/opt/hostedtoolcache/Ruby/3.2.2/x64/include/ruby-3.2.0/x86_64-linux -I/opt/hostedtoolcache/Ruby/3.2.2/x64/include/ruby-3.2.0/ruby/backward -I/opt/hostedtoolcache/Ruby/3.2.2/x64/include/ruby-3.2.0 -I../../../../ext/openssl -DRUBY_EXTCONF_H="extconf.h" -I/opt/hostedtoolcache/Ruby/3.2.2/x64/include -DENABLE_PATH_CHECK=0 -fPIC -O3 -fno-fast-math -ggdb3 -Wall -Wextra -Wdeprecated-declarations -Wdiv-by-zero -Wduplicated-cond -Wimplicit-function-declaration -Wimplicit-int -Wmisleading-indentation -Wpointer-arith -Wwrite-strings -Wold-style-definition -Wimplicit-fallthrough=0 -Wmissing-noreturn -Wno-cast-function-type -Wno-constant-logical-operand -Wno-long-long -Wno-missing-field-initializers -Wno-overlength-strings -Wno-packed-bitfield-compat -Wno-parentheses-equality -Wno-self-assign -Wno-tautological-compare -Wno-unused-parameter -Wno-unused-value -Wsuggest-attribute=format -Wsuggest-attribute=noreturn -Wunused-variable -Wundef -fPIC -o openssl_missing.o -c ../../../../ext/openssl/openssl_missing.c

macOS Ruby 3.2
https://github.com/junaruga/openssl/actions/runs/5049251346/jobs/9058660439#step:6:85

clang -I. -I/Users/runner/hostedtoolcache/Ruby/3.2.2/x64/include/ruby-3.2.0/x86_64-darwin20 -I/Users/runner/hostedtoolcache/Ruby/3.2.2/x64/include/ruby-3.2.0/ruby/backward -I/Users/runner/hostedtoolcache/Ruby/3.2.2/x64/include/ruby-3.2.0 -I../../../../ext/openssl -DRUBY_EXTCONF_H="extconf.h" -I/Users/runner/hostedtoolcache/Ruby/3.2.2/x64/openssl/include -I/Users/runner/hostedtoolcache/Ruby/3.2.2/x64/include -DENABLE_PATH_CHECK=0 -I/usr/local/opt/gmp/include -D_XOPEN_SOURCE -D_DARWIN_C_SOURCE -D_DARWIN_UNLIMITED_SELECT -D_REENTRANT -fno-common -fdeclspec -O3 -fno-fast-math -ggdb3 -Wall -Wextra -Wextra-tokens -Wdeprecated-declarations -Wdivision-by-zero -Wdiv-by-zero -Wimplicit-function-declaration -Wimplicit-int -Wmisleading-indentation -Wpointer-arith -Wshorten-64-to-32 -Wwrite-strings -Wold-style-definition -Wmissing-noreturn -Wno-constant-logical-operand -Wno-long-long -Wno-missing-field-initializers -Wno-overlength-strings -Wno-parentheses-equality -Wno-self-assign -Wno-tautological-compare -Wno-unused-parameter -Wno-unused-value -Wunused-variable -Wundef -fno-common -pipe -o openssl_missing.o -c ../../../../ext/openssl/openssl_missing.c


Enable the verbose option in mkmf.rb to print the compiling commands in the process of the rake compile. Because it's helpful to see what compiler warnings are checked.

The script only runs in Linux and macOS. Not Windows. Because the sh script doesn't work in Windows. For the syntax, see the reference.[1]

Right now there is a way to configure Ruby with --enable-mkmf-verbose in this purpose. But there is no formal way to enable the verbose mode in runtime of Ruby. My intention is that this commit is a workaround for this purpose.

[1] https://docs.github.com/en/actions/learn-github-actions/variables

  • Default environment variables - RUNNER_OS
  • Detecting the operating system

@junaruga junaruga force-pushed the wip/enable-mkmf-verbose branch from 915d05f to 9e9c1be Compare May 22, 2023 19:02
@junaruga junaruga changed the title Enable the verbose mode in the mkmf.rb. CI: Enable the verbose mode in the mkmf.rb. May 22, 2023
@junaruga junaruga force-pushed the wip/enable-mkmf-verbose branch from 9e9c1be to 65500c4 Compare May 22, 2023 20:55
@junaruga
Copy link
Member Author

Note the compiler warnings are always printed whether the verbose mode is enabled.

Here is the CI case of macOS Ruby 2.6 for this PR. Some compiler warnings are printed.

https://github.com/ruby/openssl/actions/runs/5050315365/jobs/9060885830?pr=623#step:6:88

clang -I. -c ../../../../ext/openssl/ossl_asn1.c
../../../../ext/openssl/ossl_asn1.c:69:34: warning: '(' and '{' tokens introducing statement expression appear in different macro expansion contexts [-Wcompound-token-split-by-macro]
return rb_funcall2(rb_cTime, rb_intern("utc"), 6, argv);
^~~~~~~~~~~~~~~~

And here is the CI case of macOS Ruby 2.6 for the current master branch. The same compiler warnings are printed.

https://github.com/ruby/openssl/actions/runs/5019473754/jobs/9000009197#step:5:88

compiling ../../../../ext/openssl/ossl_asn1.c
../../../../ext/openssl/ossl_asn1.c:69:34: warning: '(' and '{' tokens introducing statement expression appear in different macro expansion contexts [-Wcompound-token-split-by-macro]
return rb_funcall2(rb_cTime, rb_intern("utc"), 6, argv);
^~~~~~~~~~~~~~~~

sed -i -e 's/^V = .*/V = 1/' "${mkmf_rb_path}"

# Print the difference between the original and modified file.
diff "${mkmf_rb_path}.orig" "${mkmf_rb_path}" || :
Copy link
Member Author

Choose a reason for hiding this comment

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

Note the diff command is to notice and debug easily when the mkmf.rb may change in the future.

@junaruga junaruga force-pushed the wip/enable-mkmf-verbose branch from 65500c4 to 3ee4bd9 Compare May 23, 2023 07:37
@junaruga
Copy link
Member Author

Note I aligned the style of the new shell script enable-mkmf-verbose with the existing tool/update-gh-pages, such as tab indent.

@junaruga junaruga force-pushed the wip/enable-mkmf-verbose branch 2 times, most recently from 1f5ffec to fe6d280 Compare May 23, 2023 09:07
@junaruga
Copy link
Member Author

junaruga commented May 23, 2023

https://bugs.ruby-lang.org/projects/ruby/wiki/DeveloperHowTo

I changed my mind. I changed the indent from a tab to 2 spaces. Because the tab indent is forbidden in the current coding style in the Ruby project. So, the choice is 2 or 4 spaces indent. And seeing the shell script files in the tool directory in ruby/ruby, the 4 spaces indent is more popular than the 2 spaces indent.

https://bugs.ruby-lang.org/projects/ruby/wiki/DeveloperHowTo

4 for C
2 for Ruby
...
Do not use TABs in ruby codes

Here is the result in the tool directory in the ruby/ruby.

$ grep -r "/bin/sh" tool/
tool/merger.rb:#!/bin/sh
tool/build-transcode:#!/bin/sh
tool/rmdirs:#!/bin/sh
tool/rbinstall.rb:#!/bin/sh
tool/ifchange:#!/bin/sh
tool/install-sh:#!/bin/sh
tool/bisect.sh:#!/bin/sh
tool/test-annocheck.sh:#!/bin/sh -eu
tool/wasm-clangw:#!/bin/sh
tool/test/test_sync_default_gems.rb:        File.write("#{dir}/tool/ok", "#!/bin/sh\n""echo ok\n")
tool/git-refresh:#!/bin/sh
tool/travis_retry.sh:#!/bin/sh -eu

$ find . -name "*.sh"
./spec/ruby/command_line/fixtures/bin/hybrid_launcher.sh
./tool/release.sh
./tool/ci_functions.sh
./tool/bisect.sh
./tool/test-annocheck.sh
./tool/disable_ipv6.sh
./tool/travis_retry.sh
./tool/travis_wait.sh
./autogen.sh
./ext/digest/test.sh
./ext/io/console/buildgem.sh

Enable the verbose option in mkmf.rb to print the compiling commands in the
process of the `rake compile`. Because it's helpful to see what compiler
warnings are checked.

The script only runs in Linux and macOS. Not Windows. Because the sh script
doesn't work in Windows. For the syntax, see the reference.[1]

Right now there is a way to configure Ruby with `--enable-mkmf-verbose` in this
purpose. But there is no formal way to enable the verbose mode in runtime of
Ruby. My intention is that this commit is a workaround for this purpose.

[1] https://docs.github.com/en/actions/learn-github-actions/variables
  * Default environment variables - RUNNER_OS
  * Detecting the operating system
@junaruga junaruga force-pushed the wip/enable-mkmf-verbose branch from fe6d280 to a832f5c Compare May 23, 2023 09:33
@junaruga junaruga merged commit 78e9197 into ruby:master May 23, 2023
@junaruga junaruga deleted the wip/enable-mkmf-verbose branch May 23, 2023 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant