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

overrides: update build system for pyzmq 26.0.0 to scikit-build #1603

Merged
merged 2 commits into from
May 2, 2024

Conversation

Samuel-Martineau
Copy link
Contributor

Since about two months ago, pyzmq requires scikit-build to be built. This is a temporary workaround up until #568 is sorted out.

Contribution checklist (recommended but not always applicable/required):

  • There's an automated test for this change
  • Commit messages or code include references to related issues or PRs (including third parties)
  • Commit messages are conventional - examples from the log include "feat: add changelog files to fixup hook", "fix(contourpy): allow wheel usage", and "test: add sqlalchemy2 test"

@Samuel-Martineau Samuel-Martineau force-pushed the patch-1 branch 2 times, most recently from 9cbbb5f to be80699 Compare April 18, 2024 14:21
@Samuel-Martineau Samuel-Martineau changed the title fix: build system for pyzmq overrides: update build system for pyzmq 26.0.0 to scikit-build Apr 18, 2024
@multun
Copy link

multun commented Apr 30, 2024

@Samuel-Martineau I'm having the same issue, and had to make the following fixes:

  • added scikit-build-core to pyzmq dependencies, which is not the same as scikit-build
  • patched scikit-build-core to help it find meson and ninja
scikit-build-core = super.scikit-build-core.overridePythonAttrs (
  old: {
    preConfigure = ''
      sed -i '/def _get_cmake_path/i def _get_cmake_path(**_): yield Path("${pkgs.cmake}/bin/cmake")
           s/def _get_cmake_path/def _get_cmake_path_old/' src/scikit_build_core/program_search.py
      sed -i '/def _get_ninja_path/i def _get_ninja_path(**_): yield Path("${pkgs.ninja}/bin/ninja")
           s/def _get_ninja_path/def _get_ninja_path_old/' src/scikit_build_core/program_search.py
    '';
    propagatedBuildInputs = (old.propagatedBuildInputs or []) ++ [super.pyproject-metadata super.pathspec];
  }
);

Is your PR already in a working state? if so, what nixpkgs channel / pyzmq version did you test it with?

@Samuel-Martineau
Copy link
Contributor Author

@multun If I am frankly honest, I don't recall how I tested this, all that I know is that I managed to get it running somehow. It is therefore quite likely that there are changes that I did that I forgot to include. I will try to test it again from scratch, though I suspect that your work is right and that those changes are indeed needed. If that's the case, would you mind me including your code in this PR?

@multun
Copy link

multun commented Apr 30, 2024

Oh yeah help yourself. Let me know if you need review / testing, so far I haven't tested anything.

@cpcloud
Copy link
Collaborator

cpcloud commented May 2, 2024

Ok, I managed to get this working without too many hacks and was able to add a passing test.

@cpcloud
Copy link
Collaborator

cpcloud commented May 2, 2024

Passing locally, merging!

@cpcloud cpcloud merged commit 64a3034 into nix-community:master May 2, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants