This fixes issue #1318 - on Solarish systems __posix_readdir_r is only

defined for 32-bit Solarish targets.  Since rust doesn't currently support 32-bit Solarish targets, the line is both unnecessary and wrong.
As found with the latest fd binary, removing the line fixed the fd build (and fd then works as expected).

r? @gnzlbg
(rust_highfive has picked a reviewer for you, use r? to override)

It would be nice if someone with solaris experience could take a look, maybe @bgermann  ?

It can be confirmed by running nm on /lib/libc.so vs. /lib/amd64/libc.so on a Solaris system (the former should have __posix_readdir_r while the latter (the 64-bit lib) will not.  Due to all the symbol gymnastics in dirent.h, it's very easy to miss the difference if you just try looking at the the header file.

I would be surprised if Solaris has changed behavior.  This appears to have been the case since at least 2006 (looking at what symbols are exported in libc in the commit history which goes back to the OpenSolaris launch) which predates the Solaris/illumos split, and was strictly for backwards compatibility.
It looks like what happened was prior to 64-bit systems arriving on the scene, the interface was changed by POSIX, so the old interface (taking only two arguments) was left for backwards compatibility, while the final POSIX standard interface was added (using some header tricks for new programs to use the new version -- __posix_readdir_r) as well as the large file variant for 32-bit apps.  Since 64-bit apps post-date the interface change, only the final POSIX version was exposed for 64-bit applications.

The change is accurate. Please also see
https://docs.oracle.com/cd/E36784_01/html/E36873/libc-3lib.html and
https://www.unix.com/man-page/opensolaris/3LIB/libc/
which list __posix_readdir_r at a section titled "The following interfaces are unique to the 32-bit version of this library"

Thank you for taking a look. Would it be possible to add a documentation
comment explaining that and containing the relevant links? And maybe extend
the commit message with this information as well?
There is a lot of knowledge that remains spread in the PRs that is hard to
find when looking at a blame or when somebody tries to make a change. Right
now me and Alex are the single points of failure when it comes to remember
where to find what or who to ping, and I am trying to slowly change that.
Particularly for linkage, which is specially tricky due to how libc in
crates.io interacts with the built-in one.

Sure -- would something like
/// 64-bit Solaris and illumos libc only has readdir_r.  A 32-bit Solaris or illumos target (if ever created)
/// should use __posix_readdir_r.  See libc(3LIB) on Solaris or illumos (https://illumos.org/man/3lib/libc).

work?  I figure it'll be easier to hash out the wording here than do a lot of churn on the PR.  I used the illumos man link since that's likely to be a more stable URL than docs.oracle.com (I think they've reorganized things at least once or twice on that site, making older links no longer work).

I'd rather err in the side of too many links, and add the links that @bgermann mentioned to the comment, but otherwise LGTM. I'll merge when the PR is updated. Thank you both!

@bors: r+

:pushpin: Commit e45fa4a217c5501e427404980c5f0db712730f58 has been approved by gnzlbg

:hourglass: Testing commit e45fa4a217c5501e427404980c5f0db712730f58 with merge dfda3b0d01939e42e5f05999cb8692624b1d8077...

:sunny: Test successful - checks-cirrus, checks-travis, status-appveyor
Approved by: gnzlbg
Pushing dfda3b0d01939e42e5f05999cb8692624b1d8077 to master...


Ansible

Ansible setup and integration with Jenkins.

This is not feasible and also not needed with our solution


Bump @storybook/csf

It is currently semi-pinned via
"@storybook/csf": "^0.0.1"

which, by npm's semver implementation, will always remain less than 0.1.0 (which is the latest version https://www.npmjs.com/package/@storybook/csf)

Fixed by:
https://github.com/storybookjs/eslint-plugin-storybook/pull/171


[BUG] cdef except -1 silently ignored by cpdef caller and not raised!

Describe the bug
cpdef function calling cdef function that raises in case of a return value of -1 would silently ignore the exception and simply skip executing the chunk of code starting from the cdef function.
This IMO is a very bad behavior/feature/bug, b/c it makes debugging very hard and confusing.
To Reproduce
Code to reproduce the behaviour:
cdef class Dummy:

    cdef int test(self) nogil except -1:
        return -1   # should raise

cpdef void test() except *:
    cdef Dummy d = Dummy()

    print("before d.test")
    d.test()
    print("called d.test")

Expected behavior
I would expect d.test() to raise and that exception gets propagated to the caller test as well.
Instead, if you run the above, anything after line d.test() (inclusive) is simply ignored w/o any error/warning msgs!
Environment (please complete the following information):

OS: Linux (Centos)
Python version 3.8
Cython version 3.0a10

Additional context
Add any other context about the problem here.

No. You're misunderstanding how it works.
cdef int test(self) nogil except -1:
    return -1   # should raise

What this says is: if Cython raises an error, then it'll use a value of -1 to indicate it to the caller.
It does not say that a value of -1 is converted into an exception. All return -1 will do is confuse the caller (because it expects an exception to be set).
What you want is something like:
cdef int test(self) nogil except -1:
    raise ValueError

No. You're misunderstanding how it works.
cdef int test(self) nogil except -1:
    return -1   # should raise

What this says is: if Cython raises an error, then it'll use a value of -1 to indicate it to the caller. It does not say that a value of -1 is converted into an exception. All return -1 will do is confuse the caller (because it expects an exception to be set).
What you want is something like:
cdef int test(self) nogil except -1:
    raise ValueError

So except -1 is just a way for caller to realize that it needs to propagate the exception raised inside the cdef function.

But then why print("called d.test") part of the example above was skipped since there's no exception raised.

So except -1 is just a way for caller to realize that it needs to propagate the exception raised inside the cdef function.

Yes.

This really isn't well-defined. It expects there to be an exception if you return -1. It may or may not check. You shouldn't rely on it.
If -1 is a legitimate value to return then you should use except ?-1 instead (which says "a value of -1 might be an exception")

except ?-1

I see... so let me summarize to make sure:
First of all, picking except -1 over except * (c only. no c++) is largely due the function return type (i.e. int) here, so faster (i.e. only check one value), and it's only a signal to the caller for checking and propagating exception(s).
If, for example, we wanna to raise if an index is -1, re-using the example above, we should just raise inside the cdef function to be safe, instead of return -1 and counting on the caller to treat it as an exception (which it won't).
Finally, the reason why I saw the weird behavior (to me) above that the function runs but everything after d.test() is ignored, is b/c I was basically doing something undefined, i.e. telling the caller that there's an exception, but there's not, which is undefined.

Yes, it looks like you understand it correctly now.

Yes, it looks like you understand it correctly now.

Thank you very much! Really appreciate the great help!


Add basic BAI2 parser

@SteveMarshall @ibrechin
This is the start of a BAI2 parser library, it's not perfect and it's not stable but it's a start.
I could probably spend a week refactoring it and making it very nice but I'm not sure we have time for that so I'm just going to trust you guys and make only the changes that you suggest.

It would be good if BAI2_IGNORE_INTEGRITY_CHECKS set to 1 still logged the results of the integrity checks, instead of raising.

This looks good to me. I have no important changes to suggest.

@SteveMarshall you can open an issue and ideally submit a PR if you want :-)
It's probably not part of this PR and not high priority atm.