Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Activity
    Alvaro Moran
    @dirac3000
    any hint?
    Daan De Meyer
    @DaanDeMeyer
    So drain will try and read all the output of the process so that's supposed to get stuck. If you set the nonblocking option though, read shouldn't be blocking. If you make an issue on Github with a minimal amount of code to reproduce the problem, I can look into it when I have some free time.
    Alvaro Moran
    @dirac3000
    sure thank a lot! See DaanDeMeyer/reproc#41
    Wolf Vollprecht
    @wolfv
    Hi, thanks for this useful project! I am planning to use it in micromamba, a package manager for all languages & operating systems (based on the conda package manager)
    For this, I am adding a recipe for reproc to the conda-forge distribution: conda-forge/staged-recipes#13113
    once this would be merged we'd have automated builds for Windows, Linux and OS X (as well as Linux-ARM64 and PPC64LE :)
    I've got two questions: What is the ABI compatibility range with reproc approximately? Does a program linked to 14.0 work with 14.X?
    and do you want to be a co-maintainer @DaanDeMeyer ? I am happy to add you. It's not a lot of work, just merging a PR by the bots here and there, whenever a new version comes out
    Wolf Vollprecht
    @wolfv
    also, clock_gettime is only available from osx 10.12 upwards. I am going to patch this, using a polyfill for versions before that (we support 10.9 on conda-forge)
    are you interested in a PR for that?
    Daan De Meyer
    @DaanDeMeyer
    I haven't been considering ABI compatibility for now. Whether it's ABI compatible will depend on what exactly changed in a release.
    I don't really have any spare cycles to be a co-maintainer. You can add me but I can't promise I'll get to any PRs in a timely fashion.
    Daan De Meyer
    @DaanDeMeyer
    Problem with the backport is that I have no way to test it. I prefer only having code that I can test one way or another either with CI or locally. Keep it in conda-forge for now.
    The rt fix in cmake that you do is interesting though. Can you expand on that one? That may be something to include in reproc. I'm curious why it's only necessary for the tests
    Wolf Vollprecht
    @wolfv

    thanks for the replies! I am going to add you just so you get notifications. There won't be any work until you make a release, and even then hopefully everything will work smoothly :)

    The rt fix is something I had to do a couple of times already for conda-recipes. I am not sure exactly why it's needed when compiling with the conda-compilers, vs. the regular OS compilers. Maybe the add -lrt implicitly?

    I just added it to the tests, which means that people who want to use the timeout functionality will have to link themselves with rt.
    Do you think linking the .so would be better, instead of the testts?
    Wolf Vollprecht
    @wolfv
    this is the feedstock now: https://github.com/conda-forge/reproc-feedstock from where new builds will be pushed to the conda-forge repository
    Daan De Meyer
    @DaanDeMeyer
    I'm curious what the exact failure is when rt is missing, I'm assuming I used some symbol from rt but I can't immediately figure out which one that would be
    Wolf Vollprecht
    @wolfv
    clock_gettime :)
    Daan De Meyer
    @DaanDeMeyer
    Ahhh, alright, I'll just add that to reproc.
    Daan De Meyer
    @DaanDeMeyer
    I pushed a commit that links against librt on unix (but not osx) platforms. Can you give it a test?
    Brad Larsen
    @bradlarsen

    Hey @DaanDeMeyer -- useful project! I have been using reproc in an application, and have noticed a couple issues:

    (1) The ASSERT_UNUSED macro is used in several places for error handling instead of checking function contracts or invariants. The end result is that in debug builds, sometimes reproc crashes the application with an assertion failure for something that is actually just an uncommon but valid return value.

    (2) The EINTR error code, used for interrupted system calls, isn't handled explicitly by reproc at all. Using reproc++, I have seen things like process creation failing, and drain failing, due to interrupted system calls. In these places I have been able to put a do { ec = ... } while (ec.value() == EINTR); around the call in my application, but I was wondering if this is something you would want to handle in the reproc / reproc++ libraries themselves.

    I can make a couple PRs if you'd like, but I figured I would check with you first!
    Daan De Meyer
    @DaanDeMeyer
    (1) is 100% bugs on reproc's side. The asserts should always be true, if they aren't, I've made an incorrect assumption somwhere and that should be fixed
    I'd definitely appreciate a PR for those
    (2) is more complex, sometimes users might want to handle EINTR themselves instead of relying on reproc to do it. Usually, you'd get around this by specifying SA_RESTART in a call to sigaction when you want to restart the system call on EINTR for a specific signal. Would that work for your use case? If not, there's no harm in adding an option to reproc that tells it to retry system calls with the usual do while approach.
    Daan De Meyer
    @DaanDeMeyer
    It would introduce a bit of churn since every function doing a system call would need to take a parameter telling it to retry system calls but that might be unavoidable.
    Daan De Meyer
    @DaanDeMeyer
    Of course, this setting is global per signal but that might work for your use case
    Daan De Meyer
    @DaanDeMeyer
    I looked again and now I remember fully, I use ASSERT_UNUSED in scenarios where I can't report the error as a return value to the user. This is usually in cleanup functions or after forking when spawning a process. My idea was that these were only for DEBUG builds anyway and making these asserts might help me figure out if these commands could fail or not. I would not remove the asserts but rather modify them to ignore the errors you're seeing and add a comment to them explaining why these errors could happen.
    Brad Larsen
    @bradlarsen
    @DaanDeMeyer I am back from holiday vacation now and will put together a PR for the assertion failures in the next couple weeks.
    Most of the places where I have seen ASSERT_UNUSED fail was because system calls could fail with EINTR
    Daan De Meyer
    @DaanDeMeyer
    Thinking more about this, I think we can retry EINTRs for system calls that we don't report errors for (those that are followed by ASSERT_UNUSED).
    None of those are expected to block very long anyway
    Daan De Meyer
    @DaanDeMeyer
    Actually, don't retry close() when EINTR occurs, see https://bugs.chromium.org/p/chromium/issues/detail?id=269623
    So just ignore EINTR there. The rest should be safe to retry
    Brad Larsen
    @bradlarsen
    Great; thanks for the pointers. I'm gonna try to get to this later this week.
    AokiYuune
    @AokiYuune
    Good Night! I'm a beginner of reproc. Just
    for learning.