search

ngi-nix / ngipkgs

Nix packages and services for projects supported through the NGI program
https://ngi-nix.github.io/ngipkgs
MIT License
35 stars 18 forks source link

openXC7 #7

Closed fricklerhandwerk closed 1 year ago

fricklerhandwerk commented 1 year ago

Free and open source FPGA toolchain for AMD/Xilinx Series 7 chips

This seems to be a non-trivial project with multiple dependencies. Break down into separate issues as needed.

jleightcap commented 1 year ago

@ngi-nix/moss is claiming this!

hansfbaier commented 1 year ago

Thanks for the devshell! I still have an issue. If I try to load the yosys-ghdl plugin like this:

$ yosys
yosys> plugin -i /nix/store/wfhih024a3hcri75dzbhphw622ra3p0w-yosys-ghdl-2022.01.11/share/yosys/plugins/ghdl.so
ERROR: Can't load module `/nix/store/wfhih024a3hcri75dzbhphw622ra3p0w-yosys-ghdl-2022.01.11/share/yosys/plugins/ghdl.so': libgnat-10.so: cannot open shared object file: No such file or directory

Then it misses the mentioned library. How can you add shared library dependencies to the devshell? Also, how can the user get the path to the plugin to load (eg in a Makefile), so that the nix store location does not need to be hardcoded in the Makefile?

albertchae commented 1 year ago

@hansfbaier thanks for the report. We'll look into fixing that and also your question about the path for the Makefile during our next mob session on Thursday

jleightcap commented 1 year ago

Yeah, seconding the thanks for the update. I wanted to look at using the upstream yosys plugins during our next mob (see: https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/compilers/yosys/plugins/ghdl.nix)

Without having looked closely at the locally defined builds/versions, was there a reason for packaging separately outside of upstream?

hansfbaier commented 1 year ago

I nead a special yosys version (0.17) as later versions trigger a non implemented feature in nextpnr-xilinx.

jleightcap commented 1 year ago

Thanks for the info, I see how you selectively pulled definitions in from upstream at the correct version. We ended up meeting for shorter than expected today, hoping to have more of an update when we meet this Saturday 🙂

albertchae commented 1 year ago

@hansfbaier how are you starting running the dev shell? We tried the following on 2 different machines and were able to load the yosys-ghdl plugin successfully

[~/toolchain-nix]$ nix develop
[nix(openXC7)] yosys

 /----------------------------------------------------------------------------\
 |                                                                            |
 |  yosys -- Yosys Open SYnthesis Suite                                       |
 |                                                                            |
 |  Copyright (C) 2012 - 2020  Claire Xenia Wolf <claire@yosyshq.com>         |
 |                                                                            |
 |  Permission to use, copy, modify, and/or distribute this software for any  |
 |  purpose with or without fee is hereby granted, provided that the above    |
 |  copyright notice and this permission notice appear in all copies.         |
 |                                                                            |
 |  THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES  |
 |  WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF          |
 |  MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR   |
 |  ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES    |
 |  WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN     |
 |  ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF   |
 |  OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.            |
 |                                                                            |
 \----------------------------------------------------------------------------/

 Yosys 0.17 (git sha1 yosys-0.17, gcc 11.3.0 -fPIC -Os)

yosys> plugin -i /nix/store/wfhih024a3hcri75dzbhphw622ra3p0w-yosys-ghdl-2022.01.11/share/yosys/plugins/ghdl.so

yosys>
hansfbaier commented 1 year ago

Yes, I also used nix develop, but on Ubuntu, not NixOS. Interesting, that is actually the main issue that nix set out to solve. I wonder why it fails on my machine.

albertchae commented 1 year ago

We'll try debugging it on ubuntu during our next session

hansfbaier commented 1 year ago

DISTRIB_ID=Ubuntu DISTRIB_RELEASE=22.04 DISTRIB_CODENAME=jammy DISTRIB_DESCRIPTION="Ubuntu 22.04.2 LTS"

hansfbaier commented 1 year ago

I found this with strace:

