Fix dependency issues (and fastecdsa windows depencency) #380
Conversation
|
Edit: Nevermind, fixed. |
setup.py
Outdated
| @@ -33,6 +35,7 @@ | |||
| "ipython", | |||
| "setuptools>=36.2.0", | |||
| "tox>=3.13.2,<4.0.0", | |||
| "eth_utils", | |||
There was a problem hiding this comment.
Why do we need eth_utils here?
There was a problem hiding this comment.
From the issue template, it's asked to run python -m eth_utils, it'll be useful to have that as a part of a default development suite.
There was a problem hiding this comment.
I think that's the typo in the issue template and we should fix it. IMO we shouldn't have eth_utils here.
setup.py
Outdated
| # See the following issues for more information; | ||
| # https://github.com/libp2p/py-libp2p/issues/363 | ||
| # https://github.com/AntonKueltz/fastecdsa/issues/11 | ||
| "fastecdsa@git+https://github.com/shadowjonathan/fastecdsa-any@" |
There was a problem hiding this comment.
Would you mind having a PR in AntonKueltz/fastecdsa to fix the issue there, and then we depend on their fixed release? IIRC, PyPI does not accept this kind of git dependency?
There was a problem hiding this comment.
I've already notified AntonKueltz/fastecdsa about this, but it is going slow, I'm adding this dependency specifically for development on windows.
The alternative is to make a pypi package called fastedcsa-any, which'll contain two wheels (win32, win_amd64) so windows installation actually works without it resorting to trying to build a wheel from scratch.
Should I impliment the latter one?
There was a problem hiding this comment.
I've uploaded python wheels for fastecdsa to pypi: fastecdsa-any
These are built for for windows (32-bit and 64-bit) python 3.6 through 3.8, library version 1.7.4 and 1.7.5 in a matrix, so I suggest replacing this git+http notion (pypi accepts any kind of dependency, it does not care for dependencies, only uploaded tarballs and files) with fastecdsa-any for windows systems specifically.
There was a problem hiding this comment.
The current requirement works for me(fastecdsa-any==1.7.5 for windows and fastecdsa==1.7.5 for the rest of the platforms). We can switch to something like fastecdsa>=1.7.6 after they support windows.
add fastecdsa-any requirements
ShadowJonathan
force-pushed
the
fix_dependencies
branch
from
b39795a
to
1124fc8
Compare
|
Force-pushed changes to the branch to simplify the git history, the original branch (with original commits) can be found on |
setup.py
Outdated
| @@ -33,6 +35,7 @@ | |||
| "ipython", | |||
| "setuptools>=36.2.0", | |||
| "tox>=3.13.2,<4.0.0", | |||
| "eth_utils", | |||
There was a problem hiding this comment.
I think that's the typo in the issue template and we should fix it. IMO we shouldn't have eth_utils here.
+add:
p2pclient, pexpect, eth_utilstosetup.py+add: custom
fastecdsastrategy in regards to windows+bump:
fastecdsafrom1.7.4to1.7.5(only changes are 2 lines over here, in
point.py)What was wrong?
Default
pip install -e .[dev]did not result in complete installation of all dependencies needed for normal development.#363, windows installations and development fails because
fastecdsadoes not have a wheel, nor a easily-installable library by default.How was it fixed?
Adding 3 extra dependencies in
testanddev, correspondent with:Furthermore, adding a modded version of
fastecdsafrom https://github.com/shadowjonathan/fastecdsa-any when a windows platform is detected.fastecdsadoes currently not support nor build a windows wheel, but I have found a (probable) way to make installation succeed with the right libraries.Warning: this implementation is not tested, I am adding that modded version to this library exactly to test and find problems with a build, and if fastecdsa is viable for libp2p in the end.
(see AntonKueltz/fastecdsa#11)
Lastly,
fastecdsais bumped to1.7.5for uniformity with the modded windows version, adiffbetween the two versions revealed to me that these two lines are the only things that changed between the versions. Please test and verify if this does not break things.To-Do
fastecdsa==1.7.5on all platforms to assure nothing is brokenCute Animal Picture