-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Support FreeBSD 12 (64-bit inodes) #5199
Conversation
Very exciting! Would it be possible to enable 11.1 or is there something specific to current that's required? |
11.1 worked as is. On 12 it was broken because of the inode/dirent/stat changes in the kernel. This fix uses the new structs on 12. |
Ah, great. Sorry for the noise then. |
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.
This is fine for now, but we should probably think a bit more about how to handle the BSDs' libc in the future.
FreeBSD 12.0 is C bindings should be considered as automatically generated. Starting to put flags to support one or another version is unexpected. Also, using Maybe we can introduce flags without the leading |
Yeah, that's why I said
I guess I should work on that… Point releases don't break ABI, you're not wrong. |
@ysbaddaden you can't consider the C bindings as automatically generated unless they are automatically generated. I never got the generator to work and contributors are unlikely to do the same, unless the whole generation process comes down to a simple, fairly portable command. I doubt many of the recent lib_c changes were not manually done. |
I never said we can't maintain C bindings manually, but to keep in mind they should be auto-generated and will eventually be (since we can't rely on fixed sets, as this issue outlines) to help avoid changes that would prevent this. I don't understand how nobody got the generator working. Yes, cross compiling headers for a foreign platform is tricky (because local compiler & system headers get in the way), but generating bindings for the current platform is a mere |
This issue also means that autogeneration would only generate code for one OS version, unless you make it read headers from multiple versions. |
@ysbaddaden the benefits of automation here are consistency and reproducibility. As people have demonstrated with their PRs, doing it manually is easier than using |
@@ -9,9 +9,17 @@ lib LibC | |||
alias DevT = UInt | |||
alias GidT = UInt | |||
alias IdT = Long | |||
alias InoT = UInt | |||
{% if flag?(:"freebsd12.0") %} |
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.
No need to use symbol, you can just write:
{% if flag?("freebsd12.0") %}
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.
Hm, now this was merged with symbol...?
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.
It was just a note, not super necessary to make it work. There's also no difference regarding performance or anything, just style.
(also we're
unknown
now instead ofportbld
, so I added a symlink there)THANK YOU for simply splitting the llvm target on
-
and shoving everything into flags. Rust currently drops the version number from targets and it's painful.// Ideally, this would use a version comparison
>= 12.0
, but this is fine for now.