openat(AT_FDCWD, "/nix/store/2j8jqmnd9l7plihhf713yf291c9vyqjm-glibc-2.35-224/lib/libgnat-10.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)

I looked into the directory. I tlooks ok, except that libgnat does not exist there:

$ ls /nix/store/2j8jqmnd9l7plihhf713yf291c9vyqjm-glibc-2.35-224/lib
Mcrt1.o  crti.o   grcrt1.o              libanl.so    libc_malloc_debug.so    libdl.so.2     libm.so.6       libnsl.so.1         libnss_db.so.2     libnss_hesiod.so.2  libresolv.so    libthread_db.so    locale
Scrt1.o  crtn.o   ld-linux-x86-64.so.2  libanl.so.1  libc_malloc_debug.so.0  libgcc_s.so    libmemusage.so  libnss_compat.so    libnss_dns.so.2    libpcprofile.so     libresolv.so.2  libthread_db.so.1  rcrt1.o
audit    gconv    libBrokenLocale.so    libc.so      libc_nonshared.a        libgcc_s.so.1  libmvec.so      libnss_compat.so.2  libnss_files.so.2  libpthread.so       librt.so        libutil.so
crt1.o   gcrt1.o  libBrokenLocale.so.1  libc.so.6    libdl.so                libm.so        libmvec.so.1    libnss_db.so        libnss_hesiod.so   libpthread.so.0     librt.so.1      libutil.so.1
hansfbaier commented 1 year ago

If I do

sudo /nix/var/nix/profiles/default/bin/nix --extra-experimental-features nix-command store repair /nix/store/2j8jqmnd9l7plihhf713yf291c9vyqjm-glibc-2.35-224

it seems to reinstall the package, but libgnat is still not there.

albertchae commented 1 year ago

@hansfbaier we tried a few different things but we are still having trouble reproducing your issue

Honestly, we're kind of at a loss at what to try next. Can you tell us your nix --version? We were able to get working dev shells on 2.15.1, 2.15.0 and 2.13. Also, we installed nix using the determinate systems installer.

hansfbaier commented 1 year ago

Thanks, sorry I can't do anything at the moment, my PC is defective and I have to figure out which part needs to be replaced.

hansfbaier commented 1 year ago

Hmm, got by PC up and running again. Removed flake.lock, then it downloaded and recompiled everything, but I still have the same issue.

hansfbaier commented 1 year ago

Ah, if I use the ghdl plugin from the same hash version as you, then it also works for me:

 Yosys 0.17 (git sha1 yosys-0.17, gcc 11.3.0 -fPIC -Os)

yosys> plugin -i /nix/store/wfhih024a3hcri75dzbhphw622ra3p0w-yosys-ghdl-2022.01.11/share/yosys/plugins/ghdl.so

yosys>

So the question is, how does the user find the right path to the ghdl plugin? Can that be automated somehow?

albertchae commented 1 year ago

@hansfbaier sorry to hear about your PC issues but I'm glad you have it running again. Could you tell us your nix version and also how you installed it?

If you're available to meet on video call sometime this week for a live debugging session, that might be next best option.

hansfbaier commented 1 year ago

@albertchae I just got it working, by deleting flake.lock and using your hash for the yosys plugin.

hansfbaier commented 1 year ago

@albertchae I have to add that the culprit is probably my active nix profile. When I tried a new user it works, but in my nix profile it does not.

hansfbaier commented 1 year ago 
$ nix profile list
0 flake:nixpkgs#legacyPackages.x86_64-linux.vscodium github:NixOS/nixpkgs/4a56ce9727a0c5478a836a0d8a8f641c5b9a3d5f#legacyPackages.x86_64-linux.vscodium /nix/store/m5m2z5hr4cdqdxir96p6620xx8mqqdnz-vscodium-1.80.2.23209
1 flake:nixpkgs#legacyPackages.x86_64-linux.librewolf github:NixOS/nixpkgs/0cb867999eec4085e1c9ca61c09b72261fa63bb4#legacyPackages.x86_64-linux.librewolf /nix/store/dxj6jzbd5ik01zpdqlw0lr4yppvvb9ik-librewolf-113.0-3
2 flake:nixpkgs#legacyPackages.x86_64-linux.github-desktop github:NixOS/nixpkgs/0cb867999eec4085e1c9ca61c09b72261fa63bb4#legacyPackages.x86_64-linux.github-desktop /nix/store/fwxqw86ba3r3yxxkhjypm30x5lrx6kdk-github-desktop-3.2.1
3 flake:nixpkgs#legacyPackages.x86_64-linux.helix github:NixOS/nixpkgs/3acb5c4264c490e7714d503c7166a3fde0c51324#legacyPackages.x86_64-linux.helix /nix/store/h24wygkxsrfwsyaxi35faxx3hfwdaqks-helix-23.05
4 flake:nixpkgs#legacyPackages.x86_64-linux.poetry github:NixOS/nixpkgs/0cb867999eec4085e1c9ca61c09b72261fa63bb4#legacyPackages.x86_64-linux.poetry /nix/store/plyy5cj3q364mzyjmczm95grsf8wpr6y-python3.10-poetry-1.4.2
5 flake:nixpkgs#legacyPackages.x86_64-linux.verilator github:NixOS/nixpkgs/0cb867999eec4085e1c9ca61c09b72261fa63bb4#legacyPackages.x86_64-linux.verilator /nix/store/my28m8asnxnrpj47d3hiznygsmaqv3rh-verilator-5.010
6 flake:nixpkgs#legacyPackages.x86_64-linux.lldb github:NixOS/nixpkgs/0cb867999eec4085e1c9ca61c09b72261fa63bb4#legacyPackages.x86_64-linux.lldb /nix/store/nc6534f8dbzf0m266yddwvngmvd5k2in-lldb-14.0.6
hansfbaier commented 1 year ago

@albertchae No, it was not the profile, I narrowed it down: I had LD_LIBRARY_PATH set to /usr/local/lib. As soon as I remove that environment variable, it starts working. So the remaining question is: how does the user discover the proper path of the ghdl plugin for his current yosys version?

hansfbaier commented 1 year ago

@albertchae Ah, I got that one too, the packages set the environment variable NIX_YOSYS_PLUGIN_DIRS which contains the right path. Many thanks!

hansfbaier commented 1 year ago

I still get this issue though:

 nix flake check
[...]
warning: flake output attribute 'overlay' is deprecated; use 'overlays.default' instead
error: Package ‘ghdl-mcode-2.0.0’ in /nix/store/qdmn9i1jly7b4glixk82ankridmha7ha-source/flake.nix:97 is not supported on ‘x86_64-darwin’, refusing to evaluate.

       a) To temporarily allow packages that are unsupported for this system, you can use an environment variable
          for a single invocation of the nix tools.

            $ export NIXPKGS_ALLOW_UNSUPPORTED_SYSTEM=1

        Note: For `nix shell`, `nix build`, `nix develop` or any other Nix 2.4+
        (Flake) command, `--impure` must be passed in order to read this
        environment variable.

       b) For `nixos-rebuild` you can set
         { nixpkgs.config.allowUnsupportedSystem = true; }
       in configuration.nix to override this.

       c) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
         { allowUnsupportedSystem = true; }
       to ~/.config/nixpkgs/config.nix.
