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

Add basic support and documentation for CMake build system #51

Closed
wants to merge 7 commits into from
Closed

Conversation

ParticleG
Copy link
Contributor

No description provided.

@masatake
Copy link
Member

masatake commented May 6, 2024

Thank you. Could you tell me how to build libreadtags with your change?
I'm using Fedora. On the platform, cmake command is available.
I would like to add 'building libreadtags with cmake' to the test cases run on GitHub Actions.

@ParticleG
Copy link
Contributor Author

Thank you for your reply. I only use the main source files (readtag. c and readtag. h) to build into the static library. I currently do not have a Linux development environment, but you can try building it using the following command:

cd ${source_dir}
mkdir build_debug
cmake -DCMAKE_BUILD_TYPE=Debug -S . -B build_debug
cmake --build build_debug --target libreadtags -j ${cpu_thread_count}

@ParticleG
Copy link
Contributor Author

I'm currently configuring my WSL environment, I have never used autoconf or automake before. Can you provide me with a generated makefile? If you can also provide me with a makefile for the test directory, I should also be able to complete the CMakeList.txt for the test directory.

README.md Outdated Show resolved Hide resolved
Co-authored-by: K.Takata <kentkt@csc.jp>
CMakeLists.txt Outdated
cmake_minimum_required(VERSION 3.25)
project(libreadtags)

set(CMAKE_C_STANDARD 11)
Copy link
Member

Choose a reason for hiding this comment

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

Unlike ctags itself, I would like to keep libreadtags compilable with C99.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've just found AC-PROG-CC-C99 in configure.ac. I'll update that.

@masatake
Copy link
Member

masatake commented May 7, 2024

Could you tell me the use case of the file?

If you assume cmake as an alternative build system to autotools<1>, I expect cmake to provide libreadtags.so with soname and an install target.

If you assume cmake is just for building the library at a specified directory<2>, maybe the current version you submitted here is enough for merging.

@ParticleG
Copy link
Contributor Author

ParticleG commented May 7, 2024

Could you tell me the use case of the file?

If you assume cmake as an alternative build system to autotools<1>, I expect cmake to provide libreadtags.so with soname and an install target.

If you assume cmake is just for building the library at a specified directory<2>, maybe the current version you submitted here is enough for merging.

cmake could create an install target with install() method, and canbe executed with cmake --build ${build_dir} --target install. But I currently don't have the makefile generated by the autotools for CMake coding reference.

Current CMakeList.txt is enough for other developers to use libreadtags in their CMake project as a dependency library

@ParticleG
Copy link
Contributor Author

I am not very familiar with the autoconf+automake system. Based on my search results, the vast majority of configuration parameters should be in configure.ac and makefile.am? If that's true, maybe I could try to write an install target

@ParticleG
Copy link
Contributor Author

I'm using the command in the .circleci folder to configure the project. I see the LT_VERSION is [2:2:1] which would result in libreadtags.so.1.1.2, but the project version is 0.3.0 in AC_INIT step. May I ask why they are not the same?

- Add options to build shared library
- Update README.md
@ParticleG
Copy link
Contributor Author

ParticleG commented May 7, 2024

I've updated README.md for how to build & install a shared lib. If you follow these commands, it should generate libreadtags.so and corresponding sonames (Current are libreadtags.so.1 and libreadtags.so.1.1.2), and install to the /usr/local directory.

If there's anything still missing (like installing man or other stuff), just tell me to add them

@masatake
Copy link
Member

masatake commented May 7, 2024

I'm using the command in the .circleci folder to configure the project. I see the LT_VERSION is [2:2:1] which would result in libreadtags.so.1.1.2, but the project version is 0.3.0 in AC_INIT step. May I ask why they are not the same?

LT_VERSION specifies the version of the library interface.
So even if we add a commit, we will not update the LT_VERSION as far as the interface is not changed. e.g. fixing a bug in a test case.

Could you give me time? I must understand your changes well.

@ParticleG
Copy link
Contributor Author

I'm using the command in the .circleci folder to configure the project. I see the LT_VERSION is [2:2:1] which would result in libreadtags.so.1.1.2, but the project version is 0.3.0 in AC_INIT step. May I ask why they are not the same?

LT_VERSION specifies the version of the library interface. So even if we add a commit, we will not update the LT_VERSION as far as the interface is not changed. e.g. fixing a bug in a test case.

Could you give me time? I must understand your changes well.

Of course

@ParticleG
Copy link
Contributor Author

ParticleG commented May 9, 2024

I'm using the command in the .circleci folder to configure the project. I see the LT_VERSION is [2:2:1] which would result in libreadtags.so.1.1.2, but the project version is 0.3.0 in AC_INIT step. May I ask why they are not the same?

LT_VERSION specifies the version of the library interface. So even if we add a commit, we will not update the LT_VERSION as far as the interface is not changed. e.g. fixing a bug in a test case.

Could you give me time? I must understand your changes well.

I forgot to commit README.md, sorry. Now you could try building it with instructions in README.md

@masatake
Copy link
Member

I'm sorry for working on this pull request so slowly.
I cannot find a bulk time to concentrate on this topic.

https://github.com/openSUSE/libsolv/tree/master may have all I must study to apply cmake to libreadtags.

I wonder we should prepare FindLibReadtags.cmake like https://github.com/openSUSE/libsolv/blob/master/cmake/modules/FindLibSolv.cmake .

My concerns (and interests):

  • Unify the places where the maintainer specifies various versions.
  • Run the test cases as we do in autotools. (both locally and CI/CD environments)
  • Generate and install libreadtags.pc as we do in autotools.
  • Prepare make install target as we do in autotools.
  • Prepare FindLibSolv.cmake and the way to install it.
  • ...

@ParticleG
Copy link
Contributor Author

ParticleG commented Jun 4, 2024

I'm sorry for working on this pull request so slowly. I cannot find a bulk time to concentrate on this topic.

https://github.com/openSUSE/libsolv/tree/master may have all I must study to apply cmake to libreadtags.

I wonder we should prepare FindLibReadtags.cmake like https://github.com/openSUSE/libsolv/blob/master/cmake/modules/FindLibSolv.cmake .

My concerns (and interests):

  • Unify the places where the maintainer specifies various versions.
  • Run the test cases as we do in autotools. (both locally and CI/CD environments)
  • Generate and install libreadtags.pc as we do in autotools.
  • Prepare make install target as we do in autotools.
  • Prepare FindLibSolv.cmake and the way to install it.
  • ...

Actually, if we prepare CMakeList.txt properly, other CMake projects just need to use find_package method to load this package, see Finding Packages. But if you use CMake method FetchContent to add libreadtags to your project, then current changes should be enough. I've nerver tried to create proper "CMake Package Configuration Files", since I'm coding on Windows and use vcpkg to manage CMake packages. Maybe I'll try that in my spare time.

By the way, OpenSUSE's CMake standard is too old (2.x.x) which is deprecated according to CMake official docs. If you want to learn more about CMake, you should look for something more modern (> 3.5), like IXWebSocket, it also have FindXXX.cmake in it FindDeflate.cmake

@masatake
Copy link
Member

masatake commented Jun 14, 2024

Sorry for making you wait so long.
Looks good. The changes for README.md is excellent. All I wanted to know is written in the changes.

I'll take over this to adjust minor things.
See #53.

@masatake masatake closed this Jun 14, 2024
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.

3 participants