raylib: add some common build settings to options #24585
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Thanks @Julianiolo for taking the time to add the options :)
Co-authored-by: Abril Rincón Blanco <[email protected]>
This comment has been minimized.
This comment has been minimized.
|
As this configuration is not exposed as project option, I would suggest using Conan configuration to pass any custom definition to the compiler instead: https://docs.conan.io/2/reference/config_files/global_conf.html You can add it to your profile, or use via command line directly: This configuration will pass As it may affect the package ID, in case changing the result of the library behavior, you still can inject that configuration as part of the package ID as well: So, in summary |
|
@uilianries I think they are intended as project options, raylib just manages them via this config.h file, not via some build script. (as It also has build scripts for like 5+ different build systems) The intention is to be able to set these options from a consuming recipe, would that even be possible with the configuration? |
|
@Julianiolo I'm reading raylib cmake files and indeed those options are exposed: https://github.com/raysan5/raylib/blob/5.0/CMakeOptions.txt#L31. Why passing via
Configurations work for both cases, building and consuming. |
|
Oh WOW, how did I not see that lol, I was looking at the wrong CMakeLists.txt.... |
This comment has been minimized.
This comment has been minimized.
|
@Julianiolo That could work, but need to test first. Thank for spotting it. |
recipes/raylib/all/conanfile.py
Outdated
| true_false = lambda x: True if x else False | ||
| tc.variables["SUPPORT_MODULE_RSHAPES"] = true_false(self.options.module_rshapes) |
There was a problem hiding this comment.
| true_false = lambda x: True if x else False | |
| tc.variables["SUPPORT_MODULE_RSHAPES"] = true_false(self.options.module_rshapes) | |
| tc.variables["SUPPORT_MODULE_RSHAPES"] = self.options.module_rshapes |
The OptionValue is converted to boolean already, you should not need another helper function.
recipes/raylib/all/conanfile.py
Outdated
| } | ||
| default_options = { | ||
| "shared": False, | ||
| "fPIC": True, | ||
| "opengl_version": None, | ||
|
|
||
| "module_rshapes": True, |
There was a problem hiding this comment.
I would suggest adding only option that you really need for now, otherwise, it will increase the recipe maintenance. We can not validate each combination, which means there is a risk breaking things depending the combination.
I also would request a build log, if possible, using the options that you are adding, I mean, using the non-default option value, to make sure it's not broken. I asking it because we found recipes with custom options that are not working and the CI really does not check it (only shared, fPIC and header_only are checked), so any user opens an issue months later reporting about that option is broken and recipe/upstream is bugged.
Co-authored-by: Uilian Ries <[email protected]>
|
@uilianries I just saw this: raysan5/raylib@307c998 So I guess maybe the patching approach is actually more future safe? On second thought, why does it parse the config.h to then produce cmake options, when it could just include the config.h in the code?? |
This comment has been minimized.
This comment has been minimized.
@Julianiolo because we avoid patching anything as we will become the maintainers of that patch. The project offers transparent options via CMake, and it reflects to the Conan generator CMaketoolchain, that's is well prepared and integrated to work with the project, and we have been using it for any other project. So, using CMake will consume less maintenance from our side. |
|
@uilianries No, with that commit, it does not support options via cmake anymore (or am I reading this wrong?) I'm going to investigate this... |
|
Ok, turns out I was reading that completely wrong, lol |
This comment has been minimized.
This comment has been minimized.
|
So now I removed some options, and added a @uilianries Do you mean by build logs just the output from |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@uilianries so what is the status? |
|
@Julianiolo like I commented before (#24585 (comment)) I would recommend only adding those options that you really need. Plus, add a full build log with those options values (e.g module_rshapes is True, then we need a build with False). As you can see, as more option, worst is tracking and maintaining. Regards. |
Build log
|
Conan v1 pipeline ✔️All green in build 9 (
Conan v2 pipeline ✔️
All green in build 9 ( |
There was a problem hiding this comment.
@Julianiolo Thank you for updating your PR.
There are still points that need few changes:
- The raylib project does not use customize build by default, neither other package managers that I visited are using it by default, so I don't recommend doing it by default as will affect every user.
- In case needing to extra cmake definitions, you still can use Conan config, e.g:
conan install ... -c -c tools.cmake.cmaketoolchain:extra_variables='{"SUPPORT_CAMERA_SYSTEM": "ON", "SUPPORT_GESTURES_SYSTEM": "ON"}'
The same can be part of your profile. You can obtain more information in the CMakeToolchain and Config docs page.
Still, it will not affect you package ID directly, so you can enforce it by using the tools.info.package_id:confs:
-c tools.info.package_id:confs='["tools.cmake.cmaketoolchain:extra_variables"]'
This will tell Conan to use extra_variables as part of the Package ID.
- The options
module_raudio,events_waiting, andcustom_frame_controlare directly affected bycustomize_build, so I would suggest removing them in configure, as you can not change them in case customize_build is False:
def configure(self):
if not self.customize_build:
del module_raudio
del events_waiting
del custom_frame_control
- You do not need to parse to "ON"/"OFF" those options values, the boolean values work perfectly with CMake, we have being using it since many years and is compatible with both CMake 2.8 and +3.x
recipes/raylib/all/conanfile.py
Outdated
|
|
||
| tc.variables["CUSTOMIZE_BUILD"] = "ON" | ||
| on_off = lambda x: "ON" if x else "OFF" | ||
| tc.variables["SUPPORT_MODULE_RAUDIO"] = on_off(self.options.module_raudio) | ||
|
|
||
| # this makes it include the headers rcamera.h, rgesture.h and rprand.h | ||
| tc.variables["SUPPORT_CAMERA_SYSTEM"] = "ON" | ||
| tc.variables["SUPPORT_GESTURES_SYSTEM"] = "ON" | ||
| tc.variables["SUPPORT_RPRAND_GENERATOR"] = "ON" | ||
|
|
||
| tc.variables["SUPPORT_EVENTS_WAITING"] = on_off(self.options.events_waiting) | ||
| tc.variables["SUPPORT_CUSTOM_FRAME_CONTROL"] = on_off(self.options.custom_frame_control) | ||
|
|
There was a problem hiding this comment.
| tc.variables["CUSTOMIZE_BUILD"] = "ON" | |
| on_off = lambda x: "ON" if x else "OFF" | |
| tc.variables["SUPPORT_MODULE_RAUDIO"] = on_off(self.options.module_raudio) | |
| # this makes it include the headers rcamera.h, rgesture.h and rprand.h | |
| tc.variables["SUPPORT_CAMERA_SYSTEM"] = "ON" | |
| tc.variables["SUPPORT_GESTURES_SYSTEM"] = "ON" | |
| tc.variables["SUPPORT_RPRAND_GENERATOR"] = "ON" | |
| tc.variables["SUPPORT_EVENTS_WAITING"] = on_off(self.options.events_waiting) | |
| tc.variables["SUPPORT_CUSTOM_FRAME_CONTROL"] = on_off(self.options.custom_frame_control) | |
| tc.variables["CUSTOMIZE_BUILD"] = self.options.customize_build | |
| if self.options.customize_build: | |
| tc.variables["SUPPORT_MODULE_RAUDIO"] = self.options.get_safe("module_raudio") | |
| tc.variables["SUPPORT_EVENTS_WAITING"] = self.options.get_safe("events_waiting") | |
| tc.variables["SUPPORT_CUSTOM_FRAME_CONTROL"] = self.options.get_safe("custom_frame_control") |
I would ask to remove this forced customization and simplify it instead:
- The upstream goes to the opposite way by default: https://github.com/raysan5/raylib/blob/5.0/CMakeOptions.txt#L11. The
CUSTOMIZE_BUILDdisabled, same for camera, gestures and rprand - Not even others package managers are customizing it, real example:
- AUR: https://gitlab.archlinux.org/archlinux/packaging/packages/raylib/-/blob/main/PKGBUILD?ref_type=heads
- Fedora: https://src.fedoraproject.org/rpms/raylib/blob/rawhide/f/raylib.spec
- Homebrew: https://github.com/Homebrew/homebrew-core/blob/79b0412c280d8a1283a2f38f444cad436261e1c9/Formula/r/raylib.rb
|
@uilianries huge thanks for taking the time to review this so thoroughly :) I see you removed the This was designed as a fix to #24472, but I guess it does not work without the customized build. So maybe doing it like the PR suggests is better? Btw, does turning on customize build by itself even do anything? Build log======== Exporting recipe to the cache ======== ======== Input profiles ======== Profile build: ======== Computing dependency graph ======== ======== Computing necessary packages ======== ======== Installing packages ======== -------- Installing package raylib/5.0 (3 of 3) -------- Update the VERSION argument value or use a ... suffix to tell -- Using Conan toolchain: C:/Users/-/.conan2/p/b/rayliee81208d77cdb/b/build/generators/conan_toolchain.cmake raylib/5.0: Running CMake.build() 1>Checking Build System raylib/5.0: Package 'ddd4486cb5134320eb1ec58b8e53a4272a2d2fad' built raylib/5.0: package(): Packaged 3 '.h' files: raylib.h, raymath.h, rlgl.h ======== Launching test_package ======== ======== Computing dependency graph ======== ======== Computing necessary packages ======== ======== Installing packages ======== ======== Testing the package ======== ======== Testing the package: Building ======== Update the VERSION argument value or use a ... suffix to tell -- Using Conan toolchain: C:/Users/--/cpp/libs/conan-center-index/recipes/raylib/all/test_package/build/msvc-193-x86_64-20-release/generators/conan_toolchain.cmake raylib/5.0 (test package): Running CMake.build() 1>Checking Build System ======== Testing the package: Executing test ======== |
|
@Julianiolo Thank you for pointing that detail! I'll discuss with Abril about it!
Yes, the upstream uses |
|
@uilianries my question is if enabling customize build actually changes anything in the result. Shouldn't it just produce the exact same thing if no other options are changed? |
|
@Julianiolo No, https://github.com/raysan5/raylib/blob/5.0/CMakeOptions.txt#L38
The option More information: https://cmake.org/cmake/help/latest/module/CMakeDependentOption.html |
|
@uilianries But shouldn't it still be ON if customize build is OFF, due to |
|
@Julianiolo Correct, but you may consider https://github.com/raysan5/raylib/blob/5.0/cmake/CompileDefinitions.cmake#L12 Without After talking with @AbrilRBS, we considered exposing those 3 options Speaking about the PR #24472, that PR is copying those extra headers, but the upstream does not copy and will not (It was asked directly to the upstream). So, do you need those headers as well? In case positive, will need to copy them according to the active option. |
Summary
Changes to recipe: raylib/*
Motivation
raylib defines a lot of build settings in
config.h. I added some of the most common/important ones to the recipe optionsDetails
We just patch the
config.hfile to match the options. This seems to be the easiest way to do this, and it makes theconfig.hfile mirror the current configuration (although that file is currently not exported; I guess this is relevant to that: #24472).