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 config #3

Merged
merged 10 commits into from
Jun 16, 2023
Merged

CMake build config #3

merged 10 commits into from
Jun 16, 2023

Conversation

guoh27
Copy link
Contributor

@guoh27 guoh27 commented Jun 2, 2023

No description provided.

@gmarcais
Copy link
Owner

gmarcais commented Jun 2, 2023

Not to get into a compilation system war, but is there a compelling reason to use CMake that Autotools does not cover? It is a relatively simple project, I am more used to Autotools and don't yet see a clear benefit to switching.

The proposed CMakelists.txt does not generate the noshell.pc file and does not support testing.

@guoh27
Copy link
Contributor Author

guoh27 commented Jun 3, 2023

Now cmake is full available.
It will get the version from configure.ac.
And contains both installation and testing content.

@guoh27 guoh27 changed the title add CMakeLists.txt and fix a no inline error CMake build config Jun 3, 2023
@gmarcais gmarcais changed the base branch from master to develop June 3, 2023 11:22
@gmarcais
Copy link
Owner

gmarcais commented Jun 3, 2023

That looks more promising. I changed the base to branch develop. Also, when trying your PR, I get the following error:

gusqt@gus-laptop ~/D/NoShell  1 > cmake -S . -B build-cmake                                                                                          45163d1?
CMake Warning (dev) at /usr/share/cmake/Modules/GNUInstallDirs.cmake:243 (message):
  Unable to determine default CMAKE_INSTALL_LIBDIR directory because no
  target architecture is known.  Please enable at least one language before
  including GNUInstallDirs.
Call Stack (most recent call first):
  CMakeLists.txt:4 (include)
This warning is for project developers.  Use -Wno-dev to suppress it.

@guoh27
Copy link
Contributor Author

guoh27 commented Jun 3, 2023

An exists issue #409

@gmarcais
Copy link
Owner

gmarcais commented Jun 3, 2023

It seems to compile and run the test properly, and I should merge that PR. I have one more request: although I did ask to be able to run the tests, can the dependency on gtest be optional? One should be able to compile and install the library even if gtest is not installed. If gtest is not installed, the testing programs are not compiled and at most a warning is printed.

@guoh27
Copy link
Contributor Author

guoh27 commented Jun 3, 2023

Yes, gtest is optional, you can turn off the build of tests by passing -DNOSHELL_BUILD_TESTS=OFF to cmake, for example cmake -S . -B build -DNOSHELL_BUILD_TESTS=OFF,
At the same time, when NoShell is the topmost project, this option is ON by default, and when it is in another project's build tree, this option is OFF by default.

@gmarcais gmarcais merged commit 8a94922 into gmarcais:develop Jun 16, 2023
@gmarcais
Copy link
Owner

Thank you for the PR. I merged it. It will appear in the next version.

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