MaterializeInc / rust-sasl

Cyrus SASL bindings for Rust
Apache License 2.0
4 stars 20 forks source link

Build fails on Windows #38

Closed pbor closed 2 years ago

pbor commented 2 years ago

Build fails on windows with the following error:

error[E0432]: unresolved import `libc::iovec`
  --> C:\omitted\sasl2-sys-0.1.16\src\sasl.rs:18:61
   |
18 | use libc::{c_char, c_int, c_uchar, c_uint, c_ulong, c_void, iovec};
   |                                                             ^^^^^ no `iovec` in the root

error[E0432]: unresolved import `libc::iovec`
  --> C:\omitted\sasl2-sys-0.1.16\src\saslplug.rs:18:61
   |
18 | use libc::{c_char, c_int, c_uchar, c_uint, c_ulong, c_void, iovec};
   |                                                             ^^^^^ no `iovec` in the root

I guess the parts that use iovec need to be conditional

pbor commented 2 years ago

It seems cyrus is doing this:

#ifdef _WIN32
/* Define to have the same layout as a WSABUF */
#ifndef STRUCT_IOVEC_DEFINED
#define STRUCT_IOVEC_DEFINED 1
struct iovec {
    long iov_len;
    char *iov_base;
};
#endif
#else
struct iovec;                    /* Defined in OS headers */
#endif

So I guess one way would be to do define a similar struct also in rust-sasl and use it instead of libc::iovec in the case of Windows

benesch commented 2 years ago

Huh, how are you compiling this on Windows? MinGW? Cygwin?

On Wed, Dec 29, 2021 at 4:45 AM Paolo Borelli @.***> wrote:

It seems cyrus is doing this:

ifdef _WIN32

/ Define to have the same layout as a WSABUF /

ifndef STRUCT_IOVEC_DEFINED

define STRUCT_IOVEC_DEFINED 1

struct iovec { long iov_len; char *iov_base; };

endif

else

struct iovec; / Defined in OS headers /

endif

So I guess one way would be to do define a similar struct also in rust-sasl and use it instead of libc::iovec in the case of Windows

— Reply to this email directly, view it on GitHub https://github.com/MaterializeInc/rust-sasl/issues/38#issuecomment-1002498437, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGXSICGHQL5RXMOP4QXH7LUTLKDDANCNFSM5K5TI7DQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>

pbor commented 2 years ago

MSVC

pbor commented 2 years ago

We build cyrus (and the other libraries we use) with this script: https://github.com/wingtk/gvsbuild

benesch commented 2 years ago

What makes configure and make work though?

pbor commented 2 years ago

I am not sure I follow the question... Note that this is not about the bundled version of cyrus, it is about the rust bindings themselves: rust-sasl is using libc::iovec but that does not exist on Windows, at least with the MSVC target. Thus we need to conditionally provide our own rust struct.

If you are asking about how gvsbuild builds cyrus, it seems to be calling nmake: https://github.com/wingtk/gvsbuild/blob/master/gvsbuild/projects.py#L184

benesch commented 2 years ago

Sorry, our messages crossed each other in the ether. This is the answer I was looking for:

We build cyrus (and the other libraries we use) with this script: https://github.com/wingtk/gvsbuild

Thanks!

Anyway, I think I understand now. You're not using the vendored feature (which doesn't work on Windows), but dynamically linking a copy of libsasl2 that you've managed to build yourself on Windows. That makes sense. I'm just trying to figure out how to get this wired up in CI so that I can actually test this.

pbor commented 2 years ago

https://github.com/gtk-rs/gtk-rs-core/blob/master/.github/workflows/windows.yml uses gvsbuild to build the entire GTK stack and store it in cache so that it does not need to rebuild it each time... I guess the same could be done for cyrus, lmdb, openssl etc

pbor commented 2 years ago

I hate to bug on open source maintainers... but is there anything I can help with to unblock this? Would the small build fix I mentioned in the first comment considered trivial enough to not require a CLA?