-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
pythonPackages.rlp: 1.0.1 -> 1.0.2 (with some new deps) #45644
Conversation
30c4ccd
to
4728055
Compare
It's nowadays possible to use markdown directly, so no need for Could you inform upstream? |
4728055
to
61eb081
Compare
Ok. I noticed nox-review is giving some failures at the moment. I'll try to fix them before merging. |
@FRidh Now nox-review passed. I needed to add some more commits to make the fixes. |
doCheck = false; | ||
|
||
meta = with stdenv.lib; { | ||
description = "Backport of shutil.which from Python 3.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.
Maybe python 3 should be disabled here?
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.
sha256 = "16sa3adkf71862cb9pk747pw80a2f1v5m915ijb4fgj309xrlhyx"; | ||
}; | ||
|
||
# Tests fail: "ValueError: underlying buffer has been detached" |
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.
Is this comment copy-and-pasted from python-u2flib-host?
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 tried to build and the test phase raised that error. I tried to fix it but it has probably something to do with the test environment in nix. Not sure though.
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.
For me this test works perfectly fine.
Upstream is also informed: ethereum/eth-utils#124 |
@GrahamcOfBorg build python3Packages.backports_shutil_which |
Success on aarch64-linux (full log) Attempted: python3Packages.backports_shutil_which Partial log (click to expand)
|
Success on x86_64-darwin (full log) Attempted: python3Packages.backports_shutil_which Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: python3Packages.backports_shutil_which Partial log (click to expand)
|
Is this ok now? |
|
||
meta = with stdenv.lib; { | ||
description = "A simple, cross-platform, pure Python module for JavaScript-like message boxes"; | ||
homepage = "https://github.com/asweigart/PyMsgBox"; |
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.
Homepage says this depends on tkinter. Btw, you should leave out the quotes.
substituteInPlace setup.py --replace \'setuptools-markdown\' "" | ||
''; | ||
|
||
disabled = !(pythonAtLeast "3.5"); |
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.
pythonOlder
substituteInPlace setup.py --replace \'setuptools-markdown\' "" | ||
''; | ||
|
||
disabled = !(pythonAtLeast "3.5"); |
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.
pythonOlder
pkgs/top-level/python-packages.nix
Outdated
@@ -437,6 +439,8 @@ in { | |||
|
|||
pymatgen = callPackage ../development/python-modules/pymatgen { }; | |||
|
|||
PyMsgBox = callPackage ../development/python-modules/PyMsgBox { }; |
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'd prefer using pymsgbox
, also as dir name.
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.
Yes, I can change the dir name. How about adding below that:
pymsgbox = PyMsgBox;
I've heard the package name should be equal to the name in PyPI, so that's why I thought it should be available at least as PyMsgBox
.
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 would also prefer if it matches. We should have a policy on that, so we do this consistently for python packages.
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.
How about a policy to have both the PyPI name and a lowercase alias?
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've written this before several times. We prefer to use normalized names throughout, because that matches well with the naming we use throughout Nixpkgs. The pname
to fetchPypi
however has to match to the filename used because PyPI does not translate that part of the url. If for fetchPypi
the name cannot be normalized, then it may be more convenient to use the same one for buildPython*
.
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.
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.
Ok. let's make this convention part of nixpkgs doc: #45822 (comment)
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.
So I should rename backports_shutil_which
to backports-shutil-which
, right?
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.
Yes
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.
Fixed.
Success on x86_64-darwin (full log) Attempted: python3.pkgs.eth-hash, python3.pkgs.eth-typing, python3.pkgs.eth-utils The following builds were skipped because they don't evaluate on x86_64-darwin: pythob2.pkgs.eth-hash, python2.pkgs.eth-typing, python2.pkgs.eth-utils Partial log (click to expand)
|
Success on aarch64-linux (full log) Attempted: python3.pkgs.eth-hash, python3.pkgs.eth-typing, python3.pkgs.eth-utils The following builds were skipped because they don't evaluate on aarch64-linux: pythob2.pkgs.eth-hash, python2.pkgs.eth-typing, python2.pkgs.eth-utils Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: python3.pkgs.eth-hash, python3.pkgs.eth-typing, python3.pkgs.eth-utils The following builds were skipped because they don't evaluate on x86_64-linux: pythob2.pkgs.eth-hash, python2.pkgs.eth-typing, python2.pkgs.eth-utils Partial log (click to expand)
|
All of the
You should either provide an appropriate |
At least |
Thanks! It's fine to set |
9202945
to
d44bb56
Compare
I put I enabled tests for |
Once ported to 18.09, it would contribute to fixing around 12 outstanding builds on x86_64, see ZHF #45960, and the generated reports. (Subscribing to keep track of the changes.) |
Upstream of |
d44bb56
to
fea2d86
Compare
Done. I also updated the version numbers of eth-utils and eth-typing. Nox review passed. |
fea2d86
to
8d4afa5
Compare
Thanks a lot @jluttine! |
How does that work? Shall I cherry pick these 10 commits to that release branch? I can do it but I've never done it before so need to learn it. |
Well, then it's a good thing to learn :) |
[release-18.09 735b1e1] pythonPackages.eth-hash: init at 0.1.14 Pushed for ZHF #45960 |
Motivation for this change
Updated and fixed RLP Python package. Some dependencies are added as separate commits.
fixes #44224
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)