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

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

Reply via email to