[GH-ISSUE #168] cmake compatibility #141

Closed
opened 2026-05-05 11:46:04 -06:00 by gitea-mirror · 10 comments
Owner

Originally created by @remicollet on GitHub (Apr 27, 2018).
Original GitHub issue: https://github.com/jmcnamara/libxlsxwriter/issues/168

Originally assigned to: @jmcnamara on GitHub.

Despite

cmake_minimum_required(VERSION 2.8.7)

It fails with cmake 2.8.12

CMake Error at CMakeLists.txt:171 (target_sources):
  Unknown CMake command "target_sources".

Originally created by @remicollet on GitHub (Apr 27, 2018). Original GitHub issue: https://github.com/jmcnamara/libxlsxwriter/issues/168 Originally assigned to: @jmcnamara on GitHub. Despite cmake_minimum_required(VERSION 2.8.7) It fails with cmake 2.8.12 ``` CMake Error at CMakeLists.txt:171 (target_sources): Unknown CMake command "target_sources". ```
gitea-mirror 2026-05-05 11:46:04 -06:00
  • closed this issue
  • added the
    bug
    cmake
    labels
Author
Owner

@jmcnamara commented on GitHub (Apr 27, 2018):

@Alexhuszagh Can you take a look at this as well.

Or @remicollet can you submit a Pull Request.

<!-- gh-comment-id:384913129 --> @jmcnamara commented on GitHub (Apr 27, 2018): @Alexhuszagh Can you take a look at this as well. Or @remicollet can you submit a Pull Request.
Author
Owner

@remicollet commented on GitHub (Apr 27, 2018):

The problem is that I'm not a cmake expert, so not able to tell which is the right required version... only that

  • fails with cmake 2.8.12
  • passes with cmake 3.6.1 (the older 3.x version I have on Fedora/RHEL)
<!-- gh-comment-id:384915000 --> @remicollet commented on GitHub (Apr 27, 2018): The problem is that I'm not a cmake expert, so not able to tell which is the right required version... only that * fails with cmake 2.8.12 * passes with cmake 3.6.1 (the older 3.x version I have on Fedora/RHEL)
Author
Owner

@remicollet commented on GitHub (Apr 27, 2018):

According to cmake changelog (in 3.1rst)


* The :command:`target_sources` command was added to add to the
  :prop_tgt:`SOURCES` target property.

<!-- gh-comment-id:384916874 --> @remicollet commented on GitHub (Apr 27, 2018): According to cmake changelog (in 3.1rst) ``` * The :command:`target_sources` command was added to add to the :prop_tgt:`SOURCES` target property. ```
Author
Owner

@jmcnamara commented on GitHub (Apr 27, 2018):

The problem is that I'm not a cmake expert,

Neither am I. That's why @Alexhuszagh normally handles these issues.

I'm guessing that you are looking at this from the point of view of packaging. Is your preference to use cmake instead of make?

The make build system is still the primary build system for libxlsxwriter. Cmake is the secondary build system.

<!-- gh-comment-id:384943944 --> @jmcnamara commented on GitHub (Apr 27, 2018): > The problem is that I'm not a cmake expert, Neither am I. That's why @Alexhuszagh normally handles these issues. I'm guessing that you are looking at this from the point of view of packaging. Is your preference to use cmake instead of make? The make build system is still the primary build system for libxlsxwriter. Cmake is the secondary build system.
Author
Owner

@remicollet commented on GitHub (Apr 27, 2018):

I'm guessing that you are looking at this from the point of view of packaging.

Yes

Is your preference to use cmake instead of make?

No, but I notice a CMakeFile so I use cmake ;)
And I always consider as suspect a static Makefile (without cmake/autotools...)

<!-- gh-comment-id:384946900 --> @remicollet commented on GitHub (Apr 27, 2018): > I'm guessing that you are looking at this from the point of view of packaging. Yes > Is your preference to use cmake instead of make? No, but I notice a CMakeFile so I use cmake ;) And I always consider as suspect a static Makefile (without cmake/autotools...)
Author
Owner

