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

[NO-TICKET] Finish autoformatting profiler with standardrb #3845

Merged
merged 2 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .git-blame-ignore-revs
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ aba860197ce4f708e917e27323884d8efe3692ca
acee9c7f3d953551cc0f20ef60e5045432bcf7e6
6e4193c5bf3c2b948c91598c7a70bc34b59872fa
b1481b215d8b1bf33a1d53419d1b95ebd1a70877
01ec575b25bc68edfea7de2046ce3c43bc3ed4af
12 changes: 0 additions & 12 deletions .standard_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,3 @@ ignore:
- spec/support/**/**
- tasks/**/**
- yard/**/**

# These profiling ignores are ALL going to be fixed in separate PRs
- lib/datadog/profiling/**/**:
- Style/StringLiterals
- spec/datadog/profiling/**/**:
- Style/StringLiterals
- ext/datadog_profiling_loader/extconf.rb:
- Style/StringLiterals
- ext/datadog_profiling_native_extension/extconf.rb:
- Style/StringLiterals
- ext/datadog_profiling_native_extension/native_extension_helpers.rb:
- Style/StringLiterals
30 changes: 15 additions & 15 deletions ext/datadog_profiling_loader/extconf.rb
Original file line number Diff line number Diff line change
@@ -1,64 +1,64 @@
# rubocop:disable Style/StderrPuts
# rubocop:disable Style/GlobalVars

if RUBY_ENGINE != 'ruby' || Gem.win_platform?
if RUBY_ENGINE != "ruby" || Gem.win_platform?
$stderr.puts(
'WARN: Skipping build of Datadog profiling loader. See Datadog profiling native extension note for details.'
"WARN: Skipping build of Datadog profiling loader. See Datadog profiling native extension note for details."
)

File.write('Makefile', 'all install clean: # dummy makefile that does nothing')
File.write("Makefile", "all install clean: # dummy makefile that does nothing")
exit
end

require 'mkmf'
require "mkmf"

# mkmf on modern Rubies actually has an append_cflags that does something similar
# (see https://github.com/ruby/ruby/pull/5760), but as usual we need a bit more boilerplate to deal with legacy Rubies
def add_compiler_flag(flag)
if try_cflags(flag)
$CFLAGS << ' ' << flag
$CFLAGS << " " << flag
else
$stderr.puts("WARNING: '#{flag}' not accepted by compiler, skipping it")
end
end

# Because we can't control what compiler versions our customers use, shipping with -Werror by default is a no-go.
# But we can enable it in CI, so that we quickly spot any new warnings that just got introduced.
add_compiler_flag '-Werror' if ENV['DATADOG_GEM_CI'] == 'true'
add_compiler_flag "-Werror" if ENV["DATADOG_GEM_CI"] == "true"

# Older gcc releases may not default to C99 and we need to ask for this. This is also used:
# * by upstream Ruby -- search for gnu99 in the codebase
# * by msgpack, another datadog gem dependency
# (https://github.com/msgpack/msgpack-ruby/blob/18ce08f6d612fe973843c366ac9a0b74c4e50599/ext/msgpack/extconf.rb#L8)
add_compiler_flag '-std=gnu99'
add_compiler_flag "-std=gnu99"

# Gets really noisy when we include the MJIT header, let's omit it (TODO: Use #pragma GCC diagnostic instead?)
add_compiler_flag '-Wno-unused-function'
add_compiler_flag "-Wno-unused-function"

# Allow defining variables at any point in a function
add_compiler_flag '-Wno-declaration-after-statement'
add_compiler_flag "-Wno-declaration-after-statement"

# If we forget to include a Ruby header, the function call may still appear to work, but then
# cause a segfault later. Let's ensure that never happens.
add_compiler_flag '-Werror-implicit-function-declaration'
add_compiler_flag "-Werror-implicit-function-declaration"

# Warn on unused parameters to functions. Use `DDTRACE_UNUSED` to mark things as known-to-not-be-used.
add_compiler_flag '-Wunused-parameter'
add_compiler_flag "-Wunused-parameter"

# The native extension is not intended to expose any symbols/functions for other native libraries to use;
# the sole exception being `Init_datadog_profiling_loader` which needs to be visible for Ruby to call it when
# it `dlopen`s the library.
#
# By setting this compiler flag, we tell it to assume that everything is private unless explicitly stated.
# For more details see https://gcc.gnu.org/wiki/Visibility
add_compiler_flag '-fvisibility=hidden'
add_compiler_flag "-fvisibility=hidden"

# Avoid legacy C definitions
add_compiler_flag '-Wold-style-definition'
add_compiler_flag "-Wold-style-definition"

