Add Windows support

[auterium]

Achieving Windows support was not so simple. Here's a list of walls I came across:

1. Linux/OSX requires just 1 include path (include/server), while Windows requires 4:

include\server\port\win32_msvc
include\server\port\win32
include\server
include

2. By default, bindgen generates bindings for everything it can find. Under Windows, windows.h is included, which (if I'm not mistaken) generates bindings for all of Windows API. This causes to include a lot of things that are not properly supported. Keeping the default generates 138k LoC, while under Linux it generates 32k. With the included changes, the resulting postgres.rs file is 3198 LoC. I'd suggest a similar path is taken under Linux to reduce build time and binary size.
3. During build time, in the pg_create_stmt_bin! macro, the dylib extension is defined based on the OS. The extension for Windows was missing.
4. There are differences between Windows and Linux for the function names that Postgres uses. In one case, the signature of the function was different (1 arg in Windows, 2 in Linux).
5. Linker arg to ignore that some symbols won't be available until runtime is -undefineddynamic_lookup under Linux, while under Windows is /FORCE

With the included changes I was able to compile the adding example and successfully test with PG11 under Windows. I've not tested any other of the examples nor other PG versions.

[intgr]

This whitelist can limit the usefulness of pg-extend-rs; some extension modules use pg_sys to get access to additional PostgreSQL functions without needing to run bindgen themself.

Have you considered any alternatives to such whitelisting?

[auterium]

I agree it can limit the usefulness, but it generates a HUGE file if you leave as default everything:

• Linux generated file is 38k LoC
• Windows generated file is 132k LoC

I'm not confortable with having a hard-coded list, that's why I limited only for Windows while an alternative is figured out

[bluejekyll]

Could we introduce a feature flag that would enable all symbols vs. limit them?

“all-symbols” seems fairly obvious. How we dice and slice the smaller sets based on which features you’re enabling is a little harder to see.

[auterium]

The problem I encountered was that "allowing all" introuced a lot of things (if I'm not wrong, it imports the full Windows API) which I started by blacklisting them, but the list is so big, that it's simpler to do a whitelisting (which works recursively) of what was used from the crate.

As I mentioned, this whitelisting based approach is only for Windows. I left the "include all" approach for the unix family

[auterium]

I'm open to suggestions on how to tackle the white/black listing. One option would be to be able to pull the list of all the PG symbols and use it for the whitelisting, but I have no idea on how to do that or where to pull it from. If understand correctly how bindgen works, whitelisting that list of symbols will recursively include any non-PG symbols that the PG ones use

[intgr]

Even if it does include the entirety of Windows API headers, what problems does that cause? Looking at bindgen's issue tracker, it seems that's a common way to use it. (e.g. rust-lang/rust-bindgen#1562 "I'm pretty sure on Firefox bindgen ends up including windows.h without issues")

You said it's slow to compile; is that the only reason why you want whitelisting? How much time does it save?

If it's merely an optimization of build speed, I don't see why this needs to be Windows-specific at all. Omitting potentially useful symbols from a module depending on platform, for only build speed reasons, seems like very surprising behavior to me, likely to cause porting issues to downstream users.

If the speed gains are worthwhile, a feature flag, as suggested by @bluejekyll, sounds like a great idea. Then Unix builds could also take advantage of it, if desired. But that would better be done as a separate PR IMO, and strip whitelisting from this PR.

[auterium]

Thanks for the comment @intgr. The reason to not include all was not due to slow compilation, but because it includes problematic code that has issues at the Rust compiler level.

error[E0588]: packed type cannot transitively contain a `[repr(align)]` type
     --> E:\rust-projects\pg-extend-rs\target\release\build\pg-extend-7be983388aa96263\out/postgres.rs:32012:1
      |
32012 | / pub struct _IMAGE_TLS_DIRECTORY64 {
32013 | |     pub StartAddressOfRawData: ULONGLONG,
32014 | |     pub EndAddressOfRawData: ULONGLONG,
32015 | |     pub AddressOfIndex: ULONGLONG,
...     |
32018 | |     pub __bindgen_anon_1: _IMAGE_TLS_DIRECTORY64__bindgen_ty_1,
32019 | | }
      | |_^

error[E0277]: arrays only have std trait implementations for lengths 0..=32
     --> E:\rust-projects\pg-extend-rs\target\release\build\pg-extend-7be983388aa96263\out/postgres.rs:51363:5
      |
51363 |     pub __bindgen_padding_0: [u8; 40usize],
      |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::array::LengthAtMost32` is not implemented for `[u8; 40]`
      |
      = note: required because of the requirements on the impl of `std::fmt::Debug` for `[u8; 40]`
      = note: required because of the requirements on the impl of `std::fmt::Debug` for `&[u8; 40]`
      = note: required for the cast to the object type `dyn std::fmt::Debug`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0277`.
error: could not compile `pg-extend`.

I first tried the blacklisting approach but it was a difficult one so I moved to the whitelisting approach.

On the other hand (based on what I understood from @bluejekyll's comment, please correct me if I'm wrong), the list of whitelisted symbols could be based on feature flags, but I would still aim for the "include all" to be to whitelist all PG related things, which will recursivelly include any extra things that are required.

The fact that there's a ~130k LoC reduction was just a bonus, not the main reason for the PR

[intgr]

Looks like this issue

error[E0277]: arrays only have std trait implementations for lengths 0..=32

would be solved by updating bindgen. Our Cargo.lock file references bindgen 0.48.1 which is more than half a year old!

The other issue might be solved as well (rust-lang/rust-bindgen#1498?). Give it a shot.

[auterium]

I bumped bindgen to 0.51.1 and still the same errors

[intgr]

To nitpick, flags differ based on the linker used; on Windows, the MSVC link.exe is used.

[auterium]

Thanks for the clarification (I'm not familiar with C compialtion). The reason I put "based on OS family" is because the cargo config is doing the distinction by the target family. I'm open to better wording

[bluejekyll]

Ok, this has been discussed in the discord channel. The agreement is that while there is potentially a way to improve this in the future, this will be sufficient to get us started on Windows.

Thanks for the PR @auterium!