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
Add flexmock, sqlalchemy-utils, pgpy, and py-gfm #50328
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
Thanks for the PR.
Could you add meta
attributes for flexmock
, PGPy
and py-gfm
?
Lancelot SIX <notifications@github.com> writes:
lsix requested changes on this pull request.
Hi,
Thanks for the PR.
Could you add `meta` attributes for `flexmock`, `PGPy` and `py-gfm` ?
Okay, I added the `meta` attributes, and added one more package,
'sshpubkeys'. Let me know if you need any more changes :)
|
worldofpeace <notifications@github.com> writes:
+ meta = with lib; {
+ homepage = https://github.com/zopieux/py-gfm;
+ description = "This is an implementation of GitHub-Flavored Markdown written as an extension to the Python Markdown library. It aims for maximal compatibility with GitHub's rendering.";
This description is too long.
Fixed in 9e0430c
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The requested changes should be done for the packages I didn't comment on as well.
You might want to read through https://nixos.org/nixpkgs/manual/#python again.
Robert Schütz <notifications@github.com> writes:
dotlambda requested changes on this pull request.
The requested changes should be done for the packages I didn't comment on as well.
Okay, I think I covered all of them. They all have meta, metaintainers, etc.
You might want to read through https://nixos.org/nixpkgs/manual/#python again.
Looking at the contributing guidelines, this is what I see:
Python libraries are called from python-packages.nix and packaged with
buildPythonPackage. The expression of a library should be in
pkgs/development/python-modules/<name>/default.nix. Libraries in
pkgs/top-level/python-packages.nix are sorted quasi-alphabetically to
avoid merge conflicts.
Check.
Python applications live outside of python-packages.nix and are packaged
with buildPythonApplication.
None of these are apps.
Make sure libraries build for all Python interpreters.
I think they all succeed. I'll look into testing this thoroughly over
the weekend or next week.
By default we enable tests. Make sure the tests are found and, in the
case of libraries, are passing for all interpreters. If certain tests
fail they can be disabled individually. Try to avoid disabling the tests
altogether. In any case, when you disable tests, leave a comment
explaining why.
Only one package failed to find the tests, and I added a comment
explaining that.
Commit names of Python libraries should reflect that they are Python
libraries, so write for example pythonPackages.numpy: 1.11 -> 1.12.
Check.
Attribute names in python-packages.nix should be normalized according to
PEP 0503. This means that characters should be converted to lowercase
and . and _ should be replaced by a single - (foo-bar-baz instead of
Foo__Bar.baz )
Check.
Okay, anything else I can do?
|
@dotlambda Anything I can do to help move this along? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don`t need to set any of these meta attributes
downloadPage = foo;
priority = 0;
broken = false;
updateWalker = true;
downloadPage
could be useful but we already know where we can download python packages from (pypi.org).
Additionally you've set platforms
to all
. This is already set to automatically to
platforms = python.meta.platforms; |
which should be sufficient.
@bsima |
e698546
to
a00aa73
Compare
worldofpeace <notifications@github.com> writes:
@bsima
Could you also `fixup` these additional commits with an interactive rebase?
It would make reviewing commit by commit possible.
Yes, I rebased to make things more clear, and updated the meta
info. Thanks for the feedback, and let me know if there is anything else
I can do.
|
}; | ||
|
||
checkInputs = [ nose pytest ]; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noticed the tests weren't being ran.
This should fix that:
checkPhase = '' | |
pytest tests/ | |
''; |
|
||
meta = with lib; { | ||
homepage = https://flexmock.readthedocs.io/en/latest/; | ||
description = "flexmock is a testing library for Python."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
description
should not have the name of said software in it or a period at the end.
https://nixos.org/nixpkgs/manual/#sec-standard-meta-attributes
Don’t include a period at the end. Don’t include newline characters. Capitalise the first character. For brevity, don’t repeat the name of package — just describe what it does.
description = "flexmock is a testing library for Python."; | |
description = "A testing library for Python that makes it easy to create mocks, stubs, and fakes"; |
meta = with lib; { | ||
homepage = https://flexmock.readthedocs.io/en/latest/; | ||
description = "flexmock is a testing library for Python."; | ||
longDescription = '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This description isn't long so we don't need it.
}: | ||
|
||
buildPythonPackage rec { | ||
pname = "PGPy"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're normalizing pname
now so this should be
pname = "PGPy"; | |
pname = "pgpy"; |
version = "0.4.3"; | ||
|
||
src = fetchPypi { | ||
inherit pname version; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the pname
will most likely make this not work here.
So we might need:
inherit pname version; | |
pname = "PGPy"; | |
inherit version; |
|
||
meta = with lib; { | ||
homepage = https://github.com/SecurityInnovation/PGPy; | ||
description = "Pretty Good Privacy for Python 2 and 3."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
description = "Pretty Good Privacy for Python 2 and 3."; | |
description = "Pretty Good Privacy for Python 2 and 3"; |
sha256 = "04412dddd6882ac0c5d5daf4326c28d481421851a68e25e7ac8e06cc9dc2b902"; | ||
}; | ||
|
||
propogatedBuildInputs = [ cryptography pyasn1 six singledispatch ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only depend on enum34 if Python is older than 3.4
We can constrain the addition of that with
propogatedBuildInputs = [ cryptography pyasn1 six singledispatch ]; | |
propogatedBuildInputs = [ cryptography pyasn1 six singledispatch ] ++ lib.optional (pythonOlder "3.4") enum34; |
|
||
meta = with lib; { | ||
homepage = https://github.com/zopieux/py-gfm; | ||
description = "GitHub-Flavored Markdown as an extension to the Python Markdown library."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
description = "GitHub-Flavored Markdown as an extension to the Python Markdown library."; | |
description = "GitHub-Flavored Markdown as an extension to the Python Markdown library"; |
meta = with lib; { | ||
homepage = https://github.com/zopieux/py-gfm; | ||
description = "GitHub-Flavored Markdown as an extension to the Python Markdown library."; | ||
longDescription = '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can have the long description if you want, but it's just a second sentence.
sha256 = "ef6750c579d26651cfd23968258b604228fd71b2a4e1f71dea3bea289e01377e"; | ||
}; | ||
|
||
buildInputs = [ markdown ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buildInputs = [ markdown ]; | |
propagatedBuildInputs = [ markdown ]; |
}: | ||
|
||
buildPythonPackage rec { | ||
pname = "SQLAlchemy-Utils"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're normalizing pname now so this should be
pname = "SQLAlchemy-Utils"; | |
pname = "sqlalchemy-utils"; |
version = "0.33.6"; | ||
|
||
src = fetchPypi { | ||
inherit pname version; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the pname will most likely make this not work here.
So we might need:
inherit pname version; | |
pname = "SQLAlchemy-Utils"; | |
inherit version; |
sha256 = "45ab41c90bfb8dd676e83179be3088b3f2d64b613e3b590187163dd941c22d4c"; | ||
}; | ||
checkInputs = [ pytest flexmock mock python-dateutil pytz ]; | ||
buildInputs = [ six sqlalchemy ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buildInputs = [ six sqlalchemy ]; | |
propagatedBuildInputs = [ six sqlalchemy ]; |
|
||
meta = with lib; { | ||
homepage = https://github.com/kvesteri/sqlalchemy-utils; | ||
description = "Various utility functions and datatypes for SQLAlchemy."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
description = "Various utility functions and datatypes for SQLAlchemy."; | |
description = "Various utility functions and datatypes for SQLAlchemy"; |
sha256 = "b388399caeeccdc145f06fd0d2665eeecc545385c60b55c282a15a022215af80"; | ||
}; | ||
|
||
buildInputs = [ cryptography ecdsa ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buildInputs = [ cryptography ecdsa ]; | |
propagatedBuildInputs = [ cryptography ecdsa ]; |
buildInputs = [ cryptography ecdsa ]; | ||
|
||
# For some reason nix can't find the "tests" module. | ||
doCheck = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems something is not right with the pypi tarball.
Everything works fine when I use the github repo.
src = fetchFromGitHub {
owner = "ojarva";
repo = "python-sshpubkeys";
rev = "v${version}";
sha256 = "0000000000000000000000000000000000000000000000000000"; # change this
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add a comment above it explaining why we have to use github.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally tests need aren't running or are missing checkInputs
for:
- py-gfm
- pgpy
Closing in lieu of #54425 - much more progress and better code in that one :) |
Motivation for this change
Trying to build core.sr.ht for NixOS and running into missing packages, figured I would add them upstream. I tested each one with
nix-build -A pythonPackages.<pkgname>
to make sure they built.Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)