On Fri, Sep 16, 2016 at 9:58 AM, Michael Paquier
<michael.paqu...@gmail.com> wrote:
> Okay. That sounds good to me. I donas well a look around, and cmake really 
> looks like a robust alternative to ./configure. Now I am aware of the fact 
> that your patch recommends to build 't recall what your patch is
> exactly doing but could you still keep the vanilla Makefiles around?
> This will reduce the diff of the patch, and we'd need anyway to keep
> the former Makefile methods around until the buildfarm scripts are
> updated, the animals do the switch and then get green. So a period
> where both live in parallel is unavoidable.
>
> I heard as well that MySQL is using cmake... It may be interesting to
> see how they are integrating with it as a large project and avoid
> their past mistakes.

I could not resist so I just had a look at your patch. I had as well a
look around, and cmake really looks like a robust alternative to
./configure. In short, people doing now that:
./configure
make
make install
Would just do that:
cmake .
make
make install

I am no cmake guru yet, but VPATH builds are supported out of the box,
which is cool.

Your patch recommends to build with cmake after creating build/, now I
would expect most users to run cmake from the root folder. However
this causes all the Makefiles to get overwritten. As supporting all
platforms at once with cmake is going to be uncommitable, we are going
to need both methods able to live together for a while. Well, they can
coexist with this patch as long as cmake is not run from the root of
the code tree, which is acceptable for me as long as the switch is not
completed. However others may think differently on the matter.

Instead of getting support for all existing platforms, I would
recommend as well focusing only on one platform for the time being,
Linux, and get the work done correctly for that first. Once there is
something committed, we will be able to patch the buildfarm, and get
machines to switch to cmake one by one. After those are switched, we
could extend that. Another point of contention is support for
extensions. How long should we keep support for the existing PGXS? How
external extensions would compile with the new thing infrastructure?

Which brings me to another point, your patch is focused on features,
meaning that per-OS checks are all grouped by feature, but it may be a
good idea to split checks by OS if necessary, with for example
per-platform files and scripts in cmake/os/. And we could have just
something for Linux now.

A couple of issues I have noticed with your patch after a first set of tests:
- root's .gitignore needs to add entries for CMakeFiles and
cmake_install.cmake. You need as well to ignore CMakeCache
- A couple of headers are generated, like cubeparse.h (?)
- Currently a lot of users do things like that:
cd src/test/regress/ && make check
But this patch breaks that, and that's not cool. Recovery tests in
src/test/regress won't run either.

That's all I have for now. Looking forward to seeing some progress here.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to