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

python-jsonrpc-server: remove ujson version contraint #91707

Merged
merged 1 commit into from Jul 26, 2020

Conversation

DavHau
Copy link
Member

@DavHau DavHau commented Jun 28, 2020

Motivation for this change

Since 141a029 the version constraint of the ujson dependency is changed via sed from "ujson<=1.35" to "ujson>=1.35" to allow for for a newer ujson version.
This is bad for adaptability. If one decides to actually include a correct version of ujson (by using mach-nix for example), then the installation fails despite all dependencies are correct.

I think a better way to bypass upstream version containts is to just remove them.
@Mic92

Things done

Modify the sed command to replace 'ujson<=XXX' with 'ujson'

ujson builds were failing when using nixpkgs-review, but I guess this has other reasons !?

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@FRidh
Copy link
Member

FRidh commented Jun 28, 2020

mach-nix should probably just pass --no-dependencies to pip, assuming you fully trust the earlier determined dependencies.

@DavHau
Copy link
Member Author

DavHau commented Jun 28, 2020

mach-nix should probably just pass --no-dependencies to pip, assuming you fully trust the earlier determined dependencies.

I didn't know about that possibility, thanks. But i don't feel that this is something I'd like to do, since I don't like to trust the dependency resolver blindly. Having this check can reveal problems during build which otherwise would lead to a broken environment without further notice.
I guess --no-dependencies isn't used in nixpkgs for the same reason.

@jonringer
Copy link
Contributor

jonringer commented Jun 29, 2020

I guess --no-dependencies isn't used in nixpkgs for the same reason.

It does pass --no-deps during wheel build, but not when installing

@DavHau
Copy link
Member Author

DavHau commented Jun 29, 2020

I could also fix problems like this on the mach-nix end, for sure. But I thought composability is important for nixpkgs.
The proposed change improves the composability without introducing any disadvantages, correct?
I'd rather improve nixpkgs instead of creating my own custom overrides.

@FRidh FRidh merged commit 62c481a into NixOS:master Jul 26, 2020
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.

None yet

3 participants