(use '--show-trace' to show detailed location information)
hansfbaier commented 1 year ago

@albertchae I forgot, there are still three packages missing from the current nix flake:

albertchae commented 1 year ago

@hansfbaier we are looking into the 3 packages you said are missing. At least for prjxray, it looks like it is defined here https://github.com/openXC7/toolchain-nix/blob/ee91979e8f164b5775b2d951689c26a7dd46a872/flake.nix#L329-L364 and the 2 binaries xc7frames2bit and xc7patch are available in the dev shell. Is there something specific missing from this project in the flake?

We'll look into adding prjxray-db to the flake. What is expected here? Do we just want to make the files under artix7, kintex7, spartan7, and zynq7 available on the path?

We'll also look into getting someone to review your fasm PR for nixpkgs.

hansfbaier commented 1 year ago

@albertchae Yes, I overlooked prjxray, that is already done. Yes, the files under prjxray-db just need to be made available in the store, and we need an environment variable that points to it.

hansfbaier commented 1 year ago

@albertchae Wait, prjxray-db is already installed as part of nextpnr-xilinx. Then we just would need an environment variable pointing to it.

hansfbaier commented 1 year ago

@albertchae I will try the environment variable. fasm would be the priority for you, because I cannot do that.

hansfbaier commented 1 year ago

