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: http://www.postgresql.org/mailpref/pgsql-hackers
