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

python3.pkgs.thinc: fix build #222275

Closed
wants to merge 1 commit into from
Closed

python3.pkgs.thinc: fix build #222275

wants to merge 1 commit into from

Conversation

benxiao
Copy link
Contributor

@benxiao benxiao commented Mar 21, 2023

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Copy link
Member

@bcdarwin bcdarwin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; builds fine on x86_64-NixOS.

@@ -71,6 +71,10 @@ buildPythonPackage rec {
pytestCheckHook
];

postPatch = ''
substituteInPlace setup.cfg --replace "blis>=0.7.8,<0.8.0" "blis"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note you can often use pythonRelaxDepsHook to relax or remove a dependency.

Copy link
Contributor Author

@benxiao benxiao Mar 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bcdarwin I tried that. but it doesn't seem to affect setup.cfg. I was looking at the pythoinRelexDepsHook shell script, but its still a mystery what files they will act on. by the way. really appreciate that you review my MRs. thanks.

Copy link
Contributor

@danieldk danieldk Mar 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't do this. BLIS 0.9.x is broken and causes segfaults. The upper bound of 0.8.0 is there for a reason.

explosion/thinc#771 (comment)
#207070

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for spotting this. I will close it now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danieldk, shall we revert blis to version 0.7.9, so it is breaking spacy?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I did that before, but a mass-update undid it again. There is a suggestion here:

#218614 (comment)

to prevent that from happening in the future

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benxiao benxiao closed this Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants