rust-lang
FreeBSD: add evdev structures
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @JohnTitor (or someone else) some time within the next two weeks.
Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:
@rustbot author: the review is finished, PR author should check the comments and take action accordingly@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue
Some context: On FreeBSD the evdev headers are not part of the base system, but come from a package named evdev-proto. It seems this package should be installed to make this PR compile.
@devnexen You did some FreeBSD PRs already, could you please look at this one too?
Some context: On FreeBSD the evdev headers are not part of the base system, but come from a package named
evdev-proto. It seems this package should be installed to make this PR compile.
Just quick response here: by definition if it is not part of the system thus has no place in this crate, e.g. libutil is still part of builtin libraries. my 2 cents :)
Yes, but on Linux these data types are part of libc. Because of this, the consumers of this library that are using these types will fail to compile on FreeBSD. This is the case for the new release of https://github.com/mqudsi/syngesture
I think that putting these types here is the most correct solution, but I know nothing of Rust ecosystem.
I get it, but Linux is Linux. However though, nothing is stopping you define those types in the requested project (or to publish a crate dedicated to those sort of things ?). But hey, I m not the maintainer of libc :) I do not get to decide.
nothing is stopping you define those types in the requested project
This would require patching for each project that depends on these types in libc.
or to publish a crate dedicated to those sort of things ?
This would probably require patching too, although much less? Maybe it is a way to go then?
Regardless, to make CI work here you would need to add the related dependency. CI at the moment only install what necessaty, it appears, for rustup nothing else. The decision to merge it or not is more @JohnTitor call :)
I think you need to declare the appropriate evdev-proto header in libc-test/build.rs in the freebsd's list.
Also note you need to treat struct field such as input_event::type_ as it is for sure not its real name, see other example how it is done in libc-test/build.rs.
@devnexen thank you for the comments! Maybe you can advise me how to create a flag for checking file in /usr/local/include ?
@devnexen thank you for the comments! Maybe you can advise me how to create a flag for checking file in /usr/local/include ?
Are you able to run tests locally ?
@devnexen thank you for the comments! Maybe you can advise me how to create a flag for checking file in /usr/local/include ?
Are you able to run tests locally ?
Yes
I asked about this as a policy question in https://github.com/rust-lang/libc/issues/3839. Until then:
@rustbot blocked
:umbrella: The latest upstream changes (presumably #3889) made this pull request unmergeable. Please resolve the merge conflicts.
On FreeBSD the evdev headers are not part of the base system, but come from a package named
evdev-proto.
<linux/input.h> is also available as <dev/evdev/input.h> (ditto input-event-codes.h and uinput.h). For static platform bindings it may be fine to test against FreeBSD headers which are subset of Linux. This should help unblock progress here by sidestepping #3839.
CC @wulf7 who implemented evdev on FreeBSD and defined evdev-proto policy.
On FreeBSD the evdev headers are not part of the base system, but come from a package named
evdev-proto.
<linux/input.h>is also available as<dev/evdev/input.h>(ditto input-event-codes.h and uinput.h). For static platform bindings it may be fine to test against FreeBSD headers which are subset of Linux. This should help unblock progress here by sidestepping #3839.CC @wulf7 who implemented evdev on FreeBSD and defined evdev-proto policy.
If this code is FreeBSD-only, you can rely on <dev/evdev/input.h> If some chunks of it are shared with Linux, then <linux/input.h> is preferable to not use extra #ifdef-s in shared sections
@tgross35 Can we get this in? The change does not use headers from 3rd-party packages anymore, so #3839 is now unrelated.
@tgross35 Can we get this in? The change does not use headers from 3rd-party packages anymore, so #3839 is now unrelated.
Thanks for the ping, I didn't realize this was ready (for future reference, @rustbot review updates the labels).
@Awoonyaa one nit above and can you add these to libc-test/semver/freebsd.txt?
Cc @asomers
@rustbot author
The definitions look correct to me. Personally I'd be tempted to expose these under an evdev:: module rather than at the crate root. But Linux already exposes them at the crate root, so I guess the horse has already left the barn.
The definitions look correct to me. Personally I'd be tempted to expose these under an
evdev::module rather than at the crate root. But Linux already exposes them at the crate root, so I guess the horse has already left the barn.
Thanks for taking a look. I have been thinking that modules mirroring C header paths might be worth doing for 1.0, then only reexporting a common subset for backward compatibility. Especially considering this would make our sources easier to navigate and cross reference to the system headers. But indeed we may as well be consistent here.
Hi
On Mon, Dec 23, 2024, 3:56 AM Trevor Gross @.***> wrote:
The definitions look correct to me. Personally I'd be tempted to expose these under an evdev:: module rather than at the crate root. But Linux already exposes them at the crate root, so I guess the horse has already left the barn.
Thanks for taking a look. I have been thinking that modules mirroring C header paths might be worth doing for 1.0, then only reexporting a common subset for backward compatibility. Especially considering this would make our sources easier to navigate and cross reference to the system headers. But indeed we may as well be consistent here.
— Reply to this email directly, view it on GitHub https://github.com/rust-lang/libc/pull/3756#issuecomment-2558616235, or unsubscribe https://github.com/notifications/unsubscribe-auth/BKS2BZKO3EPKO5VPIYUP4SD2G4YPXAVCNFSM6AAAAABJWHGAZKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNJYGYYTMMRTGU . You are receiving this because you are subscribed to this thread.Message ID: @.***>
Where are you from?
On Mon, Dec 23, 2024, 3:56 AM Akash Khan @.***> wrote:
Hi
On Mon, Dec 23, 2024, 3:56 AM Trevor Gross @.***> wrote:
The definitions look correct to me. Personally I'd be tempted to expose these under an evdev:: module rather than at the crate root. But Linux already exposes them at the crate root, so I guess the horse has already left the barn.
Thanks for taking a look. I have been thinking that modules mirroring C header paths might be worth doing for 1.0, then only reexporting a common subset for backward compatibility. Especially considering this would make our sources easier to navigate and cross reference to the system headers. But indeed we may as well be consistent here.
— Reply to this email directly, view it on GitHub https://github.com/rust-lang/libc/pull/3756#issuecomment-2558616235, or unsubscribe https://github.com/notifications/unsubscribe-auth/BKS2BZKO3EPKO5VPIYUP4SD2G4YPXAVCNFSM6AAAAABJWHGAZKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNJYGYYTMMRTGU . You are receiving this because you are subscribed to this thread.Message ID: @.***>
Can we get this in, please? Or is it something else we need to fix?
I believe this is ready except for https://github.com/rust-lang/libc/pull/3756#discussion_r1895040761