@albertchae OK, environment variable done. https://github.com/openXC7/toolchain-nix/commit/ea5955778e80c4b66a2dc28ce97b5e7f31cb99bf

hansfbaier commented 1 year ago

@albertchae I found a bug in the pypy3 package. https://github.com/NixOS/nixpkgs/issues/251126 I need it to generate the chip databases.

albertchae commented 1 year ago

@albertchae I will try the environment variable. fasm would be the priority for you, because I cannot do that.

This is for https://github.com/NixOS/nixpkgs/pull/224448 correct? We will try to ping people to review that for you, but I think it would be best to split it into 2 PRs (one to add yourself as a maintainer and the other to add fasm). The first commit should just be to add yourself as a maintainer (see similar feedback https://github.com/NixOS/nixpkgs/pull/249880#issuecomment-1683434380). And the second commit should have the message python310Packages.fasm: init at 0.0.2.post100. Can you link us those PRs after they are ready and we can try to ping the right people?

hansfbaier commented 1 year ago

@albertchae Moment please, will make two PRs.

hansfbaier commented 1 year ago

@albertchae OK done: https://github.com/NixOS/nixpkgs/pull/251333 I made a separate PR for the maintainer, but was told here to make one out of it: https://github.com/NixOS/nixpkgs/pull/224448

albertchae commented 1 year ago

@hansfbaier oh sorry about the conflicting messaging about separate PRs, but looks like someone is actively reviewing the PR now 🎉

hansfbaier commented 1 year ago

@albertchae I added a nix package with the pre built chip databases to the flake. Unfortunately I cannot see the environment variable from its setup hook in the devshell. What am I missing here? Also, I would like to call the builder with different arguments to create different flakes with the different FPGA families (artiz7, kintex7, spartan7 zynq7). What would be the best way to go about it? I also would like to make those optional. How would I want to do that?

jleightcap commented 1 year ago

Hey Hans, we are doing a holistic check in on the state of OpenXC7 as it related to the Summer of Nix program.

You've done the lion's share of the actual packaging in https://github.com/openXC7/toolchain-nix! We're happy to provide support, as it is related to the Summer of Nix program's goals, but besides the few PRs we contributed, we're unclear as to what remains to be packaged.

There's an opportunity to refactor some of the Nix code to be more upstreamable into nixpkgs, provided this refactoring aligns with the goals given to us for Summer of Nix (we have to verify this, any thoughts @fricklerhandwerk ?) we'd be happy to work on this.

Feel free to chat here, or we'd be happy to set up a short meeting if you'd like the chat over how Summer of Nix could best support further packaging.

hansfbaier commented 1 year ago

@jleightcap Yes an effort towards upstreaming would be greatly appreciated, because that is where I have no experience whatsoever with.

fricklerhandwerk commented 1 year ago

@jleightcap I suggest @ngi-nix/moss to do the refactoring as you'll have enough experience to do it swiftly, and then hand over to @hansfbaier to make a PR to Nixpkgs to become a maintainer.

Nixpkgs has a freshly revamped contributor guide that should hopefully be fairly easy to follow if fixups are needed in the future. @hansfbaier ping me personally if you find issues with those, and I'll make sure you get questions answered and the documentation improved accordingly.

hansfbaier commented 1 year ago

@jleightcap Some of the packages (yosys, nextpnr with ghdl plugins) I actually have copy pasted from upstream and modified to use the openXC7 repos.

jleightcap commented 1 year ago

@ngi-nix/moss began work today on refactoring each package into a form that is (ideally) upstreamable. See: https://github.com/openXC7/toolchain-nix/pull/4

hansfbaier commented 1 year ago

