Re: [Pixman] [PATCH] Add CMake build rules

2016-05-22 Thread Emil Velikov
On 22 May 2016 at 10:47, whitequark  wrote:
> On 2016-05-22 09:20, Pekka Paalanen wrote:
>>
>> [snip]
>>
>> Assuming people were happy adding a new build system, which I very
>> much doubt,
>
>
> I've checked the mailing list of Cairo before starting this work.
> There have been two attempts at introducing a CMake buildsystem, and
> no conceptual objections were raised. While Pixman is not Cairo,
> I considered that enough to give this patch a try.
>
>> [snip]
>>
>> That seems to be the only thing you use from the current build
>> system. IOW, you are duplicating:
>> - all source file lists
>> - all build rules (built binaries)
>> - all hand-written code checking for compiler features
>> - config.h template
>
>
> Generating CMake and autoconf build rules from a shared source would
> surely result in a system that is more complex, and harder to understand,
> than both of those alone. I do not think that such a system would be
> of benefit to anyone.
>
Err... wrong ? The following is not too complex, right ?

set(pixman_sources
 parse_list("Makefile.sources" $(libpixman_sources))
)

Obviously one needs to add the said function. Sadly the other bits in
pq's list might be rather hard to implement, depending on your cmake
skills.

>> [snip]
>>
>> For what benefit? So that people who set up cross-build toolchains
>> and projects as their job have one less component to make that
>> setup for?
>
>
> So that people would avoid the setup of cross-build toolchains from
> becoming their job.
>
Can you elaborate how cmake makes thing easier ? I think that there's
some misunderstanding here, which brings you to false assumptions.
Please list the steps you have to do for each one.


As the person trying to juggle the Mesa build systems... I would
kindly urge you against the idea. Please look if the cross-build
toolchains thing is an actual issue, before introducing and/or
maintaining another build system.

Obviously my word hear means jack, so ...

-Emil
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman


Re: [Pixman] [PATCH] Add CMake build rules

2016-05-22 Thread Koen Kooi
Op 22-05-16 om 11:47 schreef whitequark:
> On 2016-05-22 09:20, Pekka Paalanen wrote:
>> [snip]
>> 
>> For what benefit? So that people who set up cross-build toolchains and
>> projects as their job have one less component to make that setup for?
> 
> So that people would avoid the setup of cross-build toolchains from 
> becoming their job.

LOL, cross-building with cmake is a joke. While core-cmake cross-build
functionality is awesome (a toolchain file!), the people writing the
FindLibFoo stuff are ignorantly hardcoding /usr/lib, /usr/include and
friends. And worse, patches for that are generally land on the bottom of the
review pile.

And for the record, cross-build toolchains and buidsystems *are* my job and
I hate cmake with a passion.

Even if all of the above magically gets fixed, having more than 1
buildsystem for a project is a drain on resources and usually ends up with
"But it *does* work in my pet system, WONTFIX" type of replies to people who
run into issues.

regards,

Koen


___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman


Re: [Pixman] [PATCH] Add CMake build rules

2016-05-22 Thread whitequark

On 2016-05-22 09:20, Pekka Paalanen wrote:

[snip]

Assuming people were happy adding a new build system, which I very
much doubt,


I've checked the mailing list of Cairo before starting this work.
There have been two attempts at introducing a CMake buildsystem, and
no conceptual objections were raised. While Pixman is not Cairo,
I considered that enough to give this patch a try.


[snip]

That seems to be the only thing you use from the current build
system. IOW, you are duplicating:
- all source file lists
- all build rules (built binaries)
- all hand-written code checking for compiler features
- config.h template


Generating CMake and autoconf build rules from a shared source would
surely result in a system that is more complex, and harder to 
understand,

than both of those alone. I do not think that such a system would be
of benefit to anyone.


[snip]

For what benefit? So that people who set up cross-build toolchains
and projects as their job have one less component to make that
setup for?


So that people would avoid the setup of cross-build toolchains from
becoming their job.

In any case, the purpose of submitting this patch is less of insisting
on action from upstream and more of sharing my work. If upstream 
considers

it valuable, I am willing to spend time coordinating the integration.
If not? That's fine by me as well; I'll just keep using the fork.

--
whitequark
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman


