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

pyspread: Correct dependencies and make wrapper #94537

Closed
wants to merge 1 commit into from

Conversation

ShamrockLee
Copy link
Contributor

@ShamrockLee ShamrockLee commented Aug 2, 2020

Change dependent python version to 3.6-3.9
Change dependent widget toolkit to pyqt5
Add a wrapper so as to execute the application from $out/bin.
Maintainers needed.

Motivation for this change

pyspread 1.99.x depends on Python 3.6-3.9 and pyqt5 instead of Python 2.7, wxPython or pygtk. Some dependent packages have also dropped Python2 support.
A wrapper is made using overrideAttrs and pythonFull.withPackages to make it executable out of the box.

It is the first time for me to package a Python application (rewrite the expressions of a package) in nixpkgs.
There might be some better or more efficient ways to do so.

Things done
  • 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/) (Tested python36Packages.pyspread)
  • Determined the impact on package closure size (by running nix path-info -S before and after) (The closure size of pyspread is now 706115728)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ShamrockLee
Copy link
Contributor Author

@obadz @FRidh

@ShamrockLee ShamrockLee changed the title pyspread: Correct dependencies and make wrapper [WIP] pyspread: Correct dependencies and make wrapper Aug 14, 2020
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/need-help-wrapping-pyspread/8573/1

@ShamrockLee ShamrockLee changed the title [WIP] pyspread: Correct dependencies and make wrapper pyspread: Correct dependencies and make wrapper Aug 15, 2020
@ShamrockLee ShamrockLee marked this pull request as ready for review August 15, 2020 18:11
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

If this is just meant to be just the application, and you don't need to export the package as a python package, then it should probably be moved out of python-modules and packaged as an application. Please see https://github.com/NixOS/nixpkgs/blob/master/doc/languages-frameworks/python.section.md#building-packages-and-applications and https://github.com/NixOS/nixpkgs/blob/master/doc/languages-frameworks/python.section.md#buildpythonapplication-function

then you would just do something like:

mkDerivationWith buildPythonApplication rec {
 ... 
}

pkgs/development/python-modules/pyspread/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/pyspread/default.nix Outdated Show resolved Hide resolved
@ShamrockLee
Copy link
Contributor Author

ShamrockLee commented Aug 16, 2020

@jonringer Thank you for reviewing.
PySpread runs Python expressions users have written inside the cells, and people may want to write those expressions with different versions of Python interpreter.

Users may also want extra packages when writing those expressions. Since pip install (the usual way for PySpread to install packages for users) won't work in NixOS, I set an argument for users to specify their custom package set.

There is also an API to be imported in python scripts, and that's the reason I preserve pyspread-unwrapped.

@ShamrockLee
Copy link
Contributor Author

ShamrockLee commented Aug 16, 2020

I have changed the way it is wrapped and adopted the above suggestions.
@ofborg eval
@ofborg build python36Packages.pyspread python37Packages.pyspread python38Packages.pyspread python39Packages.pyspread pyspread-app

pkgs/development/python-modules/pyspread/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/pyspread/default.nix Outdated Show resolved Hide resolved
pkgs/applications/misc/pyspread-app/wrap-pyspread.nix Outdated Show resolved Hide resolved
pkgs/applications/misc/pyspread-app/wrap-pyspread.nix Outdated Show resolved Hide resolved
pkgs/applications/misc/pyspread-app/wrap-pyspread.nix Outdated Show resolved Hide resolved
@@ -22235,6 +22235,9 @@ in

pybitmessage = callPackage ../applications/networking/instant-messengers/pybitmessage { };

wrapPyspread = libsForQt5.callPackage ../applications/misc/pyspread-app/wrap-pyspread.nix { };
Copy link
Member

Choose a reason for hiding this comment

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

Expose only a single attribute, the application that works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When writing a NixOS module or a home-manager, should I call the file wrap-pyspread.nix directly?

Copy link
Contributor

Choose a reason for hiding this comment

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

no, you can reference pkgs.pyspread

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jonringer I tried for a while, but couldn't find a way to override pkgs.pyspread-app and change its pythonWithDependencies and customInterpreterName attributes.
How should I modify the wrapping expressions and write the override one to achieve this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I modified the program to make pyspread-app a set, and move the executable application to pyspread-app.app.

@ShamrockLee ShamrockLee requested a review from FRidh August 21, 2020 17:21
@zowoq
Copy link
Contributor

zowoq commented Aug 22, 2020

@ofborg eval

For the Python package
* Change dependent python version to 3.6-3.9
* Change dependent widget toolkit to pyqt5
For the application
* Add a wrapped application
* Add some functions that make overriding easier
* pyspread-app: a set containing
    * pyspread-app.app: the workable application
    * pyspread-app.wrapPyspread: the wrapping function
    * .pickRequiredDeps, .pickOptionalDeps:
functions to get deps from the given package set
@ShamrockLee
Copy link
Contributor Author

Close this PR as the previously broken package is fixed.

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

7 participants