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

Patch the build system to better support conda-build #7603

Merged
merged 19 commits into from
Aug 8, 2024

Conversation

leofang
Copy link
Member

@leofang leofang commented Jun 2, 2023

Patch taken from conda-forge/cupy-feedstock#199.

Context:

  1. The conda compiler is a cross compiler by design (and it turns out nvcc is, too). It means that a conda package can be cross-compiled instead of native-compiled. In other words, the build platform is not necessary the same as the target platform. But this creates an issue in particular for CuPy, because CuPy has never promised to support build != target. In short, CuPy has an Autotool-like dependency checking system (backed by build_shlib() and build_and_run()) that essentially checks the dependencies on the build platform (say linux64), but the actual binaries on the target platform (say aarch64) cannot be linked by the cross compiler (on linux64). A linker failure is raised when attempting to do so.
  2. The CUDA 12 bringup on conda-forge further reinforces the need to support this use case. As mentioned, we learned that nvcc is also a cross compiler (and people urge for this capability crazily in order to reduce the package build time), and when transitioning to the new package layout we see that it's best to assume a cross compiler is in use. So the patch that was originally written to only support cross compiling (+ CUDA 11), can also be tweaked to support CUDA 12 on conda-forge.

Approach:

  • This patch is turned on only during conda-build. This is possible by detecting a few env vars set by conda-build; that is, when building CuPy on CF. It's a no-op for 99% of the time.
  • This patch is written to support both the old layout (CUDA 11) and new layout (CUDA 12) on conda-forge.
  • This patch works with both native and cross compiling.
  • This patch has not been validated on Windows yet, because

Also close #8124.

@emcastillo emcastillo added cat:install Build and installation to-be-backported Pull-requests to be backported to stable branch prio:high labels Jun 2, 2023
@emcastillo emcastillo added this to the v13.0.0b1 milestone Jun 2, 2023
@leofang
Copy link
Member Author

leofang commented Jul 11, 2023

/test mini

@asi1024 asi1024 removed this from the v13.0.0b1 milestone Jul 26, 2023
@leofang
Copy link
Member Author

leofang commented Jul 30, 2023

/test mini

@leofang leofang requested a review from kmaehashi July 30, 2023 23:47
@leofang
Copy link
Member Author

leofang commented Jul 31, 2023

FYI @kmaehashi this is ready 🙂

@leofang
Copy link
Member Author

leofang commented Nov 20, 2023

Update: I updated the PR with Windows support (from conda-forge/cupy-feedstock#228). If we plan to backport this PR, I can do it manually (the CUB path lookup needs to be patched).

@leofang
Copy link
Member Author

leofang commented Nov 20, 2023

/test mini

@asi1024 asi1024 removed this from the v13.0.0rc1 milestone Nov 29, 2023
@leofang leofang marked this pull request as draft January 22, 2024 00:58
@leofang leofang marked this pull request as ready for review January 23, 2024 02:13
@leofang
Copy link
Member Author

leofang commented Jan 23, 2024

Update: The patch here is sync'd with conda-forge/cupy-feedstock#241.

@leofang
Copy link
Member Author

leofang commented Jan 29, 2024

/test mini

@leofang
Copy link
Member Author

leofang commented Jan 29, 2024

@kmaehashi Shall we try to get this merged (and backported) before the next release, so that we can avoid maintaining the patch in cupy-feedstock?

@leofang leofang added this to the v14.0.0a1 milestone Feb 5, 2024
@leofang
Copy link
Member Author

leofang commented Jun 10, 2024

Hi @kmaehashi any chance we can get this PR merged and backported to v13.2.0? It's getting more and more challenging for me to maintain a big patch for conda-forge... 😅

@leofang
Copy link
Member Author

leofang commented Jun 10, 2024

/test mini

@takagi takagi removed this from the v14.0.0a1 milestone Jun 12, 2024
@kmaehashi kmaehashi added this to the v14.0.0a1 milestone Jun 13, 2024
@leofang leofang mentioned this pull request Aug 5, 2024
@jakirkham
Copy link
Member

/test mini

(to include now required CUDA 12.5 checks)

@kmaehashi kmaehashi merged commit 50ec72b into cupy:main Aug 8, 2024
48 of 57 checks passed
chainer-ci pushed a commit to chainer-ci/cupy that referenced this pull request Aug 8, 2024
Patch the build system to better support conda-build
@leofang leofang deleted the conda_patch branch August 8, 2024 09:16
jakirkham added a commit to conda-forge-admin/cupy-feedstock that referenced this pull request Aug 22, 2024
jakirkham added a commit to conda-forge-admin/cupy-feedstock that referenced this pull request Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat:install Build and installation prio:high to-be-backported Pull-requests to be backported to stable branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hardcoded lib path in build script causes error
6 participants