Re: [Pixman] [PATCH] Add CMake build rules

2016-05-22 Thread Pekka Paalanen
On Sun, 22 May 2016 01:54:36 +
whitequark  wrote:

> CMake is a popular build system for C/C++ projects. One key property
> it has is that a library using CMake can be "drop in" vendored,
> that is, placed in the build tree of another project using CMake
> and immediately used as if it was a part of that project.
> 
> There is no intermediate configuration step or build directories,
> and it is enough to set up a (perhaps cross-)toolchain once for
> the toplevel project. This results in a massive usability improvement
> for the case where a project links many static libraries, such as
> when deploying to Windows or Android, and especially when there is
> a combinatorial explosion of targets and target options, such as
> on Android, where it is necessary to produce x86, ARM and MIPS builds
> in order to target the majority of devices.
> 
> For this reason, many projects (including, for example, zlib and
> Freetype) opt to provide CMake build rules alongside autoconf ones.
> 
> These build rules closely follow the existing autoconf-based rules,
> to the degree that any change in autoconf files should trivially
> translate to changes in CMakeLists.txt, reducing maintainer workload.
> In particular, it uses the same code to test for target-specific
> compiler features.
> 
> There is no SunOS support, because SunOS is responsible for most
> corner cases in the autoconf build rules and it is unlikely that
> anyone is interested in using CMake for SunOS builds.
> 
> The CMake build rules were tested on:
>* x86_64-linux-gnu;
>* arm-linux-gnueabi;
>* mips-linux-gnu;
>* powerpc-linux-gnu.
> On all targets, the target features were manually verified to pass
> when they ought to, and the target-specific implementations were
> manually verified to be linked in.
> 
> On x86_64-linux-gnu, all tests were verified to pass.
> On other targets, all tests were verified to link correctly.
> 
> [actual patch included as attachment]

Hi,

please always put patches as inline.

This sounds like a very very bad idea to me, and I think the
execution is even worse.

If Mesa has tought us anything, it is that a project with multiple
build systems is much harder to maintain. Mesa struggles with it at
times, even when it has many active *paid* developers using both (I
think it is down to two now.. no, three: autotools, scons and
Android) build systems.

That said, Mesa actually tried to make that maintenance manageable,
which I do not really see from your patch.

Assuming people were happy adding a new build system, which I very
much doubt, let's look at your patch.

 8 files changed, 740 insertions(+)

No deletions.

You use regexes to fetch the version numbers from configure.ac,
right?

That seems to be the only thing you use from the current build
system. IOW, you are duplicating:
- all source file lists
- all build rules (built binaries)
- all hand-written code checking for compiler features
- config.h template

I believe that all that duplication will be a huge pain for
upstream. It is already labourious to ask people to test on various
platforms, and this will double the required effort.

Adding the CMake files to Pixman may be a one-shot effort for you,
but then the upstream maintainers and developers (who are very few
and already burdened) are stuck with it, now trying to learn
how CMake works, and spending hours verifying the build systems
are consistent (or letting CMake just rot and refuse all bug
reports from CMake-built binaries, in which case why even bother
merging in the first place).

For what benefit? So that people who set up cross-build toolchains
and projects as their job have one less component to make that
setup for?

If you could promise, as a company, to assign a developer whose job
will be to maintain this in Pixman, then just maybe it could make
sense, but so far I think this patch would be a turn for the worse
for Pixman.

Or would you perhaps offer a CI system automatically building with
both autotools and CMake and verifying they work the same on all
the platforms where you care about CMake?

Upstream should either fully endorse CMake as a new official build
system for Pixman, in which case you would try to share as much
configuration with autotools as possible, or, just reject it.

I wonder what others think?


Thanks,
pq


pgpMj8PWfKp8v.pgp
Description: OpenPGP digital signature
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman


[Pixman] [PATCH] Add CMake build rules

2016-05-21 Thread whitequark

CMake is a popular build system for C/C++ projects. One key property
it has is that a library using CMake can be "drop in" vendored,
that is, placed in the build tree of another project using CMake
and immediately used as if it was a part of that project.

There is no intermediate configuration step or build directories,
and it is enough to set up a (perhaps cross-)toolchain once for
the toplevel project. This results in a massive usability improvement
for the case where a project links many static libraries, such as
when deploying to Windows or Android, and especially when there is
a combinatorial explosion of targets and target options, such as
on Android, where it is necessary to produce x86, ARM and MIPS builds
in order to target the majority of devices.

