Nfnetfilter log #1571
Nfnetfilter log #1571
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @gnzlbg (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
I'm getting some CI errors and I'd ask for little advice how to proceed. All the musl based ones fail with something like this: It seems these constants are not available in the musl dockers. However, I don't think the existence of these constants depends on the musl vs glibc platform. These come from kernel headers and I think the headers in these specific containers are simply older than the constants. Furthermore, as the code that handles them is inside kernel, not musl or other userspace library, even the existence or non-existence of the constant in some C header doesn't say much about the constant being actually useful ‒ one could compile the program on a system without the constant defined, but if there was a new enough kernel running or one took the program to other (newer) system, it would work fine. Therefore, I don't think the right way is to somehow conditionally define or not define the constants on the rust side. On the other hand, I myself don't need these specific constants, I've included them more for completeness sake. So, what's the right way forward? I'm also getting some errors on the emacscripten platforms, which seems a bit odd ‒ I didn't touch these and even as I read the tests, the headers I've added there should not get used there. Am I missing something? |
Kind of. You can't use the kernel headers with any C library, because they might have conflict (e.g. if the kernel The "musl sanitized" kernel headers that libc is verified against in the Linux musl build jobs might be a bit older than the normal ones, and might not contain these constants. If you look at the Dockerfiles in the ci/ directory, you can check that out, and maybe update them to a newer version. |
|
I've checked. The docker uses 4.4.2-2, the released version is 4.4.2-3. The constants are not available there :-(. It's not even in their master. So, what are the options?
Anything else? |
|
I've deleted the constants at fault to move forward somehow. However, I'm still getting CI errors on platforms that are unrelated to the changes and that I haven't touched. @gnzlbg Do you have an idea what might be happening in there? Is it something I've missed? |
|
@bors: r+ |
|
📌 Commit e7936ad has been approved by |
|
The javascript platforms still fail :-(, no idea why. Bors probably can do nothing about that. Besides, should I first squash that fixup, before merging? |
Nfnetfilter log This adds the constants from linux/netfilter/nfnetlink.h and nfnetlink_log.h. These are the files I need for jbaublitz/neli#48. After this gets in, I'd like to follow-up with the other nfnetlink_*.h files too, as I'd like to extend neli with further protocols in the future, but I want to do a smaller PR first to see if there are some things to tweak. I've noticed similar netfilter constants are also in the android subfolder, therefore I'm adding them there too (I don't like the copy-pasting, but it seems the other ones are already copy-pasted). I assume the test will catch it if anything is different on that platform.
|
💔 Test failed - status-azure |
|
@bors: retry |
Nfnetfilter log This adds the constants from linux/netfilter/nfnetlink.h and nfnetlink_log.h. These are the files I need for jbaublitz/neli#48. After this gets in, I'd like to follow-up with the other nfnetlink_*.h files too, as I'd like to extend neli with further protocols in the future, but I want to do a smaller PR first to see if there are some things to tweak. I've noticed similar netfilter constants are also in the android subfolder, therefore I'm adding them there too (I don't like the copy-pasting, but it seems the other ones are already copy-pasted). I assume the test will catch it if anything is different on that platform.
|
☀️ Test successful - checks-cirrus-freebsd-10, checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, status-azure |
This adds the constants from linux/netfilter/nfnetlink.h and nfnetlink_log.h. These are the files I need for jbaublitz/neli#48. After this gets in, I'd like to follow-up with the other nfnetlink_*.h files too, as I'd like to extend neli with further protocols in the future, but I want to do a smaller PR first to see if there are some things to tweak.
I've noticed similar netfilter constants are also in the android subfolder, therefore I'm adding them there too (I don't like the copy-pasting, but it seems the other ones are already copy-pasted). I assume the test will catch it if anything is different on that platform.