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 build #105

Closed
wants to merge 1 commit into from
Closed

cmake build #105

wants to merge 1 commit into from

Conversation

MerlinXYoung
Copy link

cmake build

@benhoyt
Copy link
Owner

benhoyt commented Jun 18, 2020

Hi - thanks for the contribution. I'm pretty uncertain about including all different build systems (especially for a tiny single-file library). For a start, it adds a bunch of noise files to the repo, and second, I'm not at all familiar with CMake so would have trouble reviewing this change.

Comment on lines +1 to +12
prefix=@prefix@
exec_prefix=${prefix}
libdir=@libdir@
includedir=@includedir@

Name: inih
Version: @PACKAGE_VERSION@
Description: ZeroMQ libuv.
URL: https://github.com/MerlinXYoung/inih

Libs: -L${libdir} -linih @LIBS@
Cflags: -I${includedir}
Copy link
Contributor

Choose a reason for hiding this comment

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

pkg-config files are already created by Meson, no need to add them to CMake.

Comment on lines +1 to +2
.vscode
build
Copy link
Contributor

Choose a reason for hiding this comment

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

Nothing that should be in an upstream git repo IMHO.

LIBRARY DESTINATION lib/$<CONFIG>
ARCHIVE DESTINATION lib/$<CONFIG>
)
endif()
Copy link
Contributor

Choose a reason for hiding this comment

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

No final newline.

configure_file(INIReader.pc.in INIReader.pc @ONLY)

install(TARGETS inih INIReader
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't entirely get why there needs a bindir needs to be set.

@stephanlachnit
Copy link
Contributor

stephanlachnit commented Jun 19, 2020

Overall I don't see why there needs to be a second build system, Meson is already there. Meson works under macOS, Windows, Linux and possibly every OS that can run Python.
It should be possible to have a CMake project using inih as an external project (https://cmake.org/cmake/help/latest/module/ExternalProject.html).
And also, a CMake build file is already available in vcpkg: https://github.com/microsoft/vcpkg/tree/master/ports/inih

@benhoyt
Copy link
Owner

benhoyt commented Oct 13, 2020

Closing due to lack of response, and I'd rather this were not included anyway due to me being unable to maintain N build systems.

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.

None yet

3 participants