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

Add advanced ROBOTOLOGY_SKIP_ROBOTS_CONFIGURATION CMake option #1775

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

traversaro
Copy link
Member

@traversaro traversaro commented Dec 17, 2024

This is option is not meant to be used in general, but it can be useful in setups where there is a single robots-configuration installed outside of the robotology-superbuild that is shared by different robotology-superbuild, that are build in different environments or container images.

This option is meant to be used in specific contexts, so it will be documented there and is not documented (and it is marked as advanced, to be hidden by default) in the robotology-superbuild.

This option is meant to be used with the functionality added to robots-configuration in robotology/robots-configuration#697 .

@S-Dafarra
Copy link
Collaborator

I wonder if it makes sense to have a dummy Config.cmake for robots-configuration that simply sets FOUND to true

@traversaro
Copy link
Member Author

I wonder if it makes sense to have a dummy Config.cmake for robots-configuration that simply sets FOUND to true

Even if that was there, I am not sure if it useful, for the following reasons:

  • We would need to add the install folder of robots-configuration to CMAKE_PREFIX_PATH (but this is doable)
  • More importantly, despite the name find_or_build_package does not try to call find_package since a long time ago (I can't even find the PR), as that behavior was only an endless source of problems of users for which the robotology-superbuild was breaking as they were finding an old or wrongly configured version of YARP/iDynTree/etc/etc, while since we started ignoring system installed packages, I can't remember a single time in which this behaviour was a problem or a limitation (until this comment, at least : ) ).

@traversaro
Copy link
Member Author

traversaro commented Dec 17, 2024

  • since a long time ago (I can't even find the PR)

PR found: #389 . Note that a user can still set YCM_DISABLE_SYSTEM_PACKAGES to OFF, but I do not think anyone did that since a long time.

@S-Dafarra
Copy link
Collaborator

Clear! At this point, maybe it makes sense to add the definition of an env variable indicating where are located the configurations files. This would simplify the definition of the additional commands we currently use on the robot (see for example https://github.com/robotology/robots-configuration/blob/6b22848d0654505ea4277e0c6d109dea0c58509b/ergoCubSN000/extra/scripts/installRobotsConfiguration.sh#L11). Related comment: robotology/robots-configuration#698 (comment)

If you agree, I can also provide a PR for the setup.sh/.bat files

@traversaro
Copy link
Member Author

Clear! At this point, maybe it makes sense to add the definition of an env variable indicating where are located the configurations files. This would simplify the definition of the additional commands we currently use on the robot (see for example https://github.com/robotology/robots-configuration/blob/6b22848d0654505ea4277e0c6d109dea0c58509b/ergoCubSN000/extra/scripts/installRobotsConfiguration.sh#L11). Related comment: robotology/robots-configuration#698 (comment)

Sure, that probably should be placed in .bashrc_ergoCub/ .bashrc_iCub ?

@traversaro traversaro merged commit 62239e6 into master Dec 17, 2024
19 of 29 checks passed
@traversaro traversaro deleted the skiprobconf branch December 17, 2024 21:05
@traversaro
Copy link
Member Author

I also cherry-picked the change in the releases/2024.11, so it will end up in a possible 2024.11.2 release, see 568b758 .

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.

2 participants