Skip to content
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

[ENH] Many posix declarations are missing in Cython/Includes/posix #4522

Closed
GalaxySnail opened this issue Dec 24, 2021 · 4 comments
Closed

Comments

@GalaxySnail
Copy link
Contributor

GalaxySnail commented Dec 24, 2021

Is your feature request related to a problem? Please describe.

I find that there is no wrapper for <sys/uio.h> in cython. After browsing the Cython/Includes/posix directory and POSIX.1-2017 basedefs, I realize that many posix declarations are not wrapped in cython. For example, <poll.h> has not been wrapped either.

And most of wrapped posix headers are from POSIX.1-2004, there are some new declarations in POSIX.1-2017 but not in POSIX.1-2004, such as F_DUPFD_CLOEXEC and O_CLOEXEC in <fcntl.h>.

Describe the solution you'd like

It is easy to add a wrapper for <sys/uio.h>.

# Cython/Includes/posix/uio.pxd
# https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_uio.h.html

cdef extern from "<sys/uio.h>" nogil:
    cdef struct iovec:
        void  *iov_base
        size_t iov_len
    ssize_t readv (int fd, const iovec *iov, int iovcnt)
    ssize_t writev(int fd, const iovec *iov, int iovcnt)

But it's not the most important thing.

Describe alternatives you've considered

There are two questions:

  1. Should we wrap all of posix declarations? Adding the missing headers and updating the old wrappers are possile, but are they useful? For example, should we wrap <aio.h> or <iconv.h>?
  2. Should we wrap some platform specific declarations? There are many of them in posix/mman.pxd. But maitaining many platform specific things is difficult and needs a lot of effort.

If the answer of the first question is "yes", we can just do it. If the answer is "no", how to determine which declarations should be wrapped?

If the answer of the second question is "no", how to deal with the existing platform specific declarations? If the answer is "yes", where should they be? Just in the Includes/posix folder? Or something like Includes/linux and Includes/bsd? If we choose the latter, the existing declarations need to be moved (they can be kept in the old places for compatibility). No matter what the answer is, it is still a problem to determine which declarations should be wrapped.

In brief, the promble is "Which delarations are worth being wrapped?".

@da-woods
Copy link
Contributor

I think:

  • the Posix wrappers aren't used too much (we fairly rarely see bug reports/feature requests for them) which means it isn't anyone's top priority to update them.
  • PRs are obviously welcome to wrap anything that's missing and wrapable. That could be a small PR for just the bits that are currently interesting to you or it could be more comprehensive.
  • The Cython wrapping files don't do anything that a user couldn't do (they're purely convenience). Therefore if something is missing then the user can always wrap it themselves. So them being incomplete doesn't really do any harm.
  • Platform specific declarations:
    • it doesn't break anything to tell Cython about a function that doesn't actually exist - it's only a problem when you try to use it. Therefore, adding platform specific functions is generally OK I think (preferably appropriately documented about what platforms they're available on)
    • We'd struggle to test much outside of Linux and OS X.
    • Covering these comprehensively is low priority.
    • But it's fine to wrap something that you've found personally useful?

@GalaxySnail
Copy link
Contributor Author

Thanks for your reply!

  • the Posix wrappers aren't used too much (we fairly rarely see bug reports/feature requests for them) which means it isn't anyone's top priority to update them.
  • The Cython wrapping files don't do anything that a user couldn't do (they're purely convenience). Therefore if something is missing then the user can always wrap it themselves. So them being incomplete doesn't really do any harm.
  • Platform specific declarations:
    • Covering these comprehensively is low priority.

I know. Just for convenience.

  • PRs are obviously welcome to wrap anything that's missing and wrapable. That could be a small PR for just the bits that are currently interesting to you or it could be more comprehensive.

I'm going to make a PR for adding <sys/uio.h> and updating existing posix .pxds. Now I think there's no need to add a new posix wrapper (.pxd file) before someone requests for it.

  • Platform specific declarations:
    • it doesn't break anything to tell Cython about a function that doesn't actually exist - it's only a problem when you try to use it. Therefore, adding platform specific functions is generally OK I think (preferably appropriately documented about what platforms they're available on)

Does that mean a Includes/linux is not necessary? In my opinion, if platform specific declarations increase, a namespace may be a good idea. But compatibility is also important.

  • Platform specific declarations:
    • We'd struggle to test much outside of Linux and OS X.

Linux is what I'm most interested in.

  • Platform specific declarations:
    • But it's fine to wrap something that you've found personally useful?

I'm not sure.

@da-woods
Copy link
Contributor

Does that mean a Includes/linux is not necessary? In my opinion, if platform specific declarations increase, a namespace may be a good idea. But compatibility is also important.

That's my view - I don't think an extra namespace is neccessary.

If they're just extra functions that are defined on Linux then just add them in the main file (with documentation).

If they actually conflict with existing functions (for example, the Linux version has a different set of arguments) then a separate Includes/linux would be a solution.

@GalaxySnail
Copy link
Contributor Author

Does that mean a Includes/linux is not necessary? In my opinion, if platform specific declarations increase, a namespace may be a good idea. But compatibility is also important.

That's my view - I don't think an extra namespace is neccessary.

If they're just extra functions that are defined on Linux then just add them in the main file (with documentation).

If they actually conflict with existing functions (for example, the Linux version has a different set of arguments) then a separate Includes/linux would be a solution.

OK, I see.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants