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

pls add cmake support #731

Open
1vanK opened this issue Nov 9, 2024 · 13 comments
Open

pls add cmake support #731

1vanK opened this issue Nov 9, 2024 · 13 comments

Comments

@1vanK
Copy link

1vanK commented Nov 9, 2024

subj

@Ryder17z
Copy link
Contributor

it's header-only so this doesn't make sense.

@1vanK
Copy link
Author

1vanK commented Nov 10, 2024

it's have samples

@RobLoach
Copy link
Contributor

There was an effort a while ago to encompass everything with cmake #462

While I think that may be a bit overkill for our situation, it could give you something to try out.

I also maintained some cmake definition files a while ago. You could add a Nuklear definition to it: https://github.com/robloach/cmakepacks

@abc-JYL
Copy link

abc-JYL commented Nov 25, 2024

I don't think this repo need to add cmake, because in the example folder already have a makefile and it since quite great.

@purpasmart96
Copy link

No, keep it as is. Makefiles are far more simple.

@RobLoach
Copy link
Contributor

While 462 does everything, perhaps a simpler approach may be one that...

  • Doesn't handle building the demos
  • Exposes the target and the include directory
  • Keeps it to only one CMakeLists.txt file

@Ryder17z
Copy link
Contributor

Ryder17z commented Dec 16, 2024

While 462 does everything, perhaps a simpler approach may be one that...

....
Add #753 as optional dependency then because FreeType is already set up for cmake

@trexxet
Copy link

trexxet commented Jan 4, 2025

+1, would make it easier to integrate into existing projects
just making a target and exporting include path would be enough

@Ryder17z
Copy link
Contributor

Ryder17z commented Jan 4, 2025

+1, would make it easier to integrate into existing projects
just making a target and exporting include path would be enough

Your making it sound like it's a chore to add a single-header file to a project, which is not true

@trexxet
Copy link

trexxet commented Jan 4, 2025

+1, would make it easier to integrate into existing projects
just making a target and exporting include path would be enough

Your making it sound like it's a chore to add a single-header file to a project, which is not true

Keeping side project files in your repo, even if it's a single header, is generally not a good idea.

Yes, I can use cmake's FetchContent, setup paths and make target, but won't it be nicer just to write one line FetchContent_MakeAvaliable?

@Ryder17z
Copy link
Contributor

Ryder17z commented Jan 4, 2025

Keeping side project files in your repo, even if it's a single header, is generally not a good idea.

Then why is it so common to find a folder usually called "3rd party" or etc that contains such files? Even things like giant SDK's do this.

Yes, I can use cmake's FetchContent, setup paths and make target, but won't it be nicer just to write one line FetchContent_MakeAvaliable?

It would be, assuming that there's no obnoxious configuration that needs to be done.

@trexxet
Copy link

trexxet commented Jan 4, 2025

Anyway, if someone stumbles upon this, here is my solution that can be adjusted for your own needs:

include(FetchContent)

FetchContent_Declare(
	nuklear_src
	GIT_REPOSITORY https://github.com/Immediate-Mode-UI/Nuklear.git
	GIT_TAG 4.12.3
	GIT_SHALLOW TRUE
	GIT_PROGRESS TRUE
)
FetchContent_MakeAvailable(nuklear_src)
FetchContent_GetProperties(nuklear_src)

set(NUKLEAR_PREFIX_INCLUDE ${nuklear_src_SOURCE_DIR}/include/)
file(MAKE_DIRECTORY ${NUKLEAR_PREFIX_INCLUDE})
file(COPY_FILE
	${nuklear_src_SOURCE_DIR}/nuklear.h
	${NUKLEAR_PREFIX_INCLUDE}/nuklear.h)
file(COPY_FILE
	${nuklear_src_SOURCE_DIR}/demo/sdl_opengl3/nuklear_sdl_gl3.h
	${NUKLEAR_PREFIX_INCLUDE}/nuklear_sdl_gl3.h)

# here would be fixes for switching to SDL3

add_library(nuklear INTERFACE)
target_include_directories(nuklear INTERFACE ${NUKLEAR_PREFIX_INCLUDE})
add_library(nuklear::nk ALIAS nuklear)

@Xeverous
Copy link
Contributor

I have a Nuklear C++ wrapper library project and I can tell that if Nuklear maintainers don't want it it's probably because it is an extra burden that requires CMake knowledge and additional maintenance. I somewhat disagree with this decision too but I will honor it.

I'm currently writing CMake for my library and @trexxet I think your approach is flawed because Nuklear is not technically a header-only library. It just has a source - it's just embedded into its header. In practice this means that CMake should define it as STATIC, not INTERFACE, provide both header and source files and also expose any macros one might want to define for the project as target_compile_definitions(nuklear PUBLIC ...) so that descendant projects inherit the configuration.

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

No branches or pull requests

7 participants