@jleightcap @ngi-nix/moss Great, thanks!

albertchae commented 1 year ago

@hansfbaier we decided to pull out formatting changes only to https://github.com/openXC7/toolchain-nix/pull/5 and then we had some additional questions for you in https://github.com/openXC7/toolchain-nix/pull/4#issuecomment-1728736343 that'll help us keep improving https://github.com/openXC7/toolchain-nix/pull/4

albertchae commented 1 year ago

@hansfbaier we wanted to check in since we have less than 2 weeks left for summer of nix. From our understanding of the scope of this issue, our goal was to package the entire toolchain for OpenXC7 using Nix within https://github.com/openXC7/toolchain-nix. It seemed like you already had most of that in place, but we helped you add a devShell in https://github.com/openXC7/toolchain-nix/pull/2 and then helped refactor the Nix code to be more idiomatic (and thus hopefully closer to upstreamable) with https://github.com/openXC7/toolchain-nix/pull/3 https://github.com/openXC7/toolchain-nix/pull/5 and https://github.com/openXC7/toolchain-nix/pull/4.

We believe the last missing package/tool in the toolchain was fasm which you called out in the README, so we have a PR to add this in https://github.com/openXC7/toolchain-nix/pull/6. Was there anything else you needed in the toolchain-nix repo as part of the OpenXC7 toolchain, or would you say that merging this PR would complete the scope of work under Summer of Nix? We will open a separate thread (probably as an issue in toolchain-nix repo) about continuing to try and upstream packages outside of Summer of Nix.

hansfbaier commented 1 year ago

@albertchae Thanks, for the fasm changes. When I use the fasm package, I still get a warning message that the antlr parser is not available, which degrades performance:

python3
Python 3.10.11 (main, Apr  4 2023, 22:10:32) [GCC 11.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import fasm
/nix/store/7aplvxiy7r1nl7fa2rlp2r681pihjh52-python3.10-fasm/lib/python3.10/site-packages/fasm/parser/__init__.py:30: RuntimeWarning: Unable to import fast Antlr4 parser implementation.
  ImportError: cannot import name tags

  Falling back to the much slower pure Python textX based parser
  implementation.

  Getting the faster antlr parser can normally be done by installing the
  required dependencies and then reinstalling the fasm package with:
    pip uninstall
    pip install -v fasm

  warn(
>>>

Does the fasm package require the antlr4 package as a dependency?

jleightcap commented 1 year ago

Using the ANTLR4 parser is currently quite difficult given the state of both nixpkgs and fasm. ANTLR4 is not required because the textx parser is available as a slower fallback.

If ANTRL4 support was useful for the toolchain, my general recommendation is to 'revive' the fasm repository somehow. I'd probably recommend forking the repository, as the current upstream seems abandoned; build system fixes exist on an umerged PR, for example.

We might be able to help with this, but I'd note this is likely outside of the Summer of Nix goals.

fricklerhandwerk commented 1 year ago

Does the fasm package require the antlr4 package as a dependency?

If the warning is really annoying, I'd suggest to patch out the offending lines so we can come to a conclusion here. Indeed we can't resolve an issue as deep as dealing with an unmaintained dependency.

hansfbaier commented 1 year ago

@albertchae @fricklerhandwerk point taken, yes I agree.

hansfbaier commented 1 year ago

@albertchae There is one item left, which puzzles me. Setting environment variables seems to work in prjxray-setup-hook.sh, but not in nextpnr-setup-hook.sh. I can see the variable from prjxray, but not from nextpnr in the devshell. Why might that be?

hansfbaier commented 1 year ago

I also have another question: When I want to get the path of a package, I could:

nix eval --raw ".#nextpnr-xilinx-chipdb.spartan7.outPath"

but that only works inside the flake directory. If I want to use this in a Makefile I would either have to provide the path to the flake in the local file system, or use the GitHub URL. The latter is independend of the CWD, but requires me to be online. Is there a way to get the path in a CWD independent manner offline? One would be the setup hook, but that does not seem to work for nextpnr-xilinx and the chip databases.

hansfbaier commented 1 year ago

Ah I found a way: image