Bug 433 - building pia (power instruction analyser) needs to be documented
Summary: building pia (power instruction analyser) needs to be documented
Status: RESOLVED FIXED
Alias: None
Product: Libre-SOC's first SoC
Classification: Unclassified
Component: Documentation (show other bugs)
Version: unspecified
Hardware: Other Linux
: --- enhancement
Assignee: Jacob Lifshay
URL:
Depends on:
Blocks: 324
  Show dependency treegraph
 
Reported: 2020-07-15 16:41 BST by Luke Kenneth Casson Leighton
Modified: 2020-07-29 20:13 BST (History)
3 users (show)

See Also:
NLnet milestone: ---
total budget (EUR) for completion of task and all subtasks: 0
budget (EUR) for this task, excluding subtasks' budget: 0
parent task for budget allocation:
child tasks for budget allocation:
The table of payments (in EUR) for this task; TOML format:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Luke Kenneth Casson Leighton 2020-07-15 16:41:57 BST
https://bugs.libre-soc.org/show_bug.cgi?id=432#c13

* in soc INSTALL.txt, build instructions are needed
* likewise a section in HDL_workflow
* and an addition to dev-setup-scripts
* and possible addition to soc setup.py (to be discussed)
Comment 1 Jacob Lifshay 2020-07-15 16:49:26 BST
just needs a relatively recent version of Rust (last year or so, didn't check how far back it works) -- nightly no longer needed since PyO3 v0.11.x

build script (on x86_64 Linux):
pip install .

build script (on something else or if you don't like pip -- requires a virtualenv):
cargo install maturin
maturin develop

build script (if you also don't like virtualenv):
cargo install maturin
maturin build
# could have path to wheel incorrect, icr for sure
pip install target/wheel/*.whl
Comment 2 Jacob Lifshay 2020-07-15 16:54:13 BST
(In reply to Jacob Lifshay from comment #1)
> pip install target/wheel/*.whl

make that:
python3 -m pip install target/wheel/*.whl
Comment 3 Jacob Lifshay 2020-07-15 16:57:56 BST
I keep forgetting everything :P

(In reply to Jacob Lifshay from comment #1)
> just needs a relatively recent version of Rust (last year or so, didn't
> check how far back it works) -- nightly no longer needed since PyO3 v0.11.x
> 
> build script (on x86_64 Linux):
> pip install .
> 
> build script (on something else or if you don't like pip -- requires a
> virtualenv):
> cargo install maturin
> maturin develop
should be:
maturin develop --cargo-extra-args=--features=python-extension
> 
> build script (if you also don't like virtualenv):
> cargo install maturin
> maturin build
should be:
maturin build --cargo-extra-args=--features=python-extension
> # could have path to wheel incorrect, icr for sure
> <snip>
Comment 4 Luke Kenneth Casson Leighton 2020-07-15 17:10:37 BST
(In reply to Jacob Lifshay from comment #3)
> I keep forgetting everything :P

*cackle* hence the very reason for scripting it :)
Comment 5 Luke Kenneth Casson Leighton 2020-07-24 11:25:07 BST
i realised that with the addition of this code as a critical dependency
to soc, a few days ago, it again appears that the debian team is the sole
exclusive authoritative source and "go-to location" for code that is our responsibility, not theirs.

i've created gitolite3@libre-soc.org:power-instruction-analyzer.git
and the build and install documentation should refer to that.

a case _could_ be made that this is a 3rd party library...
Comment 6 Luke Kenneth Casson Leighton 2020-07-24 12:10:34 BST
raising priority on this one as it's blocking progress.  the README
in pia is missing build instructions, and "pip3 install ." doesn't
work because there is no setup.py.

this is probably because there is no instructions on what to do
such that setup.py *gets created*... or... it is a missing file.
Comment 7 Cole Poirier 2020-07-24 18:35:45 BST
(In reply to Luke Kenneth Casson Leighton from comment #6)
> raising priority on this one as it's blocking progress.  the README
> in pia is missing build instructions, and "pip3 install ." doesn't
> work because there is no setup.py.
> 
> this is probably because there is no instructions on what to do
> such that setup.py *gets created*... or... it is a missing file.

I'll work on this today. Will comment here in the next few hours with my progress/problems.
Comment 8 Jacob Lifshay 2020-07-24 18:57:27 BST
(In reply to Luke Kenneth Casson Leighton from comment #6)
> raising priority on this one as it's blocking progress.  the README
> in pia is missing build instructions, and "pip3 install ." doesn't
> work because there is no setup.py.
> 
> this is probably because there is no instructions on what to do
> such that setup.py *gets created*... or... it is a missing file.

setup.py is not needed since it has a pyproject.toml file, which is the new standard. if pip doesn't work for you, try updating pip.
Comment 9 Jacob Lifshay 2020-07-24 18:59:21 BST
related: https://github.com/PyO3/maturin/issues/332
"Support Android with Termux"
Comment 10 Luke Kenneth Casson Leighton 2020-07-24 19:08:10 BST
(In reply to Jacob Lifshay from comment #8)

> > this is probably because there is no instructions on what to do
> > such that setup.py *gets created*... or... it is a missing file.
> 
> setup.py is not needed since it has a pyproject.toml file, which is the new
> standard. if pip doesn't work for you, try updating pip.

so that's something that needs to be in the README / INSTALL.
Comment 11 Luke Kenneth Casson Leighton 2020-07-24 19:13:43 BST
this is not inspiring me with confidence...

lkcl@fizzy:~/src/libresoc/power-instruction-analyzer$ cargo install maturin
    Updating crates.io index
  Installing maturin v0.8.2                                                     
error: failed to compile `maturin v0.8.2`, intermediate artifacts can be found at `/tmp/cargo-installWSSMHV`

Caused by:
  failed to parse lock file at: /home/lkcl/.cargo/registry/src/github.com-1ecc6299db9ec823/maturin-0.8.2/Cargo.lock
Comment 12 Luke Kenneth Casson Leighton 2020-07-24 19:15:48 BST
this is no good.  i have absolutely no idea how to install (or upgrade) this stuff.  *full* build instructions are needed - from scratch, assuming zero prior knowledge, and zero prior tools installed.

 

   --> /home/lkcl/.cargo/registry/src/github.com-1ecc6299db9ec823/pin-project-lite-0.1.7/src/lib.rs:744:21
    |                                                                           
744 |     impl<T: ?Sized> Unpin for AlwaysUnpin<T> {}                           
    |                     ^^^^^                                                 
                                                                                
error: aborting due to 399 previous errors                                      
                                                                                
For more information about this error, try `rustc --explain E0658`.
Comment 13 Jacob Lifshay 2020-07-24 21:07:20 BST
(In reply to Luke Kenneth Casson Leighton from comment #11)
> this is not inspiring me with confidence...
> 
> lkcl@fizzy:~/src/libresoc/power-instruction-analyzer$ cargo install maturin
>     Updating crates.io index
>   Installing maturin v0.8.2                                                 
> 
> error: failed to compile `maturin v0.8.2`, intermediate artifacts can be
> found at `/tmp/cargo-installWSSMHV`
> 
> Caused by:
>   failed to parse lock file at:
> /home/lkcl/.cargo/registry/src/github.com-1ecc6299db9ec823/maturin-0.8.2/
> Cargo.lock

your version of rust is most likely too old -- if you used rustup to install it, use `rustup update` to update it.
Comment 14 Jacob Lifshay 2020-07-24 21:07:49 BST
related: https://github.com/PyO3/maturin/issues/333
Comment 15 Cole Poirier 2020-07-24 21:22:00 BST
Jacob can you update the README to this:

Program to analyze the behavior of Power ISA instructions

To build and install:

```bash
rustup default stable
rustup update
git clone https://salsa.debian.org/Kazan-team/power-instruction-analyzer.git pia
cd pia
maturin build --cargo-extra-args=--features=python-extension
python3 -m pip install target/wheels/*.whl
```

Script is available here: https://git.libre-soc.org/?p=dev-env-setup.git;a=blob_plain;f=pia-install;hb=HEAD

Will close this after I update libreriscv wiki
Comment 16 Cole Poirier 2020-07-24 21:47:27 BST
(In reply to Cole Poirier from comment #15)
> Jacob can you update the README to this:
> 
> Program to analyze the behavior of Power ISA instructions
> 
> To build and install:
> 
> ```bash
> rustup default stable
> rustup update

Forgot to add `cargo install maturin`


> git clone https://salsa.debian.org/Kazan-team/power-instruction-analyzer.git
> pia
> cd pia
> maturin build --cargo-extra-args=--features=python-extension
> python3 -m pip install target/wheels/*.whl
> ```

Correct up-to-date README should read:

Program to analyze the behavior of Power ISA instructions

To build and install:

```bash
rustup default stable
rustup update
cargo install maturin
git clone https://salsa.debian.org/Kazan-team/power-instruction-analyzer.git pia
cd pia
maturin build --cargo-extra-args=--features=python-extension
python3 -m pip install target/wheels/*.whl
```

Updated HDL_workflow to include the above. Closing.
Comment 17 Jacob Lifshay 2020-07-24 21:49:26 BST
(In reply to Cole Poirier from comment #15)
> Jacob can you update the README to this:

will do, or you can make a merge request on salsa
> 
> Program to analyze the behavior of Power ISA instructions
> 
> Script is available here:
> https://git.libre-soc.org/?p=dev-env-setup.git;a=blob_plain;f=pia-install;
> hb=HEAD
> 

That script is broken -- not everyone will have uid 1000, normally you would use $USER or whoami to get the username instead of the mess you have.
Comment 18 Jacob Lifshay 2020-07-24 21:51:59 BST
reassigning to self
Comment 19 Cole Poirier 2020-07-24 22:03:00 BST
(In reply to Jacob Lifshay from comment #17)
> (In reply to Cole Poirier from comment #15)
> > Jacob can you update the README to this:
> 
> will do, or you can make a merge request on salsa
> > 
> > Program to analyze the behavior of Power ISA instructions
> > 
> > Script is available here:
> > https://git.libre-soc.org/?p=dev-env-setup.git;a=blob_plain;f=pia-install;
> > hb=HEAD
> > 
> 
> That script is broken -- not everyone will have uid 1000, normally you would
> use $USER or whoami to get the username instead of the mess you have.

In both of those cases you will get the answer root... from examining the output of env it appears that we want $SUDO_USER
Comment 20 Jacob Lifshay 2020-07-24 22:12:07 BST
(In reply to Cole Poirier from comment #19)
> (In reply to Jacob Lifshay from comment #17)
> > That script is broken -- not everyone will have uid 1000, normally you would
> > use $USER or whoami to get the username instead of the mess you have.
> 
> In both of those cases you will get the answer root... from examining the
> output of env it appears that we want $SUDO_USER

If you need sudo to install python packages, your probably doing something wrong -- try using:
python3 -m pip install --user ...
or use pip in a virtualenv
Comment 21 Cole Poirier 2020-07-24 22:16:42 BST
(In reply to Cole Poirier from comment #19)
> (In reply to Jacob Lifshay from comment #17)
> > (In reply to Cole Poirier from comment #15)
> > > Jacob can you update the README to this:
> > 
> > will do, or you can make a merge request on salsa
> > > 
> > > Program to analyze the behavior of Power ISA instructions
> > > 
> > > Script is available here:
> > > https://git.libre-soc.org/?p=dev-env-setup.git;a=blob_plain;f=pia-install;
> > > hb=HEAD
> > > 
> > 
> > That script is broken -- not everyone will have uid 1000, normally you would
> > use $USER or whoami to get the username instead of the mess you have.
> 
> In both of those cases you will get the answer root... from examining the
> output of env it appears that we want $SUDO_USER

I'm doing a global find and replace for $MYNAME with $SUDO_USER and for $CHU_HOME with /home/$SUDO_USER.

Testing this now, then will push these changes.
Comment 22 Cole Poirier 2020-07-24 22:17:43 BST
(In reply to Jacob Lifshay from comment #20)

> If you need sudo to install python packages, your probably doing something
> wrong -- try using:
> python3 -m pip install --user ...
> or use pip in a virtualenv

Ah gotcha, was stuck in the setup.py develop mindset where you do have to use sudo. Will make this change.
Comment 23 Luke Kenneth Casson Leighton 2020-07-24 22:18:38 BST
(In reply to Cole Poirier from comment #66)

> 
> Luke, you should be able to get power_instruction_analyzer setup and
> installed properly by following https://libre-soc.org/HDL_workflow/ section
> 6.7 power_instruction_analyzer (pia)
> 
> Please let me know if you encounter bugs.

after updating rust

maturin build with python enabled worked however installing with pip did not


  ERROR: Command errored out with exit status 1:
     command: /usr/bin/python3 /tmp/tmpfh87qc0d prepare_metadata_for_build_w
eel /tmp/tmpcp76p4cf
         cwd: /tmp/pip-req-build-g70vqb94
    Complete output (8 lines):
     maturin failed
      Caused by: The given list of python interpreters is invalid
      Caused by: python is not a valid python interpreter
      Caused by: Failed to get information from the python interpreter at py
hon
      Caused by: Only python >= 3.5 is supported, while your using python 2.
    Checking for Rust toolchain....
    Running `maturin pep517 write-dist-info --metadata-directory /tmp/pip-mo
ern-metadata-j261zc3k --bindings=pyo3 --cargo-extra-args=--features python-e
tension`
    Error: Command '['maturin', 'pep517', 'write-dist-info', '--metadata-dir
ctory', '/tmp/pip-modern-metadata-j261zc3k', '--bindings=pyo3', '--cargo-ext
a-args=--features python-extension']' returned non-zero exit status 1.
    ----------------------------------------
ERROR: Command errored out with exit status 1: /usr/bin/python3 /tmp/tmpfh87
c0d prepare_metadata_for_build_wheel /tmp/tmpcp76p4cf Check the logs for ful
 command output.
Comment 24 Jacob Lifshay 2020-07-24 22:22:37 BST
(In reply to Luke Kenneth Casson Leighton from comment #23)
> (In reply to Cole Poirier from comment #66)
> 
> > 
> > Luke, you should be able to get power_instruction_analyzer setup and
> > installed properly by following https://libre-soc.org/HDL_workflow/ section
> > 6.7 power_instruction_analyzer (pia)
> > 
> > Please let me know if you encounter bugs.
> 
> after updating rust
> 
> maturin build with python enabled worked however installing with pip did not
> 
> 
>       Caused by: The given list of python interpreters is invalid
>       Caused by: python is not a valid python interpreter
>       Caused by: Failed to get information from the python interpreter at py
> hon
>       Caused by: Only python >= 3.5 is supported, while your using python 2.

that is the same as https://github.com/PyO3/maturin/issues/333

as a workaround, build in a python3 virtualenv
Comment 25 Luke Kenneth Casson Leighton 2020-07-24 22:43:16 BST
(In reply to Jacob Lifshay from comment #24)

> >       Caused by: Only python >= 3.5 is supported, while your using python 2.
> 
> that is the same as https://github.com/PyO3/maturin/issues/333
> 
> as a workaround, build in a python3 virtualenv


right.  i know what they've done.

first: they cannot spell the word "you're" ( the relative pronoun "your" is not a substitute for the shortening of the phrase "you are")

second: they have assumed that /usr/bin/python is the absolute default link to /usr/bin/python3.

if they intended to use python3 they should use "python3" as the command.

the virtualenv workaround most likely works because it adds a shell envvar which maturin notices and picks up (it is most probably $PYTHON or perhaps $PYTHON_PATH)

tried $PYTHON... nope.
tried $PYTHON_PATH...  nope

it'll be something like that.

i am not using virtualenv, it would be necessary to do a full total reinstall of all python packages for libresoc under the same virtualenv, just for a workaround of this one broken program (maturin).
Comment 26 Jacob Lifshay 2020-07-24 22:45:56 BST
(In reply to Luke Kenneth Casson Leighton from comment #25)
> (In reply to Jacob Lifshay from comment #24)
> 
> > >       Caused by: Only python >= 3.5 is supported, while your using python 2.
> > 
> > that is the same as https://github.com/PyO3/maturin/issues/333
> > 
> > as a workaround, build in a python3 virtualenv
> 
> 
> right.  i know what they've done.
> 
> first: they cannot spell the word "you're" ( the relative pronoun "your" is
> not a substitute for the shortening of the phrase "you are")
> 
> second: they have assumed that /usr/bin/python is the absolute default link
> to /usr/bin/python3.
> 
> if they intended to use python3 they should use "python3" as the command.
> 
> the virtualenv workaround most likely works because it adds a shell envvar
> which maturin notices and picks up (it is most probably $PYTHON or perhaps
> $PYTHON_PATH)
> 
> tried $PYTHON... nope.
> tried $PYTHON_PATH...  nope
> 
> it'll be something like that.
> 
> i am not using virtualenv, it would be necessary to do a full total
> reinstall of all python packages for libresoc under the same virtualenv,
> just for a workaround of this one broken program (maturin).

how about:

in virtualenv:
maturin build ...
outside virtualenv:
pip3 install target/.../*.whl
Comment 27 Cole Poirier 2020-07-24 22:49:24 BST
Script no longer uses sudo, wiki docs are up to date. Leaving this to you two to sort out the maturin issue in a way that works for Luke.
Comment 28 Luke Kenneth Casson Leighton 2020-07-24 22:54:30 BST
(In reply to Luke Kenneth Casson Leighton from comment #25)
> (In reply to Jacob Lifshay from comment #24)
> 
> > >       Caused by: Only python >= 3.5 is supported, while your using python 2.
> > 
> > that is the same as https://github.com/PyO3/maturin/issues/333
> > 
> > as a workaround, build in a python3 virtualenv
> 
> 
> right.  i know what they've done.
> 
> first: they cannot spell the word "you're" ( the relative pronoun "your" is
> not a substitute for the shortening of the phrase "you are")
> 
> second: they have assumed that /usr/bin/python is the absolute default link
> to /usr/bin/python3.
> 
> if they intended to use python3 they should use "python3" as the command.

the "usual" way this is done in python when executing other subprocesses as part of building is to examine sys.argv[0] which is of course the current executable then use *that* as the outgoing executable.

other tricks involve detecting the libraries and include headers etc. all of which setuptools and distutils covers extremely well.

setup-tools has had 20 years development behind it, and the people behind it knew what they were doing.

it basically sounds to me like maturin is attempting to reinvent things and not really being mature enough to cover the 20 years worth of corner cases that setuptools accumulated.
Comment 29 Jacob Lifshay 2020-07-24 23:05:34 BST
(In reply to Luke Kenneth Casson Leighton from comment #28)
> (In reply to Luke Kenneth Casson Leighton from comment #25)
> > if they intended to use python3 they should use "python3" as the command.
> 
> the "usual" way this is done in python when executing other subprocesses as
> part of building is to examine sys.argv[0] which is of course the current
> executable then use *that* as the outgoing executable.

that works -- except maturin doesn't get told what python's argv[0] is -- the error message for `pip install .` showed the entire list of arguments passed to maturin:
['maturin', 'pep517', 'write-dist-info', '--metadata-directory', '/tmp/pip-modern-metadata-60u_5apa', '--bindings=pyo3', '--cargo-extra-args=--features python-extension']
Comment 30 Luke Kenneth Casson Leighton 2020-07-24 23:10:56 BST
ok by manually and explicitly installing only the target/wheel/......cp37... whl file using pip3 there were no "complaints" from maturin.

i suspect that there is something wrong in the .tgz file that is also created in tbe target/wheels directory.

part of the problem is that maturin should not have "detected" the different versions installed on my system (3.5, 3.6, 3.7, 3.8).

this is likely how it tried to pick up /usr/bin/python as well which symlinks to 2.7

if i run "/usr/bin/python3.7" i expect *only* that to be built and installed.

maturin *may* have options that allow only the *specific* listed versions as part of maturin build args to be built.

it's trying to be "too clever" in other words (and getting it wrong))
Comment 31 Luke Kenneth Casson Leighton 2020-07-24 23:13:27 BST
(In reply to Jacob Lifshay from comment #29)

> '--cargo-extra-args=--features python-extension']

that there is no option to select python3.7 (or any other version) is i believe the source of the problem.

it should be
--features python-extension=3.7,3.8 (or other format)
Comment 32 Jacob Lifshay 2020-07-24 23:22:59 BST
(In reply to Luke Kenneth Casson Leighton from comment #31)
> (In reply to Jacob Lifshay from comment #29)
> 
> > '--cargo-extra-args=--features python-extension']
> 
> that there is no option to select python3.7 (or any other version) is i
> believe the source of the problem.
> 
> it should be
> --features python-extension=3.7,3.8 (or other format)

those are Rust features, not python features.

If you want to explicitly select an interpreter when using maturin, use:
maturin build -i python3 --cargo-...
I haven't tested that though.

The .tar.gz file created in target/ is the sdist. installing it will try to build power-instruction-analyzer all over again, rather than using the wheels that you just built.
Comment 33 Jacob Lifshay 2020-07-25 03:47:20 BST
(In reply to Luke Kenneth Casson Leighton from comment #25)
> (In reply to Jacob Lifshay from comment #24)
> 
> > >       Caused by: Only python >= 3.5 is supported, while your using python 2.
> 
> first: they cannot spell the word "you're" ( the relative pronoun "your" is
> not a substitute for the shortening of the phrase "you are")

grammar fix PR: https://github.com/PyO3/maturin/pull/334
Comment 35 Luke Kenneth Casson Leighton 2020-07-25 13:30:59 BST
fantastic, i think it's safe to put back the use of pia in div... wait,
have to think through how that impacts the compunits, first.
Comment 36 Jacob Lifshay 2020-07-29 19:43:16 BST
(In reply to Luke Kenneth Casson Leighton from comment #30)
> ok by manually and explicitly installing only the target/wheel/......cp37...
> whl file using pip3 there were no "complaints" from maturin.
> 
> i suspect that there is something wrong in the .tgz file that is also
> created in tbe target/wheels directory.
> 
> part of the problem is that maturin should not have "detected" the different
> versions installed on my system (3.5, 3.6, 3.7, 3.8).
> 
> this is likely how it tried to pick up /usr/bin/python as well which
> symlinks to 2.7
> 
> if i run "/usr/bin/python3.7" i expect *only* that to be built and installed.
> 
> maturin *may* have options that allow only the *specific* listed versions as
> part of maturin build args to be built.
> 
> it's trying to be "too clever" in other words (and getting it wrong))

Fix in progress: https://github.com/PyO3/maturin/pull/342
Comment 37 Luke Kenneth Casson Leighton 2020-07-29 20:13:01 BST
(In reply to Jacob Lifshay from comment #36)

> Fix in progress: https://github.com/PyO3/maturin/pull/342

nice! i'm quite impressed that thry listen and react so quickly.