On Tue, Jan 3, 2017 at 11:11 PM, Peter Eisentraut
<peter.eisentr...@2ndquadrant.com> wrote:
> On 12/30/16 9:10 AM, Yuriy Zhuravlev wrote:
>> cmake_v2_2_c_define.patch
>> Small chages in c.h . At first it is “#pragma fenv_access (off)” it is
>> necessary if we use /fp:strict for MSVC compiler. Without this pragma we
>> can’t calc floats for const variables in compiller time (2 * M_PI for
>> example). Strict mode important if we want to be close with ieee754
>> float format on MSVC (1.0 / 0.0 = inf for example).  Detail info here:
>> https://msdn.microsoft.com/en-us/library/e7s85ffb.aspx
> I don't understand what this has to do with cmake.  If this is a
> worthwhile improvement for the Windows build, then please explain why,
> with a "before" and "after" output and a patch for the existing build
> system as well.
>> Second change is because I find and set HAVE_INT128 directly from CMake.
>> PG_INT128_TYPE used only for autoconfig scripts.
> It might also be worth refactoring the existing Autoconf code here to
> make this consistent.
> (My assumption is that if we were to move forward with cmake or any
> other build system change, we would have to keep the old one alongside
> at least for a little while.  So any changes to the C code would need to
> be side-ported.)
>> cmake_v2_3_rijndael.patch
>> First I added special wraparound because here CMake have circular
>> dependency (cmake very smart here). Second I removed rijndael.tbl
>> because it generated during build process every time.
> Please explain what the circular dependency is.  If there is one, we
> should also side-port this change.
>> cmake_v2_4_uuid.patch
>> Another small patch. Right place for uuid.h I find by CMake and not
>> necessary this ifdef hell.
> This patch removes the uuid.h include but doesn't add it anywhere else.
> How does it work?
>> Questions for discussion:
>> In generated project by CMake we always have only one enter point. Also
>> INSTALL macross support only including to “all” targets. It follows that
>> it is impossible build contrib modules separately only with “all”
>> target. Here write about this behavior:
>> https://cmake.org/cmake/help/v3.7/prop_tgt/EXCLUDE_FROM_ALL.html
> Yeah, I think this is how the MSVC stuff effectively works right now as
> well.

I am marking this patch as "returned with feedback". That's quite a
heavy change and it looks to be too late in the development cycle of
PG10 to consider it. Peter's commit bits, who is also the reviewer,
are beginning to smoke as well after everything that has happened for
the logical replication changes.

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

Reply via email to