setup.py: add "wheel" to setup_requires by alanvgreen · Pull Request #349 · amaranth-lang/amaranth

alanvgreen · 2020-04-10T01:41:32Z

Fixes problem when installing into a python venv created with

$ python3 -m venv my_env

Fixes problem when installing into a python venv created with $ python3 -m venv my_env See m-labs/nmigen#323

whitequark · 2020-04-10T01:55:54Z

Do you know why this dependency is necessary? It looks like Python is trying to build a wheel here, but the wheel library doesn't exist, which seems like a strange situation.

Does this only happen if you install nmigen from git in virtualenv, and not otherwise?

codecov · 2020-04-10T02:04:48Z

Codecov Report

Merging #349 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #349   +/-   ##
=======================================
  Coverage   82.64%   82.64%           
=======================================
  Files          35       35           
  Lines        5948     5948           
  Branches     1208     1208           
=======================================
  Hits         4916     4916           
  Misses        868      868           
  Partials      164      164

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3899b2b...3899b2b. Read the comment docs.

The default __repr__() from typing.NamedTuple does not include the module name, so the replacement (which uses the preferred syntax for specifying these shapes) doesn't either.

Fixes #326.

Fixes #353.

Fixes #147.

Also known as word_select().

Before this commit, selecting a part that was fully out of bounds of a value was correctly implemented as a write to a dummy wire, but selecting a part that was only partially out of bounds resulted in a crash. Fixes #351.

Such wires are likely to trigger pathological behavior in Yosys and, if applicable, other toolchains that consume Verilog converted from RTLIL. Fixes #341.

Fixes #335.

