Add missing dependencies for: _khash_primitive_helper

[vaikas]

• [ x ] closes BUILD: Building with samurai fails while ninja works. #55615 - Add missing dependencies and one can now use samurai to build.

Thanks to @kaniini for helping me find where to add the dependencies.

[WillAyd]

Might be overlooking it but why do these modules need this as a source? Can you provide the original error you were getting?

[vaikas]

Thank you! Yeah, the original errors are in the issue under Installation Logs under Details, maybe I put them in the wrong section 🤣 . I did also dig deeper that has more details in this issue:
wolfi-dev/os#7334

[kaniini]

When building with samurai, the dependency rules are sorted after loading, while the original implementation of ninja orders the dependency rules based on when they were malloc'd. This can result in underdefined dependencies appearing to work simply as a side effect of when the dependency rule was evaluated.

In this case, both arrays.pyx and index.pyx are pulling in pandas._libs via an import (not cimport) statement, which results in khash.pyx being evaluated as part of the cython generation process. But khash.pyx requires generated code, so it needs to be generated before these modules so they can safely pull in pandas._libs.

As an aside, see #51875 which is pointing out basically the same problem.

[WillAyd]

Do these need to be listed in sources or can you add them as deps? I see a few other modules already use deps and that seems like a more natural fit

[kaniini]

Yes, probably. Testing it now!

[kaniini]

Sadly, Meson reports when using deps:

../pandas/_libs/meson.build:106:7: ERROR: Argument is of an unacceptable type 'CustomTarget'.

So I think we need to keep it as a source due to limitations of meson.

[WillAyd]

Did you declare that dependency against _khash_primitive_helper or _khash_primitive_helper_dep? Looks like the latter is used in a few places already so would be surprised if that didn't work

[vaikas]

Ok, phew that was easy 😆 PTAL, looking better?

[WillAyd]

Lgtm. Thanks for fixing up algos too.

@mroeschke @lithomas1 any objections?

[mroeschke]

No objections on my side

[jbrockmendel]

seems weird that this would be needed here

[lithomas1]

Agreed with Brock, doesn't arrays not cimport anything from pandas?

[vaikas]

Right you are! 👍 I reckon it hit that in 'earlier' tries, and the proper fix was in index? 🤷

[vaikas]

Well, it does seem to build, but breaks the unit tests?

[lithomas1]

No, that's unrelated.

#55804 should fix it.

[lithomas1]

LGTM.

[vaikas]

I don't think the failing test is related to this PR looking at other PRs that are failing similarly?

[mroeschke]

Thanks @vaikas