From b756b8ccb649e6dcde466ab193f7c36c04428035 Mon Sep 17 00:00:00 2001 From: Juan Hernandez Date: Thu, 1 Feb 2024 13:50:08 +0100 Subject: [PATCH 1/2] Update to Ruby 3 Currently the SDK doesn't build or work with Ruby 3: - The code generator needs at least Weld 3 in order to work with Java 11. - The versions of `rake`, `rubocop` and `webrick` need to be udpated to work with Ruby 3. - The C code needs to be udpated to use the `rb_thread_call_with_gvl` only when the code doesn't already hold the GVL. This is probably due to changes in `libcurl`. This patch contains fixes for all those issues. Signed-off-by: Juan Hernandez --- generator/pom.xml | 4 ++-- sdk/.rubocop.yml | 20 +++++++++++++++++++- sdk/examples/vm_backup.rb | 2 +- sdk/ext/ovirtsdk4c/ov_http_client.c | 12 ++++++++++-- sdk/ovirt-engine-sdk.gemspec | 7 ++++--- 5 files changed, 36 insertions(+), 9 deletions(-) diff --git a/generator/pom.xml b/generator/pom.xml index faef04f..264fd41 100644 --- a/generator/pom.xml +++ b/generator/pom.xml @@ -66,8 +66,8 @@ limitations under the License. org.jboss.weld.se - weld-se - 2.3.0.Final + weld-se-core + 3.1.9.Final org.slf4j diff --git a/sdk/.rubocop.yml b/sdk/.rubocop.yml index bf6bb31..c979e16 100644 --- a/sdk/.rubocop.yml +++ b/sdk/.rubocop.yml @@ -69,7 +69,7 @@ Style/DateTime: # to itself. Rubocop doesn't like these tests, but we want them anyhow, # so we exclude that spec. # -Lint/UselessComparison: +Lint/BinaryOperatorWithIdenticalOperands: Exclude: - 'spec/type_spec.rb' Style/NilComparison: @@ -95,3 +95,21 @@ AllCops: - 'lib/ovirtsdk4/version.rb' - 'pkg/**/*' - 'tmp/**/*' + +# +# This things used to work fine before upgrading Rubocop, and we don't want +# to address them now. At some point we should remove these lines, check the +# result and fix the code accordingly. +# +Style/RedundantFileExtensionInRequire: + Enabled: False +Style/OptionalBooleanParameter: + Enabled: False +Naming/VariableNumber: + Enabled: False +Style/AccessorGrouping: + Enabled: False +Style/SingleArgumentDig: + Enabled: False +Style/ZeroLengthPredicate: + Enabled: False diff --git a/sdk/examples/vm_backup.rb b/sdk/examples/vm_backup.rb index b5a004e..54bce5b 100755 --- a/sdk/examples/vm_backup.rb +++ b/sdk/examples/vm_backup.rb @@ -60,7 +60,7 @@ system_service = connection.system_service # Get the reference to the service that we will use to send events to the audit log: -events_service = system_service.events_service() +events_service = system_service.events_service # In order to send events we need to also send unique integer ids. These should usually come from an external # database, but in this example we # will just generate them from the current time in seconds since Jan 1st 1970. diff --git a/sdk/ext/ovirtsdk4c/ov_http_client.c b/sdk/ext/ovirtsdk4c/ov_http_client.c index abc4d10..0dcc11a 100644 --- a/sdk/ext/ovirtsdk4c/ov_http_client.c +++ b/sdk/ext/ovirtsdk4c/ov_http_client.c @@ -490,12 +490,20 @@ static int ov_http_client_debug_function(CURL* handle, curl_infotype type, char* /* Get the pointer to the transfer: */ ov_http_transfer_ptr(transfer, transfer_ptr); - /* Execute the debug code with the global interpreter lock acquired, as it needs to call Ruby methods: */ + /* The global interpreter lock may be acquired or not, so we need to check and either call the task directly + or else call it after acquiring the lock. Note that the `ruby_thread_has_gvl_p` function that we ue to + check if the GVL is acquired is marked as experimental, and not defined in `thread.h`, so it may be + removed at any time, but I didn't find any other way to check if the GVL is acquired. */ context.client = transfer_ptr->client; context.type = type; context.data = data; context.size = size; - rb_thread_call_with_gvl(ov_http_client_debug_task, &context); + if (ruby_thread_has_gvl_p()) { + ov_http_client_debug_task(&context); + } + else { + rb_thread_call_with_gvl(ov_http_client_debug_task, &context); + } return 0; } diff --git a/sdk/ovirt-engine-sdk.gemspec b/sdk/ovirt-engine-sdk.gemspec index 90d52ab..27f9e64 100644 --- a/sdk/ovirt-engine-sdk.gemspec +++ b/sdk/ovirt-engine-sdk.gemspec @@ -39,10 +39,11 @@ Gem::Specification.new do |spec| spec.required_ruby_version = '>= 2.5' # Build time dependencies: - spec.add_development_dependency('rake', '~> 12.3') - spec.add_development_dependency('rake-compiler', '~> 1.0') + spec.add_development_dependency('rake', '~> 13.1') + spec.add_development_dependency('rake-compiler', '~> 1.2') spec.add_development_dependency('rspec', '~> 3.7') - spec.add_development_dependency('rubocop', '0.79.0') + spec.add_development_dependency('rubocop', '1.60.2') + spec.add_development_dependency('webrick', '~> 1.8') spec.add_development_dependency('yard', '~> 0.9', '>= 0.9.12') # Run time dependencies: From 957fb6f7da1578784a4123441e931202e6c16bb6 Mon Sep 17 00:00:00 2001 From: Sandro Bonazzola Date: Wed, 27 Mar 2024 13:44:33 +0100 Subject: [PATCH 2/2] Update sdk/ext/ovirtsdk4c/ov_http_client.c Thanks Co-authored-by: eremeyev --- sdk/ext/ovirtsdk4c/ov_http_client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/ext/ovirtsdk4c/ov_http_client.c b/sdk/ext/ovirtsdk4c/ov_http_client.c index 0dcc11a..65e3f41 100644 --- a/sdk/ext/ovirtsdk4c/ov_http_client.c +++ b/sdk/ext/ovirtsdk4c/ov_http_client.c @@ -491,7 +491,7 @@ static int ov_http_client_debug_function(CURL* handle, curl_infotype type, char* ov_http_transfer_ptr(transfer, transfer_ptr); /* The global interpreter lock may be acquired or not, so we need to check and either call the task directly - or else call it after acquiring the lock. Note that the `ruby_thread_has_gvl_p` function that we ue to + or else call it after acquiring the lock. Note that the `ruby_thread_has_gvl_p` function that we use to check if the GVL is acquired is marked as experimental, and not defined in `thread.h`, so it may be removed at any time, but I didn't find any other way to check if the GVL is acquired. */ context.client = transfer_ptr->client;