# Enable all other compiler warnings
add_compiler_flag '-Wall'
add_compiler_flag '-Wextra'
add_compiler_flag "-Wall"
add_compiler_flag "-Wextra"

# Tag the native extension library with the Ruby version and Ruby platform.
# This makes it easier for development (avoids "oops I forgot to rebuild when I switched my Ruby") and ensures that
Expand Down
106 changes: 53 additions & 53 deletions ext/datadog_profiling_native_extension/extconf.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# rubocop:disable Style/StderrPuts
# rubocop:disable Style/GlobalVars

require_relative 'native_extension_helpers'
require_relative '../libdatadog_extconf_helpers'
require_relative "native_extension_helpers"
require_relative "../libdatadog_extconf_helpers"

SKIPPED_REASON_FILE = "#{__dir__}/skipped_reason.txt".freeze
# Not a problem if the file doesn't exist or we can't delete it
Expand All @@ -29,13 +29,13 @@ def skip_building_extension!(reason)
)

if fail_install_if_missing_extension
require 'mkmf'
require "mkmf"
Logging.message(
'[datadog] Failure cause: ' \
"[datadog] Failure cause: " \
"#{Datadog::Profiling::NativeExtensionHelpers::Supported.render_skipped_reason_file(**reason)}\n"
)
else
File.write('Makefile', 'all install clean: # dummy makefile that does nothing')
File.write("Makefile", "all install clean: # dummy makefile that does nothing")
end

exit
Expand Down Expand Up @@ -68,7 +68,7 @@ def skip_building_extension!(reason)

# NOTE: we MUST NOT require 'mkmf' before we check the #skip_building_extension? because the require triggers checks
# that may fail on an environment not properly setup for building Ruby extensions.
require 'mkmf'
require "mkmf"

Logging.message("[datadog] Using compiler:\n")
xsystem("#{CONFIG["CC"]} -v")
Expand All @@ -78,128 +78,128 @@ def skip_building_extension!(reason)
# (see https://github.com/ruby/ruby/pull/5760), but as usual we need a bit more boilerplate to deal with legacy Rubies
def add_compiler_flag(flag)
if try_cflags(flag)
$CFLAGS << ' ' << flag
$CFLAGS << " " << flag
else
$stderr.puts("WARNING: '#{flag}' not accepted by compiler, skipping it")
end
end

# Because we can't control what compiler versions our customers use, shipping with -Werror by default is a no-go.
# But we can enable it in CI, so that we quickly spot any new warnings that just got introduced.
add_compiler_flag '-Werror' if ENV['DATADOG_GEM_CI'] == 'true'
add_compiler_flag "-Werror" if ENV["DATADOG_GEM_CI"] == "true"

# Older gcc releases may not default to C99 and we need to ask for this. This is also used:
# * by upstream Ruby -- search for gnu99 in the codebase
# * by msgpack, another datadog gem dependency
# (https://github.com/msgpack/msgpack-ruby/blob/18ce08f6d612fe973843c366ac9a0b74c4e50599/ext/msgpack/extconf.rb#L8)
add_compiler_flag '-std=gnu99'
add_compiler_flag "-std=gnu99"

# Gets really noisy when we include the MJIT header, let's omit it (TODO: Use #pragma GCC diagnostic instead?)
add_compiler_flag '-Wno-unused-function'
add_compiler_flag "-Wno-unused-function"

# Allow defining variables at any point in a function
add_compiler_flag '-Wno-declaration-after-statement'
add_compiler_flag "-Wno-declaration-after-statement"

# If we forget to include a Ruby header, the function call may still appear to work, but then
# cause a segfault later. Let's ensure that never happens.
add_compiler_flag '-Werror-implicit-function-declaration'
add_compiler_flag "-Werror-implicit-function-declaration"

# The native extension is not intended to expose any symbols/functions for other native libraries to use;
# the sole exception being `Init_datadog_profiling_native_extension` which needs to be visible for Ruby to call it when
# it `dlopen`s the library.
#
# By setting this compiler flag, we tell it to assume that everything is private unless explicitly stated.
# For more details see https://gcc.gnu.org/wiki/Visibility
add_compiler_flag '-fvisibility=hidden'
add_compiler_flag "-fvisibility=hidden"

# Avoid legacy C definitions
add_compiler_flag '-Wold-style-definition'
add_compiler_flag "-Wold-style-definition"

# Enable all other compiler warnings
add_compiler_flag '-Wall'
add_compiler_flag '-Wextra'
add_compiler_flag "-Wall"
add_compiler_flag "-Wextra"

