PDCursesMod icon indicating copy to clipboard operation
PDCursesMod copied to clipboard

CMake Subproject Changes

Open rmorales87atx opened this issue 4 years ago • 6 comments

Hi,

I made some rudimentary changes to the CMake files so that I could include PDCursesMod as a "subproject".

I'm by no means an expert on CMake so I'm hoping I didn't mess anything else up. I'm sure this can be improved as well.

Example that will essentially only build the VT port as a static library:

	option(PDC_WIDE "" ON)
	option(PDC_BUILD_SHARED "" OFF)
	option(PDC_CHTYPE_32 "" OFF)
	option(PDC_OS2_BUILD "" OFF)
	option(PDC_DOS_BUILD "" OFF)
	option(PDC_DOSVGA_BUILD "" OFF)
	option(PDC_DOSVT_BUILD "" OFF)
	option(PDC_SDL2_BUILD "" OFF)
	option(PDC_SDL2_DEPS_BUILD "" OFF)
	option(PDC_NCURSES_BUILD "" OFF)
	option(PDC_DEMOS_BUILD "" OFF)
	add_subdirectory(PDCursesMod)
	list(APPEND LIBS vt_pdcursesstatic)

	# .. more stuff adding onto ${LIBS} ..

	target_link_libraries(my_app ${LIBS})

rmorales87atx avatar Apr 23 '21 00:04 rmorales87atx

Thank you; this looks as if it ought to be quite a help for CMake/PDCursesMod users (of whom there appear to be a fair number).

However, I'm not one of them (still using 'traditional' makefiles) and I am in no way able to comment on CMake matters. @jmalak @jwinarske @sherpya @mannyamorim @GitMensch ... this is really your chunk of the code; any thoughts?

Bill-Gray avatar Apr 24 '21 18:04 Bill-Gray

It "looks" good to me too, but I normally don't use cmake so I can't say much to this PR (just fixed one conflict). @rmorales87atx Do we need PDC_DOSVT_BUILD and PDC_VT_BUILD?

Should we have at least one of those "as subprojects" build added to Appveyor for testing (not sure how again...)?

@jmalak @jwinarske As Bill I'd really appreciate an actual review.

GitMensch avatar Jan 07 '22 22:01 GitMensch

@GitMensch I can take a look at this in a few weeks. I'm pretty busy right now. This PR requires creating some test cases that don't exist. It's not suitable for merging just yet.

jwinarske avatar Jan 11 '22 22:01 jwinarske

@jwinarske Could you please elaborate what test cases you are talking about? I am also interested in seeing this PR merged and am willing to help.

dmitmel avatar Feb 21 '23 21:02 dmitmel

@dmitmel I just worked out a better solution. I may be able to submit PR tomorrow.

jwinarske avatar Feb 22 '23 04:02 jwinarske

By the way, this PR also needs to change the way compilation settings (include_directories, target_link_libraries etc) are set in cmake/project_common.cmake because currently they are not propagated (set with the PUBLIC modifier, in other words) to dependent targets. In particular I've had problems with include paths and linkage, temporarily fixed with:

target_include_directories(my_target PUBLIC ${pdcurses_SOURCE_DIR})
target_link_libraries(my_target PRIVATE winmm.lib)  # This is just for Windows, obviously

dmitmel avatar Mar 02 '23 08:03 dmitmel