@remicollet commented on GitHub (Apr 27, 2018):

I'm guessing that you are looking at this from the point of view of packaging.

For your information I need this library for the upcoming new pecl extension excel-writer which offers bindings for this library

And if everything works as expected, RPM for Fedora / RHEL / CentOS will be available in my repository which is mostly provides PHP stuff, but is also an incubator for official Fedora/EPEL repositories.

<!-- gh-comment-id:384948811 --> @remicollet commented on GitHub (Apr 27, 2018): > I'm guessing that you are looking at this from the point of view of packaging. For your information I need this library for the upcoming new pecl extension [excel-writer](https://github.com/viest/php-ext-excel-export) which offers bindings for this library And if everything works as expected, RPM for Fedora / RHEL / CentOS will be available in my [repository ](http://rpms.remirepo.net/) which is mostly provides PHP stuff, but is also an incubator for official Fedora/EPEL repositories.
Author
Owner

@Alexhuszagh commented on GitHub (Apr 27, 2018):

FYI, @jmcnamara, I like @remicollet's commit with very minor, minor exceptions. I'm hoping to have a fix later today.

<!-- gh-comment-id:384986091 --> @Alexhuszagh commented on GitHub (Apr 27, 2018): FYI, @jmcnamara, I like @remicollet's commit with very minor, minor [exceptions](https://github.com/remicollet/libxlsxwriter/commit/80ceba65a9cbdf42d5b83612587a150de563dddd#commitcomment-28768303). I'm hoping to have a fix later today.
Author
Owner

@Alexhuszagh commented on GitHub (Apr 27, 2018):

@remicollet Good catch on the target_sources, I updated the library generation a few months ago but forgot to test the minimum version. My fault, very well done.

<!-- gh-comment-id:384986911 --> @Alexhuszagh commented on GitHub (Apr 27, 2018): @remicollet Good catch on the `target_sources`, I updated the library generation a few months ago but forgot to test the minimum version. My fault, very well done.
Author
Owner

@Alexhuszagh commented on GitHub (Apr 27, 2018):

The main issue now with multi-arch is the fact that -m32 can be passed to the CMake flags if intentionally aiming to build a 32-bit library on a 64-bit system, but this will still install to lib64.

The proper, CMake way to do this would likely be to have a separate toolchain for 32-bit toolchain that specifies the proper CMake system processor, and then could be used during project configuration, rather than "Just another option" would be to add a toolchain file for 32-bit code generation.

This correctly tells CMake that the compiler is 32-bit, not 64-bit, updates the system name, and allows Cmake to correctly determine an installation path of lib or lib64 as a resylt.

I'll also update the CMake documentation as a result.

<!-- gh-comment-id:384993906 --> @Alexhuszagh commented on GitHub (Apr 27, 2018): The main issue now with multi-arch is the fact that `-m32` can be passed to the CMake flags if intentionally aiming to build a 32-bit library on a 64-bit system, but this will still install to lib64. The proper, CMake way to do this would likely be to have a separate toolchain for 32-bit toolchain that specifies the proper CMake system processor, and then could be used during project configuration, rather than "Just another option" would be to add a toolchain file for 32-bit code generation. This correctly tells CMake that the compiler is 32-bit, not 64-bit, updates the system name, and allows Cmake to correctly determine an installation path of `lib` or `lib64` as a resylt. I'll also update the CMake documentation as a result.
Author
Owner

@jmcnamara commented on GitHub (Apr 27, 2018):

P.S. @remicollet and @Alexhuszag. Thanks for the input. In future PRs can you avoid putting defect numbers in the subject line. You can put them in the body of the commit message.

<!-- gh-comment-id:385064880 --> @jmcnamara commented on GitHub (Apr 27, 2018): P.S. @remicollet and @Alexhuszag. Thanks for the input. In future PRs can you avoid putting defect numbers in the subject line. You can put them in the body of the commit message.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/libxlsxwriter#141
No description provided.