if ENV['DDTRACE_DEBUG'] == 'true'
$defs << '-DDD_DEBUG'
CONFIG['optflags'] = '-O0'
CONFIG['debugflags'] = '-ggdb3'
if ENV["DDTRACE_DEBUG"] == "true"
$defs << "-DDD_DEBUG"
CONFIG["optflags"] = "-O0"
CONFIG["debugflags"] = "-ggdb3"
end

if RUBY_PLATFORM.include?('linux')
if RUBY_PLATFORM.include?("linux")
# Supposedly, the correct way to do this is
# ```
# have_library 'pthread'
# have_func 'pthread_getcpuclockid'
# ```
# but it's slower to build
# so instead we just assume that we have the function we need on Linux, and nowhere else
$defs << '-DHAVE_PTHREAD_GETCPUCLOCKID'
$defs << "-DHAVE_PTHREAD_GETCPUCLOCKID"

# Not available on macOS
$defs << '-DHAVE_CLOCK_MONOTONIC_COARSE'
$defs << "-DHAVE_CLOCK_MONOTONIC_COARSE"
end

have_func 'malloc_stats'
have_func "malloc_stats"

# On older Rubies, rb_postponed_job_preregister/rb_postponed_job_trigger did not exist
$defs << '-DNO_POSTPONED_TRIGGER' if RUBY_VERSION < '3.3'
$defs << "-DNO_POSTPONED_TRIGGER" if RUBY_VERSION < "3.3"

# On older Rubies, M:N threads were not available
$defs << '-DNO_MN_THREADS_AVAILABLE' if RUBY_VERSION < '3.3'
$defs << "-DNO_MN_THREADS_AVAILABLE" if RUBY_VERSION < "3.3"

# On older Rubies, we did not need to include the ractor header (this was built into the MJIT header)
$defs << '-DNO_RACTOR_HEADER_INCLUDE' if RUBY_VERSION < '3.3'
$defs << "-DNO_RACTOR_HEADER_INCLUDE" if RUBY_VERSION < "3.3"

# On older Rubies, some of the Ractor internal APIs were directly accessible
$defs << '-DUSE_RACTOR_INTERNAL_APIS_DIRECTLY' if RUBY_VERSION < '3.3'
$defs << "-DUSE_RACTOR_INTERNAL_APIS_DIRECTLY" if RUBY_VERSION < "3.3"

# On older Rubies, there was no struct rb_native_thread. See private_vm_api_acccess.c for details.
$defs << '-DNO_RB_NATIVE_THREAD' if RUBY_VERSION < '3.2'
$defs << "-DNO_RB_NATIVE_THREAD" if RUBY_VERSION < "3.2"

# On older Rubies, there was no struct rb_thread_sched (it was struct rb_global_vm_lock_struct)
$defs << '-DNO_RB_THREAD_SCHED' if RUBY_VERSION < '3.2'
$defs << "-DNO_RB_THREAD_SCHED" if RUBY_VERSION < "3.2"

# On older Rubies, the first_lineno inside a location was a VALUE and not a int (https://github.com/ruby/ruby/pull/6430)
$defs << '-DNO_INT_FIRST_LINENO' if RUBY_VERSION < '3.2'
$defs << "-DNO_INT_FIRST_LINENO" if RUBY_VERSION < "3.2"

# On older Rubies, "pop" was not a primitive operation
$defs << '-DNO_PRIMITIVE_POP' if RUBY_VERSION < '3.2'
$defs << "-DNO_PRIMITIVE_POP" if RUBY_VERSION < "3.2"

# On older Rubies, there was no tid member in the internal thread structure
$defs << '-DNO_THREAD_TID' if RUBY_VERSION < '3.1'
$defs << "-DNO_THREAD_TID" if RUBY_VERSION < "3.1"

# On older Rubies, there was no jit_return member on the rb_control_frame_t struct
$defs << '-DNO_JIT_RETURN' if RUBY_VERSION < '3.1'
$defs << "-DNO_JIT_RETURN" if RUBY_VERSION < "3.1"

# On older Rubies, rb_gc_force_recycle allowed to free objects in a way that
# would be invisible to free tracepoints, finalizers and without cleaning
# obj_to_id_tbl mappings.
$defs << '-DHAVE_WORKING_RB_GC_FORCE_RECYCLE' if RUBY_VERSION < '3.1'
$defs << "-DHAVE_WORKING_RB_GC_FORCE_RECYCLE" if RUBY_VERSION < "3.1"

# On older Rubies, there are no Ractors
$defs << '-DNO_RACTORS' if RUBY_VERSION < '3'
$defs << "-DNO_RACTORS" if RUBY_VERSION < "3"

# On older Rubies, rb_imemo_name did not exist
$defs << '-DNO_IMEMO_NAME' if RUBY_VERSION < '3'
$defs << "-DNO_IMEMO_NAME" if RUBY_VERSION < "3"