By default, if an operation produces an undefined value (a Jinja2 concept that corresponds to Python's KeyError, AttributeError, etc) then this value may be printed in a template, which is a nop. This behavior can hide bugs. This commit changes the Jinja2 behavior to raise an error instead of producing an undefined value in all cases. (We produce undefined values deliberately in a few places. Those are unaffected; it is OK to use several kinds of undefined values in one Jinja2 environment.) Fixes #337.

FFY00 · 2020-04-14T11:36:14Z

This does not belong in setup_requires, at most, it would be in extras_require. Although, I can't reproduce, it seems like a bug in your setup.

whitequark · 2020-04-14T14:57:32Z

I am not able to reproduce either:

~$ virtualenv venvtest
Using base prefix '/usr'
New python executable in /home/whitequark/venvtest/bin/python3
Also creating executable in /home/whitequark/venvtest/bin/python
Installing setuptools, pip, wheel...
done.
~$ cd venvtest/
~/venvtest$ . bin/activate
(venvtest) ~/venvtest$ pip install git+https://github.com/nmigen/nmigen.git
Collecting git+https://github.com/nmigen/nmigen.git
  Cloning https://github.com/nmigen/nmigen.git to /tmp/pip-req-build-7qdlu_3v
  Running command git clone -q https://github.com/nmigen/nmigen.git /tmp/pip-req-build-7qdlu_3v
Requirement already satisfied: setuptools in ./lib/python3.7/site-packages (from nmigen==0.3.dev33+g3346f2c) (46.1.3)
Collecting pyvcd~=0.2.0
  Downloading pyvcd-0.2.1-py2.py3-none-any.whl (17 kB)
Collecting Jinja2~=2.11
  Downloading Jinja2-2.11.2-py2.py3-none-any.whl (125 kB)
     |████████████████████████████████| 125 kB 2.7 MB/s
Collecting MarkupSafe>=0.23
  Downloading MarkupSafe-1.1.1-cp37-cp37m-manylinux1_x86_64.whl (27 kB)
Building wheels for collected packages: nmigen
  Building wheel for nmigen (setup.py) ... done
  Created wheel for nmigen: filename=nmigen-0.3.dev33+g3346f2c-py3-none-any.whl size=174944 sha256=50252426adffbfbd67f6d9ce85df534d9e1ea16c2872252e56f2e62fb4452bd7
  Stored in directory: /tmp/pip-ephem-wheel-cache-yzut3x95/wheels/82/71/f3/45c024858a8a85e66b358edd4a555c975f96a4e343a1d21e90
Successfully built nmigen
Installing collected packages: pyvcd, MarkupSafe, Jinja2, nmigen
Successfully installed Jinja2-2.11.2 MarkupSafe-1.1.1 nmigen-0.3.dev33+g3346f2c pyvcd-0.2.1
(venvtest) ~/venvtest$

mithro · 2020-04-14T15:22:33Z

I think this is a difference between virtualenv and venv (which are confusingly different tools). venv doesn't install wheel into the environment by default it seems while virtualenv does. I think virtualenv --no-wheel would give you the same behaviour as venv.

FFY00 · 2020-04-14T16:23:40Z

I created the environment with venv, just as described and I was able to install nmigen. Maybe an older version of venv didn't install wheel?

Either way, there is nothing in nmigen that explicitly requires wheel. If setuptools_scm is using wheel for the installation then wheel is its dependency. If wheel was an optional dependency of setuptools_scm and was required here, then it should be in setup_requires, but it isn't.

mithro · 2020-04-14T17:15:49Z

@FFY00 - Your on Arch Linux right? I think the problem is with how Ubuntu / Debianism mangles Python packages. Is wheel is an optional dependency of setuptools / pip?

FFY00 · 2020-04-14T17:21:51Z

Then it would be a Debian bug. No, wheel is not a dependency for either setuptool_scm or pip, at least in the latest versions.

The default __repr__() from typing.NamedTuple does not include the module name, so the replacement (which uses the preferred syntax for specifying these shapes) doesn't either.

Fixes #326.

Fixes #353.

Fixes #147.

Also known as word_select().

Before this commit, selecting a part that was fully out of bounds of a value was correctly implemented as a write to a dummy wire, but selecting a part that was only partially out of bounds resulted in a crash. Fixes #351.

Such wires are likely to trigger pathological behavior in Yosys and, if applicable, other toolchains that consume Verilog converted from RTLIL. Fixes #341.

Fixes #335.

By default, if an operation produces an undefined value (a Jinja2 concept that corresponds to Python's KeyError, AttributeError, etc) then this value may be printed in a template, which is a nop. This behavior can hide bugs. This commit changes the Jinja2 behavior to raise an error instead of producing an undefined value in all cases. (We produce undefined values deliberately in a few places. Those are unaffected; it is OK to use several kinds of undefined values in one Jinja2 environment.) Fixes #337.

Fixes problem when installing into a python venv created with $ python3 -m venv my_env See m-labs/nmigen#323

alanvgreen · 2020-04-15T05:35:39Z

I am seeing the problem on Ubuntu.

Installation is successful with "wheel" in extra_requires rather than setup_requires.

One aspect of this problem is that wheel appears to be installed by default with virtualenv, but not with venv.

FFY00 · 2020-04-15T06:01:58Z

How does your setup.py look like? There's is no extra_requires, it is called extras_require and it's not a list like setup_requires.

Can you post the exact commands to reproduce?

It seems like you messed up the history by doing a merge. Can you look at reflog, do a hard reset to a previous state and then rebase on master? This should result in a clean git tree with just one commit on top of master. Let me know if you need help with the commands.

ohsix · 2020-04-15T06:12:45Z

what version of ubuntu and python3?

whitequark · 2020-04-15T06:24:22Z

@alanvgreen I believe you are hitting an issue with the way Debian ships Python packages. If you install python3-wheel from apt, do you still have this problem?

alanvgreen · 2020-04-15T07:09:11Z

@ohsix : from the bug report

(life) osboxes@osboxes:~/life$ uname -a
Linux osboxes 5.3.0-45-generic #37-Ubuntu SMP Thu Mar 26 20:41:27 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
(life) osboxes@osboxes:~/life$ python --version
Python 3.7.5
(life) osboxes@osboxes:~/life$ lsb_release -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 19.10
Release:	19.10
Codename:	eoan

Fixes problem when installing into a python venv created with $ python3 -m venv my_env See m-labs/nmigen#323

alanvgreen · 2020-04-15T07:51:10Z

@FFY00 Thanks for your kind offer, I think I managed to straighten out my pull request. Let me know if not.

Here is a repro:

osboxes@osboxes:~$ python3 -m venv test
osboxes@osboxes:~$ cd test
osboxes@osboxes:~/test$ . bin/activate
(test) osboxes@osboxes:~/test$ pip install nmigen
Collecting nmigen
  Downloading https://files.pythonhosted.org/packages/43/08/162d253c8d41460cffda0ca4d60f7fb075bb2b787b82089189c76a33f121/nmigen-0.2.tar.gz (144kB)
    100% |████████████████████████████████| 153kB 2.8MB/s 
Collecting Jinja2 (from nmigen)
  Using cached https://files.pythonhosted.org/packages/30/9e/f663a2aa66a09d838042ae1a2c5659828bb9b41ea3a6efa20a20fd92b121/Jinja2-2.11.2-py2.py3-none-any.whl
Collecting pyvcd~=0.1.4 (from nmigen)
  Using cached https://files.pythonhosted.org/packages/01/3e/2ff169806a094365722f6670cc777f83bd93c56471ccc260cc5e0ba36b50/pyvcd-0.1.7-py2.py3-none-any.whl
Requirement already satisfied: setuptools in ./lib/python3.7/site-packages (from nmigen) (40.8.0)
Collecting MarkupSafe>=0.23 (from Jinja2->nmigen)
  Using cached https://files.pythonhosted.org/packages/98/7b/ff284bd8c80654e471b769062a9b43cc5d03e7a615048d96f4619df8d420/MarkupSafe-1.1.1-cp37-cp37m-manylinux1_x86_64.whl
Collecting six (from pyvcd~=0.1.4->nmigen)
  Using cached https://files.pythonhosted.org/packages/65/eb/1f97cb97bfc2390a276969c6fae16075da282f5058082d4cb10c6c5c1dba/six-1.14.0-py2.py3-none-any.whl
Building wheels for collected packages: nmigen
  Running setup.py bdist_wheel for nmigen ... error
  Complete output from command /home/osboxes/test/bin/python3 -u -c "import setuptools, tokenize;__file__='/tmp/pip-install-p6ju8i6j/nmigen/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" bdist_wheel -d /tmp/pip-wheel-oeglfmeo --python-tag cp37:
  usage: -c [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
     or: -c --help [cmd1 cmd2 ...]
     or: -c --help-commands
     or: -c cmd --help
  
  error: invalid command 'bdist_wheel'
  
  ----------------------------------------
  Failed building wheel for nmigen
  Running setup.py clean for nmigen
Failed to build nmigen
Installing collected packages: MarkupSafe, Jinja2, six, pyvcd, nmigen
  Running setup.py install for nmigen ... done
Successfully installed Jinja2-2.11.2 MarkupSafe-1.1.1 nmigen-0.2 pyvcd-0.1.7 six-1.14.0
(test) osboxes@osboxes:~/test$

If I then attempt to pip install nmigen a second time, things look okay:

(test) osboxes@osboxes:~/test$ pip install nmigen
Requirement already satisfied: nmigen in ./lib/python3.7/site-packages (0.2)
Requirement already satisfied: Jinja2 in ./lib/python3.7/site-packages (from nmigen) (2.11.2)
Requirement already satisfied: pyvcd~=0.1.4 in ./lib/python3.7/site-packages (from nmigen) (0.1.7)
Requirement already satisfied: setuptools in ./lib/python3.7/site-packages (from nmigen) (40.8.0)
Requirement already satisfied: MarkupSafe>=0.23 in ./lib/python3.7/site-packages (from Jinja2->nmigen) (1.1.1)
Requirement already satisfied: six in ./lib/python3.7/site-packages (from pyvcd~=0.1.4->nmigen) (1.14.0)
(test) osboxes@osboxes:~/test$ python
Python 3.7.5 (default, Nov 20 2019, 09:21:52) 
[GCC 9.2.1 20191008] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from nmigen import *
>>>

I don't know what to make of that - I wouldn't trust the installation.

Looking at https://setuptools.readthedocs.io/en/latest/setuptools.html#declaring-extras-optional-features-with-their-own-dependencies I'm not sure why extras_require is the right thing to use here. What did you have in mind?

alanvgreen · 2020-04-15T07:55:18Z

@whitequark It turns out that I already had python3-wheel installed. I can't recall whether I explicitly installed it before - it may have been a dependency of something else, but very likely it was something I tried while working through my problems.

At this stage, I'm quite happy for the resolution of this PR to be "use virtualenv, not venv" :)

whitequark · 2020-04-15T07:58:28Z

It looks like the documentation for venv describes it as deprecated since 3.6. Is that right?

programmerjake · 2020-04-15T08:07:34Z

It looks like the documentation for venv describes it as deprecated since 3.6. Is that right?

only the pyvenv script was deprecated where python -m venv is the recommended replacement:
https://docs.python.org/dev/whatsnew/3.6.html#id8

whitequark · 2020-04-15T08:29:25Z

@alanvgreen If you're happy using virtualenv then I suggest closing this, since it seems very hard to reproduce. If someone manages to find a good solution here I'm happy to merge it as well.

alanvgreen · 2020-04-15T09:27:57Z

Yup. Closing, thanks all!

graingert · 2020-04-15T14:25:08Z

@alanvgreen try

pip install git+git://github.com/graingert/nmigen@4c6070d#egg=nmigen

alanvgreen · 2020-04-15T20:10:02Z

@graingert Yes, that seems to work for me:

osboxes@osboxes:~$ python3 -m venv test
osboxes@osboxes:~$ cd test
osboxes@osboxes:~/test$ . bin/activate
(test) osboxes@osboxes:~/test$ pip install git+git://github.com/graingert/nmigen@4c6070d#egg=nmigen
Collecting nmigen from git+git://github.com/graingert/nmigen@4c6070d#egg=nmigen
  Cloning git://github.com/graingert/nmigen (to revision 4c6070d) to /tmp/pip-install-bzsob8tr/nmigen
  Did not find branch or tag '4c6070d', assuming revision or ref.
  Installing build dependencies ... done
Collecting pyvcd~=0.2.0 (from nmigen)
  Using cached https://files.pythonhosted.org/packages/95/44/1e4302b02fd5a5c65e2a3846b0a9468c7e3c9d52ba546d6ff1d17fa378a8/pyvcd-0.2.1-py2.py3-none-any.whl
Collecting Jinja2~=2.11 (from nmigen)
  Using cached https://files.pythonhosted.org/packages/30/9e/f663a2aa66a09d838042ae1a2c5659828bb9b41ea3a6efa20a20fd92b121/Jinja2-2.11.2-py2.py3-none-any.whl
Collecting MarkupSafe>=0.23 (from Jinja2~=2.11->nmigen)
  Using cached https://files.pythonhosted.org/packages/98/7b/ff284bd8c80654e471b769062a9b43cc5d03e7a615048d96f4619df8d420/MarkupSafe-1.1.1-cp37-cp37m-manylinux1_x86_64.whl
Building wheels for collected packages: nmigen
  Running setup.py bdist_wheel for nmigen ... done
  Stored in directory: /tmp/pip-ephem-wheel-cache-ekr8zajb/wheels/7f/7f/14/1d74905917927f926962a87dc38dafdb57ececd753331b2d89
Successfully built nmigen
Installing collected packages: pyvcd, MarkupSafe, Jinja2, nmigen
Successfully installed Jinja2-2.11.2 MarkupSafe-1.1.1 nmigen-0.3.dev36+g4c6070d pyvcd-0.2.1
(test) osboxes@osboxes:~/test$

whitequark · 2020-04-16T07:45:27Z

@alanvgreen Thanks for confirming! There are some issues with #358 that prevent merging it in the short term, but I'll make sure that this is fixed at some point. For now, please use virtualenv.

This is necessary to be able to install nMigen into a virtualenv that does not have `wheel` installed in certain cases. See #349.

whitequark · 2020-07-01T05:59:37Z

@alanvgreen I fixed this properly in 7fca037.

setup.py: add "wheel" to setup_requires

8acb27b

Fixes problem when installing into a python venv created with $ python3 -m venv my_env See m-labs/nmigen#323

whitequark and others added 13 commits April 12, 2020 03:28

build.plat: don't check for toolchain presence if do_build=False.

9055090

hdl.ast: improve repr() for Shape.

e9c75f7

The default __repr__() from typing.NamedTuple does not include the module name, so the replacement (which uses the preferred syntax for specifying these shapes) doesn't either.

hdl.rec: improve repr() for Layout.

7a08901

Fixes #326.

build.run: fix BuildProducts.extract to work with subdirectories.

91d6e4b

Fixes #353.

Travis: upgrade to bionic.

7ba362a

Travis: require tests to pass on pypy3.

8b13743

Fixes #147.

hdl.ast: add Value.{rotate_left,rotate_right}.

06c45c9

Clarify a few comments. NFC.

b44870e

back.rtlil: fix legalization of Part() with stride.

edd2bb2

Also known as word_select().

back.rtlil: fix expansion of Part() for partial dummy writes.

814ffde

Before this commit, selecting a part that was fully out of bounds of a value was correctly implemented as a write to a dummy wire, but selecting a part that was only partially out of bounds resulted in a crash. Fixes #351.

back.rtlil: refuse to create extremely large wires.

792f35a

Such wires are likely to trigger pathological behavior in Yosys and, if applicable, other toolchains that consume Verilog converted from RTLIL. Fixes #341.

back.rtlil: don't emit connections to zero width ports.

ee73d39

Fixes #335.

whitequark and others added 8 commits April 15, 2020 01:31

build.plat: don't check for toolchain presence if do_build=False.

745857a

hdl.ast: improve repr() for Shape.

0a2e6e2

The default __repr__() from typing.NamedTuple does not include the module name, so the replacement (which uses the preferred syntax for specifying these shapes) doesn't either.

hdl.rec: improve repr() for Layout.

70af2af

Fixes #326.

build.run: fix BuildProducts.extract to work with subdirectories.

4a3ce42

Fixes #353.

Travis: upgrade to bionic.

5ec5114

Travis: require tests to pass on pypy3.

a06db57

Fixes #147.

hdl.ast: add Value.{rotate_left,rotate_right}.

4f07755

Clarify a few comments. NFC.

bba709c

whitequark and others added 7 commits April 15, 2020 01:31

back.rtlil: fix legalization of Part() with stride.

6b0a50f

Also known as word_select().

back.rtlil: fix expansion of Part() for partial dummy writes.

a2b8492

Before this commit, selecting a part that was fully out of bounds of a value was correctly implemented as a write to a dummy wire, but selecting a part that was only partially out of bounds resulted in a crash. Fixes #351.

back.rtlil: refuse to create extremely large wires.

70b7a16

Such wires are likely to trigger pathological behavior in Yosys and, if applicable, other toolchains that consume Verilog converted from RTLIL. Fixes #341.

back.rtlil: don't emit connections to zero width ports.

b639c76

Fixes #335.

setup.py: add "wheel" to extra_requires

90b46e5

Fixes problem when installing into a python venv created with $ python3 -m venv my_env See m-labs/nmigen#323

Merge branch 'master' of github.com:alanvgreen/nmigen

8f618f1

alanvgreen added 3 commits April 15, 2020 03:18

setup.py: add "wheel" to extra_requires

a7883dc

Fixes problem when installing into a python venv created with $ python3 -m venv my_env See m-labs/nmigen#323

setup.py: add "wheel" to setup_requires

8111cc0

Fixes problem when installing into a python venv created with $ python3 -m venv my_env See m-labs/nmigen#323

Merge branch 'master' of github.com:alanvgreen/nmigen

3899b2b

alanvgreen closed this Apr 15, 2020

whitequark added a commit that referenced this pull request Jul 1, 2020

Add PEP 518 pyproject.toml.

59aabc6

This is necessary to be able to install nMigen into a virtualenv that does not have `wheel` installed in certain cases. See #349.

whitequark added a commit that referenced this pull request Jul 1, 2020

Add PEP 518 pyproject.toml.

7fca037

This is necessary to be able to install nMigen into a virtualenv that does not have `wheel` installed in certain cases. See #349.

Uh oh!

Conversation

alanvgreen commented Apr 10, 2020

Uh oh!

whitequark commented Apr 10, 2020

Uh oh!

codecov Bot commented Apr 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

FFY00 commented Apr 14, 2020

Uh oh!

whitequark commented Apr 14, 2020

Uh oh!

mithro commented Apr 14, 2020

Uh oh!

FFY00 commented Apr 14, 2020

Uh oh!

mithro commented Apr 14, 2020

Uh oh!

FFY00 commented Apr 14, 2020

Uh oh!

alanvgreen commented Apr 15, 2020

Uh oh!

FFY00 commented Apr 15, 2020

Uh oh!

ohsix commented Apr 15, 2020

Uh oh!

whitequark commented Apr 15, 2020

Uh oh!

alanvgreen commented Apr 15, 2020

Uh oh!

alanvgreen commented Apr 15, 2020

Uh oh!

alanvgreen commented Apr 15, 2020

Uh oh!

whitequark commented Apr 15, 2020

Uh oh!

programmerjake commented Apr 15, 2020

Uh oh!

whitequark commented Apr 15, 2020

Uh oh!

alanvgreen commented Apr 15, 2020

Uh oh!

graingert commented Apr 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alanvgreen commented Apr 15, 2020

Uh oh!

whitequark commented Apr 16, 2020

Uh oh!

whitequark commented Jul 1, 2020

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

8 participants

codecov Bot commented Apr 10, 2020 •

edited

Loading

graingert commented Apr 15, 2020 •

edited

Loading