Skip to content

Conversation

@chhtz
Copy link
Member

@chhtz chhtz commented Sep 4, 2025

That line breaks cmake configuration for some packages on Ubuntu 22.04, since CMake changed that variable to Ruby_EXECUTABLE.

Related: rock-core/base-cmake#94

@doudou Do you know if there is a reason RUBY_EXECUTABLE was passed manually to some packages?

@pierrewillenbrockdfki
Copy link
Contributor

The change in cmake is as follows:

old behavior:

find_package(THING) produces variables prefixed THING_, even if it is using FindThing scripts,
find_package(Thing) produces variables prefixed Thing_

new behavior:

find_package(THING) and find_package(Thing) produce variables prefixed Thing_, when the find script is called FindThing

and i think they discourage using find_package(THING) when the find scripts character casing differs.

I guess the fix is to look through everything and use find_package(Ruby), Ruby_ variables.

@pierrewillenbrockdfki
Copy link
Contributor

As for this PR: i don't see the benefit in not defining RUBY_EXECUTABLE, though they might be unused.

I'll be on vacation for 2 weeks starting monday, 2025-09-08.

@chhtz
Copy link
Member Author

chhtz commented Sep 4, 2025

According to this link, CMake's FindRuby actually will provide both Ruby_THING and RUBY_THING (the latter is discouraged to be used and deprecated since CMake 4.0)/ The problem is that some versions of FindRuby struggle if RUBY_EXECUTABLE is provided via -DRUBY_EXECUTABLE=ruby_path, giving a weird error like:

    -- Could NOT find Ruby (missing: Ruby_EXECUTABLE) (found suitable version "3.0.2", minimum required is "1.8.0")

(in Ubuntu 24.04, CMake 3.28.3, this seems not to be a problem anymore).

If there is a reason to pass Autoproj.config.ruby_executable, we need either check if cmake_version < 3.18 and provide RUBY_EXECUTABLE or Ruby_EXECUTABLE accordingly, or always pass both.
If there is no reason to manually pass Ruby_EXECUTABLE, I prefer to remove it.

@doudou
Copy link
Member

doudou commented Sep 5, 2025

According to this link, CMake's FindRuby actually will provide both Ruby_THING and RUBY_THING (the latter is discouraged to be used and deprecated since CMake 4.0)/ The problem is that some versions of FindRuby struggle if RUBY_EXECUTABLE is provided via -DRUBY_EXECUTABLE=ruby_path, giving a weird error like:

    -- Could NOT find Ruby (missing: Ruby_EXECUTABLE) (found suitable version "3.0.2", minimum required is "1.8.0")

(in Ubuntu 24.04, CMake 3.28.3, this seems not to be a problem anymore).

If there is a reason to pass Autoproj.config.ruby_executable, we need either check if cmake_version < 3.18 and provide RUBY_EXECUTABLE or Ruby_EXECUTABLE accordingly, or always pass both. If there is no reason to manually pass Ruby_EXECUTABLE, I prefer to remove it.

There is. We need to ensure everything ruby-related is using the same ruby implementation in the workspace.

Copy link
Member

@doudou doudou left a comment

Choose a reason for hiding this comment

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

These guarantee that all FindRuby find the same ruby executable. It is needed.

@chhtz chhtz marked this pull request as draft September 8, 2025 08:12
@chhtz
Copy link
Member Author

chhtz commented Sep 8, 2025

These guarantee that all FindRuby find the same ruby executable. It is needed.

Ok, fair enough. (Even though, I guess always using the default should be the same everywhere as well, I guess you have use cases where you want to use the non-default ruby version).

I will work on another patch.

@chhtz chhtz force-pushed the fix/drop_ruby_executable_definition branch from e2958ac to aea355a Compare September 8, 2025 12:41
@chhtz chhtz changed the title Remove pkg.define "RUBY_EXECUTABLE" Define RUBY_EXECUTABLE and Ruby_EXECUTABLE for packages which need it Sep 8, 2025
@chhtz chhtz marked this pull request as ready for review September 8, 2025 12:42
@chhtz chhtz requested a review from doudou September 8, 2025 12:42
@chhtz
Copy link
Member Author

chhtz commented Sep 8, 2025

This should work on Ubuntu 20.04, 22.04, and 24.04 now. The name (and location) of the helper method is open for discussion. Also, if there is an elegant way to determine the CMake version which is in use, one could define only the correct variable.

@chhtz
Copy link
Member Author

chhtz commented Sep 8, 2025

I will update orocos-toolchain/autoproj#40 accordingly, once this gets merged

@doudou
Copy link
Member

doudou commented Sep 8, 2025

These guarantee that all FindRuby find the same ruby executable. It is needed.

Ok, fair enough. (Even though, I guess always using the default should be the same everywhere as well, I guess you have use cases where you want to use the non-default ruby version).

I will work on another patch.

On our side, we regularly end up not using the default...

@planthaber
Copy link
Member

Wouldn't "set_cmake_ruby_executable" be a better name for the function. By just reading "use_ruby_executable pkg" in the package definition, i can't really figure out what it is supposed to do.

Also, if there is an elegant way to determine the CMake version which is in use, one could define only the correct variable.

https://stackoverflow.com/questions/43697154/detect-current-cmake-version-using-cmake:

if(CMAKE_VERSION VERSION_LESS "3.8.0") 
    message("Please consider to switch to CMake 3.8.0")
endif()

Other comparison operators are VERSION_EQUAL , VERSION_GREATER, VERSION_LESS_EQUAL and VERSION_GREATER_EQUAL

@chhtz
Copy link
Member Author

chhtz commented Sep 9, 2025

Wouldn't "set_cmake_ruby_executable" be a better name for the function. By just reading "use_ruby_executable pkg" in the package definition, i can't really figure out what it is supposed to do.

I'm completely open regarding the name of the function ...

Also, if there is an elegant way to determine the CMake version which is in use, one could define only the correct variable.

https://stackoverflow.com/questions/43697154/detect-current-cmake-version-using-cmake:

if(CMAKE_VERSION VERSION_LESS "3.8.0") 
    message("Please consider to switch to CMake 3.8.0")
endif()

[...]

But this only works inside cmake, not from ruby, right? I could execute cmake --version or something and evaluate the result, but IMO that is over-complicating the issue (unless this is done by autoproj already and I can just take the result -- but a quick search did not give any results for me).

@planthaber
Copy link
Member

True, it must be detected from ruby, not cmake.

It could be done using the OS version instead of the cmake version (when the default cmake is used).

@chhtz chhtz force-pushed the fix/drop_ruby_executable_definition branch from aea355a to 966d63d Compare September 16, 2025 09:11
@chhtz
Copy link
Member Author

chhtz commented Sep 16, 2025

@planthaber I changed the macro name now. Do you want to merge?

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.

5 participants