-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
module/rust: set _FILE_OFFSET_BITS=64 for bindgen #15009
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Can you use |
@bonzini No, I can't. Or, at least I need help to do that. I am open to consolidate it with |
That's fair :) I was thinking of simply using the host C compiler so that if it is MSVC, which doesn't have automatic support for large files, you won't add the option to You can get it with This leaves out the case of wanting to use bindgen for the build machine, but that's not really supported at all by the |
This will fail if no C-compiler is configured. IIRC, this could be the case when using meson for Rust with bindgen, but not adding 'c' as language to Meson. Maybe I am wrong about this? Should we force detection of a C compiler when using bindgen? Or does Meson already do this? If we proceed with this: Should I use |
Hmm, good catch. I would not add the flag (the chance is small anyway, even more so if you add the chance of having off_t change the output) and warn.
I agree. |
Thanks! I updated the code to check for the configured compiler and use |
Should we also check for c++, objc, and objc++ before giving up? It seems reasonable that someone could be building C++ + Rust and using C as an interface ABI. |
@dvdhrm: What do you think about the C++ thing? I'm not 100% on ObjC and ObjC++, I know that bindgen has some support for them, but not how much |
Oh, this was for me? Honestly, no idea. Not sure I know too much about the internals of meson to judge that. Shall I just implement it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the approach here is fine, but need to figure out the other languages problem.
Yes, I think @dcbaker is asking for basically for lang in ['c', 'cpp', 'objc', 'objcpp']:
comp = state.environment.coredata.compilers.host.get(lang, None)
if comp:
clang_args.extend(comp.get_always_args())
break
else:
mlog.warning(textwrap.dedent('''\
etc. (for/else to the rescue :)) |
@dvdhrm ping :) if you don't have time just let me know |
Thanks for the patience. I pushed the suggested changes! |
Meson sets 64-bit offsets as the default for all platforms but MSVC. Lets do the same for bindgen, to ensure we get compatible definitions. Do this by calling `get_always_args()` on the first C'ish host compiler we can find. Note that the `libc` crate does not expose 64-bit types as the default and there is no intention to do so. Instead, it exposes 32-bit default types, plus the 64-bit extended types with the `*64` suffix. This is quite unfortunate, but it seems unlikely to change [1]. However, use of `bindgen` is usually not tied to the `libc` crate. Instead, it is tied to whatever other C code in the same project does. And Meson sets `_FILE_OFFSET_BITS=64` unconditionally for all this C code. It thus seems much more plausible for Meson to also imply it for bindgen. Given that Rust code that uses the `libc` crate very likely already uses the `*64` suffixed variants, they are unaffected by whether `_FILE_OFFSET_BITS=64` is set. If they use `libc::off_t`, they already explicitly state that they use the 32-bit variant on 32-bit platforms. Hence, it is inherently incompatible to C code that uses `_FILE_OFFSET_BITS=64`. And lastly, if a Meson project is Rust-only, but generates its internal code from its public C headers, then it is better suited to actually call `add_language('c')` and ensure that Meson knows what the compiler configuration for the target platform actually is. Otherwise, bindgen cannot know what platform options to enable. Hence, warn loudly if `rust.bindgen()` is used without a configured C compiler (even if the compiler used by bindgen does not necessarily match the configured one). [1] rust-lang/libc#3223 (comment)
Meson sets 64-bit offsets as the default for all platforms but MSVC. Lets do the same for bindgen, to ensure we get compatible definitions.
Note that the
libc
crate does not expose 64-bit types as the default and there is no intention to do so. Instead, it exposes 32-bit default types, plus the 64-bit extended types with the*64
suffix. This is quite unfortunate, but it seems unlikely to change [1].However, use of
bindgen
is usually not tied to thelibc
crate. Instead, it is tied to whatever other C code in the same project does. And Meson sets_FILE_OFFSET_BITS=64
unconditionally for all this C code. It thus seems much more plausible for Meson to also imply it for bindgen.Given that Rust code that uses the
libc
crate very likely already uses the*64
suffixed variants, they are unaffected by whether_FILE_OFFSET_BITS=64
is set. If they uselibc::off_t
, they already explicitly state that they use the 32-bit variant on 32-bit platforms. Hence, it is inherently incompatible to C code that uses_FILE_OFFSET_BITS=64
.[1] rust-lang/libc#3223 (comment)