conda-forge / cupy-feedstock

A conda-smithy repository for cupy.
BSD 3-Clause "New" or "Revised" License
5 stars 23 forks source link

@conda-forge-admin, please rerender #279

Closed leofang closed 1 month ago

conda-forge-webservices[bot] commented 1 month ago

Hi! This is the friendly automated conda-forge-webservice.

I just wanted to let you know that I started rerendering the recipe in conda-forge/cupy-feedstock#280.

jakirkham commented 1 month ago

FWIW there is also @conda-forge-admin, please update version, which will start a version update PR

leofang commented 1 month ago

@jakirkham the point was not trying to update version, but to show that something in the CF infra has changed recently that a package (v13.2.0) that originally built fine can no longer be built now. Please see the rerender PR. So it's not related to any v13.3.0 change, but to CF changes.

cc @kmaehashi for vis

jakirkham commented 1 month ago

Ah ok. Encountered this recently and submitted a fix in PR: https://github.com/cupy/cupy/pull/8544

It sounds like we may need to carry that as a patch for a little while. Though it is a much smaller patch than the one we already carry and we will be able to drop our current patch

leofang commented 1 month ago

I saw the patch. What I don't understand is why we need it. Are you aware of any recent CF change that could lead to the c++ not found error? AFAIK the main branch (of this feedstock) still built fine by the end of July.

jakirkham commented 1 month ago

I think we just got lucky in that some of the Distutils components weren't being used before (like the C++ linker). Regardless we should be patching all of them to match our intended usage

jakirkham commented 1 month ago

Let's continue this in the PR where I have tried to include more detail about what I think happened: https://github.com/cupy/cupy/pull/8544#discussion_r1726342921

TL;DR It is just a Distutils change as noted and we got unlucky timing. We will just need to be more aware of the shifting Python packaging landscape

leofang commented 1 month ago

The relevant part that's being patched has already existed over a year (since last June / #199) and served across multiple CuPy releases, so forgive me to find it hard to believe the work was done based on luck 😕

I'd feel better if we figure out what's changed in conda-forge in the last two days, since earlier this week it still worked. It could turn out to be another issue that got fixed yesterday somewhere else exposing the deficiency of my patch, in which case I would happily take additional patches as needed. But right now it looks to me something just broke this feedstock, but I can't find out what (and we think it's ok to let go?!) For example, I just checked that the ctng(-activation) feedstocks did not change in the past week.

leofang commented 1 month ago

Ah it seems we were typing at the same time.

jakirkham commented 1 month ago

Yeah I know the feeling. Felt the same way when triggering a simple rebuild of v13 last night. Didn't expect to be debugging and fix that late at night

In any event, hoping that additional context helped

leofang commented 1 month ago

Thanks a lot for the efforts John! I should be able to help a bit more starting next week.

leofang commented 1 month ago

To conclude this issue:

I'd feel better if we figure out what's changed in conda-forge in the last two days

Based on @jakirkham detective work, it's because setuptools 72.2.0+ was in use, which added

native support for C++ compilers

according to its changelog. Pinning at older versions as demo'd in commit 4ceca14 works.