# On older Rubies, objects would not move
$defs << '-DNO_T_MOVED' if RUBY_VERSION < '2.7'
$defs << "-DNO_T_MOVED" if RUBY_VERSION < "2.7"

# On older Rubies, there was no RUBY_SEEN_OBJ_ID flag
$defs << '-DNO_SEEN_OBJ_ID_FLAG' if RUBY_VERSION < '2.7'
$defs << "-DNO_SEEN_OBJ_ID_FLAG" if RUBY_VERSION < "2.7"

# On older Rubies, rb_global_vm_lock_struct did not include the owner field
$defs << '-DNO_GVL_OWNER' if RUBY_VERSION < '2.6'
$defs << "-DNO_GVL_OWNER" if RUBY_VERSION < "2.6"

# On older Rubies, there was no thread->invoke_arg
$defs << '-DNO_THREAD_INVOKE_ARG' if RUBY_VERSION < '2.6'
$defs << "-DNO_THREAD_INVOKE_ARG" if RUBY_VERSION < "2.6"

# If we got here, libdatadog is available and loaded
ENV['PKG_CONFIG_PATH'] = "#{ENV["PKG_CONFIG_PATH"]}:#{Libdatadog.pkgconfig_folder}"
ENV["PKG_CONFIG_PATH"] = "#{ENV["PKG_CONFIG_PATH"]}:#{Libdatadog.pkgconfig_folder}"
Logging.message("[datadog] PKG_CONFIG_PATH set to #{ENV["PKG_CONFIG_PATH"].inspect}\n")
$stderr.puts("Using libdatadog #{Libdatadog::VERSION} from #{Libdatadog.pkgconfig_folder}")

unless pkg_config('datadog_profiling_with_rpath')
unless pkg_config("datadog_profiling_with_rpath")
Logging.message("[datadog] Ruby detected the pkg-config command is #{$PKGCONFIG.inspect}\n")

skip_building_extension!(
Expand All @@ -212,7 +212,7 @@ def add_compiler_flag(flag)
)
end

unless have_type('atomic_int', ['stdatomic.h'])
unless have_type("atomic_int", ["stdatomic.h"])
skip_building_extension!(Datadog::Profiling::NativeExtensionHelpers::Supported::COMPILER_ATOMIC_MISSING)
end

Expand Down Expand Up @@ -242,8 +242,8 @@ def add_compiler_flag(flag)
# use the MJIT header.
# Finally, the `COMMON_HEADERS` conflict with the MJIT header so we need to temporarily disable them for this check.
original_common_headers = MakeMakefile::COMMON_HEADERS
MakeMakefile::COMMON_HEADERS = ''.freeze
unless have_macro('RUBY_MJIT_H', mjit_header_file_name)
MakeMakefile::COMMON_HEADERS = "".freeze
unless have_macro("RUBY_MJIT_H", mjit_header_file_name)
skip_building_extension!(Datadog::Profiling::NativeExtensionHelpers::Supported::COMPILATION_BROKEN)
end
MakeMakefile::COMMON_HEADERS = original_common_headers
Expand All @@ -255,7 +255,7 @@ def add_compiler_flag(flag)

# Warn on unused parameters to functions. Use `DDTRACE_UNUSED` to mark things as known-to-not-be-used.
# See the comment on the same flag below for why this is done last.
add_compiler_flag '-Wunused-parameter'
add_compiler_flag "-Wunused-parameter"

create_makefile EXTENSION_NAME
else
Expand All @@ -266,23 +266,23 @@ def add_compiler_flag(flag)

create_header

require 'debase/ruby_core_source'
dir_config('ruby') # allow user to pass in non-standard core include directory
require "debase/ruby_core_source"
dir_config("ruby") # allow user to pass in non-standard core include directory

Debase::RubyCoreSource
.create_makefile_with_core(
proc do
headers_available =
have_header('vm_core.h') &&
have_header('iseq.h') &&
(RUBY_VERSION < '3.3' || have_header('ractor_core.h'))
have_header("vm_core.h") &&
have_header("iseq.h") &&
(RUBY_VERSION < "3.3" || have_header("ractor_core.h"))

if headers_available
# Warn on unused parameters to functions. Use `DDTRACE_UNUSED` to mark things as known-to-not-be-used.
# This is added as late as possible because in some Rubies we support (e.g. 3.3), adding this flag before
# checking if internal VM headers are available causes those checks to fail because of this warning (and not
# because the headers are not available.)
add_compiler_flag '-Wunused-parameter'
add_compiler_flag "-Wunused-parameter"
end

headers_available
Expand Down
Loading
Loading