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

[WIP] Experimental bootstrap using FetchContent #473

Closed
wants to merge 4 commits into from

Conversation

drdanz
Copy link
Member

@drdanz drdanz commented Sep 18, 2020

This is an experiment where I'm using a customized version of the YCMBootstrap.cmake module, that removes a lot of madness, include_url, etc., and instead it uses FetchContent to download YCM.

These are a few open points to discuss and/or to fix:

  • YCM is no longer handled by ExternalProject like all the other projects.
  • Where is USE_SYSTEM_YCM defined?
  • We might also want to consider that using FetchContent we could avoid having the YCMBootstrap.cmake file in the repository.
  • YCM_TAG must now be defined somewhere.
  • The check on the YCMBootstrap version is no longer performed
  • Where is YCM_EP_INSTALL_DIR defined?

@traversaro what do you think?

@traversaro
Copy link
Member

One meta question: why we want to use FetchContent to then install the YCM project? Personally to keep thing simple, what I was thinking by migrating the bootstrap to FetchContent was:

  • Use FetchContent to download a YCM version that can be used by just doing:
FetchContent_GetProperties(YCM)
if(NOT ycm_POPULATED)
  FetchContent_Populate(YCM)
  # This or something like include a cmake file that is in ${ycm_SOURCE_DIR}
  list(APPEND CMAKE_MODULE_PATH ${ycm_SOURCE_DIR}/modules)
endif()

Due to al the downloading from third party repos in YCM, probably this means that FetchContent will needs to be done from an offline archive.

Once the appropriate YCM modules are available, then a different YCM version is downloaded as done for all other projects, by having a BuildYCM.cmake file in https://github.com/robotology/robotology-superbuild/tree/master/cmake .

@drdanz
Copy link
Member Author

drdanz commented Sep 18, 2020

That's what I wanted to do at the beginning, but the

  list(APPEND CMAKE_MODULE_PATH ${ycm_SOURCE_DIR}/modules)

line was actually a lot more complicated than that, because the sub-projects (and probably also 3rd party projects) should be able to find YCM.

To clarify, YCM and it's modules must be available in the configure phase and must be found in a "standard" path during the build phase, so that the subprojects can find it.

An option to avoid the build-commands-executed-during-the-configure-phase-madness would be to download the YCM-<version>-all.tar.gz package, but that makes it impossible to test the modules in YCM before the release.

Therefore I am now considering to support 2 different options in the YCMBootstrap.cmake file.

Another alternative could be to have some branches auto-generated using the github actions, which could download the files, after each push, and save either the "offline" version (that requires running cmake and make in the configure phase) or the "installation" version (slightly more complicated than the other method).
Or we could use the artifacts to save the packages, that would be probably a better option.

So the options that I see are these:

Work needed (YCM) Work needed (superbuilds) Requires building at configure time Requires network (after the main download) Supports YCM development (commit, push, etc) Supports YCM stable/devel branches Requires Modules from YCM in the project
Keep using current system NO NO YES YES YES YES YES (YCMBootstrap, IncludeUrl)
FetchContent + "binary" package NO YES (change bootstrap method) NO NO NO NO NO
FetchContent + "offline" package NO YES (change bootstrap method) YES NO NO NO NO
FetchContent + "source" package NO YES (change bootstrap method) YES YES NO YES NO
FetchContent + generated "binary" packages for each commit with gh-actions + artifacts YES YES (change bootstrap method) NO NO NO YES NO
FetchContent + generated "offline" packages for each commit with gh-actions + artifacts YES YES (change bootstrap method) YES NO NO YES NO
FetchContent + generated "binary" branch for each commit with gh-actions YES YES (change bootstrap method) NO NO NO YES NO
Generate "offline" branch for each commit with gh-actions YES YES (change bootstrap method) YES NO Partially YES NO
YCMBootstrap + FetchContent + git branch YES (not much) YES (update YCMBootstrap) YES YES YES YES YES (YCMBootstrap)
Hybrid bootstrap that supports development if needed YES YES (update YCMBootstrap) In development mode YES, in normal user mode NO In development mode YES, in normal user mode NO In development mode YES, in normal user mode MAYBE In development mode YES, in normal user mode MAYBE YES (YcmBootstrap)

This draft is, in my mind, the first test towards the "hybrid bootstrap"... But perhaps the effort is too much compared to the benefits. Nonetheless we should definitely support the stable/development branches...

@traversaro
Copy link
Member

That's what I wanted to do at the beginning, but the

  list(APPEND CMAKE_MODULE_PATH ${ycm_SOURCE_DIR}/modules)

line was actually a lot more complicated than that, because the sub-projects (and probably also 3rd party projects) should be able to find YCM.

To clarify, YCM and it's modules must be available in the configure phase and must be found in a "standard" path during the build phase, so that the subprojects can find it.

But to simplify this logic, can't we just download YCM two times, once via FetchContent for the the bootstrap procedure, and the second time via ExternalProject/YCMEpHelper to install it so that it can be found by all the other projects?

@drdanz
Copy link
Member Author

drdanz commented Nov 17, 2020

It could work, but I'm afraid this might be a bit confusing...

@traversaro
Copy link
Member

It could work, but I'm afraid this might be a bit confusing...

I think that having just one project that is not installed as the others is also confusing, and having the two YCM may be a reasonable compromise.

@drdanz
Copy link
Member Author

drdanz commented Mar 25, 2021

I'm closing this for now...

@drdanz drdanz closed this Mar 25, 2021
@traversaro traversaro deleted the FetchContent_bootstrap branch June 18, 2022 09:49
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