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

[CMake] Make Examples Optional in CMAKE #216

Merged
merged 3 commits into from
Jan 26, 2025
Merged

Conversation

CJFEdu
Copy link
Contributor

@CJFEdu CJFEdu commented Jan 20, 2025

Makes including the examples optional. SDL2/SDL3 specifically have long download/build times. If they aren't being used, then this can dramatically speed up build times.

Makes including the examples optional.  SDL2/SDL3 specifically have long download/build times.  If they aren't being used, then this can dramatically speed up build times.
@FintasticMan
Copy link
Contributor

I'm not sure I understand what use case exists for running CMake with the flag disabled... The only thing the CMake project does is compile the examples, so with it disabled, CMake does nothing at all.

@CJFEdu
Copy link
Contributor Author

CJFEdu commented Jan 20, 2025

I'm not a CMAKE expert by any means, so I could be doing the following wrong. I'm using FetchContent to add Clay to my project. This adds the examples, which then adds all the dependencies.

That being said, this was just a quick solution to my specific problem. A more practical solution would probably be to have the option take a string to allow you to select which example you want to build. For example, someone that only wants to build the Raylib example shouldn't be required to download and build both versions of SDL.

@Robert-M-Muench
Copy link

See #213 same problem, same suggestion.

@nicbarker nicbarker changed the title Make Examples Optional in CMAKE [CMake] Make Examples Optional in CMAKE Jan 21, 2025
@nicbarker
Copy link
Owner

nicbarker commented Jan 21, 2025

Thanks for figuring this out! I'm fine with this as long as we flip the default option to ON, as I think it could be confusing if someone clones the repo and can't figure out why the examples won't build by default 🙂

Added separate CMAKE options to include each of the individual examples as well as one global option to include all examples.

I set build all examples to OFF as a default since downloading all of SDL 2 and 3 seems excessive.  Instead, the demo from the video and the website are turned on by default.

A note about these options should probably be added to the ReadMe as well, but I wasn't sure where the logical place to put it would be.
@CJFEdu
Copy link
Contributor Author

CJFEdu commented Jan 22, 2025

I added an update that makes the CMAKE build options more granular. I set build all examples to OFF as a default since downloading all of SDL 2 and 3 seems excessive. Instead, the demo from the video and the website are turned on by default.

These options should probably be included in the ReadMe somewhere, but I wasn't sure where the logical place to put it would be.

@nicbarker
Copy link
Owner

Sorry about the back and forth, but I think it would be best if we left the examples ON by default with the option to switch them off, both to prevent confusion and also because we build the examples through github actions on PRs to make sure they work 🙂

@Robert-M-Muench
Copy link

That's OK as well. As long as there is a way I can use the CMAKE stuff and configure it somehow to only use the CLAY code.

@CJFEdu
Copy link
Contributor Author

CJFEdu commented Jan 25, 2025

@nicbarker Done.

@nicbarker
Copy link
Owner

Looks good to me, thanks for the work here! 🙂

@nicbarker nicbarker merged commit ea6109b into nicbarker:main Jan 26, 2025
3 checks passed
Copy link

@rasmusortenholm rasmusortenholm left a comment

Choose a reason for hiding this comment

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

Copy link

@rasmusortenholm rasmusortenholm left a comment

Choose a reason for hiding this comment

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

CMakeLists.txt

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