Re: WIP: About CMake v2

From: Yury Zhuravlev <u(dot)zhuravlev(at)postgrespro(dot)ru>
To: <pgsql-hackers(at)postgresql(dot)org>
Cc: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Subject: Re: WIP: About CMake v2
Date: 2016-09-16 16:40:48
Message-ID: 5a8a9610-722f-49df-b609-35842fe4c8d6@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Michael Paquier wrote:
> 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.

It's really a good point. Forbidden run cmake from root it is better
decision for me (of course for start).

>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.

You mean in first version of patch I can focus on Linux systems?

>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?

As long as possible. I hope I can make PGXS Makefiles generator.

>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.

Currently I do not have a lot OS specific tests. All checks are doing in
same manner.

>- root's .gitignore needs to add entries for CMakeFiles and
>cmake_install.cmake. You need as well to ignore CMakeCache

Thanks I done this in last commit.

>- A couple of headers are generated, like cubeparse.h (?)

Because BISON generate header by default. I suppose author of cube launched
bison by hand but I made it automatic.

>- 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.

It seems restriction by design because in CMake you have only one enter
point.

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

I merged master to my branch and I spent time to porting all changes. I
hope send patch in the weekend without terrible flaws.

--
Yury Zhuravlev
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2016-09-16 17:04:15 Re: Quorum commit for multiple synchronous replication.
Previous Message Artur Zakirov 2016-09-16 16:31:40 Re: Bug in to_timestamp().