For this reason, many projects (including, for example, zlib and
Freetype) opt to provide CMake build rules alongside autoconf ones.

These build rules closely follow the existing autoconf-based rules,
to the degree that any change in autoconf files should trivially
translate to changes in CMakeLists.txt, reducing maintainer workload.
In particular, it uses the same code to test for target-specific
compiler features.

There is no SunOS support, because SunOS is responsible for most
corner cases in the autoconf build rules and it is unlikely that
anyone is interested in using CMake for SunOS builds.

The CMake build rules were tested on:
  * x86_64-linux-gnu;
  * arm-linux-gnueabi;
  * mips-linux-gnu;
  * powerpc-linux-gnu.
On all targets, the target features were manually verified to pass
when they ought to, and the target-specific implementations were
manually verified to be linked in.

On x86_64-linux-gnu, all tests were verified to pass.
On other targets, all tests were verified to link correctly.

[actual patch included as attachment]

--
whitequarkFrom e526d0d1c318407462b3da7e54d1210eed445f48 Mon Sep 17 00:00:00 2001
From: whitequark 
Date: Sun, 22 May 2016 01:38:11 +
Subject: [PATCH] Add CMake build rules.

CMake is a popular build system for C/C++ projects. One key property
it has is that a library using CMake can be "drop in" vendored,
that is, placed in the build tree of another project using CMake
and immediately used as if it was a part of that project.

There is no intermediate configuration step or build directories,
and it is enough to set up a (perhaps cross-)toolchain once for
the toplevel project. This results in a massive usability improvement
for the case where a project links many static libraries, such as
when deploying to Windows or Android, and especially when there is
a combinatorial explosion of targets and target options, such as
on Android, where it is necessary to produce x86, ARM and MIPS builds
in order to target the majority of devices.

For this reason, many projects (including, for example, zlib and
Freetype) opt to provide CMake build rules alongside autoconf ones.

These build rules closely follow the existing autoconf-based rules,
to the degree that any change in autoconf files should trivially
translate to changes in CMakeLists.txt, reducing maintainer workload.
In particular, it uses the same code to test for target-specific
compiler features.

There is no SunOS support, because SunOS is responsible for most
corner cases in the autoconf build rules and it is unlikely that
anyone is interested in using CMake for SunOS builds.

The CMake build rules were tested on:
  * x86_64-linux-gnu;
  * arm-linux-gnueabi;
  * mips-linux-gnu;
  * powerpc-linux-gnu.
On all targets, the target features were manually verified to pass
when they ought to, and the target-specific implementations were
manually verified to be linked in.

On x86_64-linux-gnu, all tests were verified to pass.
On other targets, all tests were verified to link correctly.
---
 CMakeLists.txt | 355 +
 cmake/CheckCFlag.cmake |  13 ++
 cmake/CheckInline.cmake|  17 ++
 cmake/CheckTargetFeature.cmake |  19 +++
 cmake/config.h.in  |  63 
 demos/CMakeLists.txt   |  42 +
 pixman/CMakeLists.txt  | 162 +++
 test/CMakeLists.txt|  69 
 8 files changed, 740 insertions(+)
 create mode 100644 CMakeLists.txt
 create mode 100644 cmake/CheckCFlag.cmake
 create mode 100644 cmake/CheckInline.cmake
 create mode 100644 cmake/CheckTargetFeature.cmake
 create mode 100644 cmake/config.h.in
 create mode 100644 demos/CMakeLists.txt
 create mode 100644 pixman/CMakeLists.txt
 create mode 100644 test/CMakeLists.txt

diff --git a/CMakeLists.txt b/CMakeLists.txt
new file mode 100644
index 000..985b9de
--- /dev/null
+++ b/CMakeLists.txt
@@ -0,0 +1,355 @@
+project(pixman C ASM)
+cmake_minimum_required(VERSION 3.0)
+set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH}
+"${CMAKE_SOURCE_DIR}/cmake/")
+
+include(CheckTypeSize)
+include(CheckIncludeFile)
+include(CheckFunctionExists)
+include(CheckLibraryExists)