Add linien-server package #36

Merged
sb10q merged 1 commits from fsagbuya/nix-servo:linien-server into master 2024-03-06 11:58:17 +08:00
Collaborator

Description

  • Build and add the linien-server package
    • Fix error when running linien-server in qemu -> other PR
-bash-5.2# linien-server
/nix/store/4h23mf9gv9zx7wv85zwx5wb6xwd81zil-python3.11-linien-server-1.0.1/bin/.linien-server-wrapped: /run/current-system/sw/bin/linien-server: line 3: syntax error near unexpected token `lambda'
### Description - [x] Build and add the `linien-server` package - Fix error when running `linien-server` in qemu -> other PR ``` -bash-5.2# linien-server /nix/store/4h23mf9gv9zx7wv85zwx5wb6xwd81zil-python3.11-linien-server-1.0.1/bin/.linien-server-wrapped: /run/current-system/sw/bin/linien-server: line 3: syntax error near unexpected token `lambda' ```
sb10q reviewed 2024-02-21 17:43:02 +08:00
flake.nix Outdated
@ -20,2 +19,4 @@
./pr-29.patch
./pr-30.patch
./network.patch
./linien-server.patch
Owner

If you need a patch to add a package to the SD image then something is wrong. Add a parameter to the image builder or something.

If you need a patch to add a package to the SD image then something is wrong. Add a parameter to the image builder or something.
Owner

Also the packages should be exported by the flake. You should not need to build the image to access them.

Also the packages should be exported by the flake. You should not need to build the image to access them.
sb10q reviewed 2024-02-21 17:45:55 +08:00
flake.nix Outdated
@ -175,6 +176,7 @@
url = "https://download.qemu.org/qemu-${version}.tar.xz";
hash = "sha256-Q8wXaAQQVYb3T5A5jzTp+FeH3/QA07ZA2B93efviZbs=";
};
patches = builtins.filter (x: (x.name or "") != "9d5b42beb6978dc6219d5dc029c9d453c6b8d503.diff") oldAttrs.patches;
Owner

What?

What?
Author
Collaborator

That patch is causing error due to nix flake update, need to remove.

That patch is causing error due to nix flake update, need to remove.
Owner

Add a code comment and explain in detail what is going on.
Do we still need to use the old qemu version anyway?

Add a code comment and explain in detail what is going on. Do we still need to use the old qemu version anyway?
Author
Collaborator

qemu 8.2.0 does not build before, however I just checked that they updated to 8.2.1. Will check if the issue has been fixed, otherwise will just add code comments here.

qemu `8.2.0` does not build before, however I just checked that they updated to `8.2.1`. Will check if the issue has been fixed, otherwise will just add code comments here.
fsagbuya marked this conversation as resolved
sb10q reviewed 2024-02-21 17:48:48 +08:00
@ -0,0 +7,4 @@
});
customKernelPackages = crosspkgs.linuxPackagesFor customKernel;
+
+ pyrp3 = pkgs.python3Packages.buildPythonPackage rec {
Owner

You need to use the linien fork and it also does not use myhdl.

You need to use the linien fork and it also does not use myhdl.
Author
Collaborator

Should we also use the elhep linien fork to build the gateware?

Should we also use the elhep linien fork to build the gateware?
Owner

Obviously yes, original linien is for red pitaya.

Obviously yes, original linien is for red pitaya.
Owner

Or just apply elhep's fast-servo support patches against original linien here.

Or just apply elhep's fast-servo support patches against original linien here.
Author
Collaborator

That sounds good. Will just apply the patch then and build the gateware from that.

That sounds good. Will just apply the patch then and build the gateware from that.
fsagbuya marked this conversation as resolved
fsagbuya changed title from Add the linien-server package to WIP: Add the linien-server package 2024-02-21 18:01:54 +08:00
fsagbuya force-pushed linien-server from b672a095c3 to 1a3a750cea 2024-02-23 16:40:01 +08:00 Compare
Owner

Just copy the migen and misoc derivations. ARTIQ is a large package, no need to bloat this one with a dependency when you're only using 20 lines of code from it.

Just copy the migen and misoc derivations. ARTIQ is a large package, no need to bloat this one with a dependency when you're only using 20 lines of code from it.
fsagbuya force-pushed linien-server from 1a3a750cea to f5bf776510 2024-02-23 17:01:58 +08:00 Compare
fsagbuya changed title from WIP: Add the linien-server package to Add support for fast-servo linien port 2024-02-23 17:02:03 +08:00
Owner

From reading the file name, it is unclear to what "add-source.patch" applies to.

From reading the file name, it is unclear to what "add-source.patch" applies to.
sb10q reviewed 2024-02-23 17:03:56 +08:00
flake.nix Outdated
@ -19,3 +21,4 @@
./pr-28.patch
./pr-29.patch
./pr-30.patch
./network.patch
Owner

?

?
fsagbuya marked this conversation as resolved
sb10q reviewed 2024-02-23 17:05:14 +08:00
flake.nix Outdated
@ -26,0 +62,4 @@
fontconfig
];
vivadoEnv = pkgs.buildFHSEnv {
Owner

not needed, remove

not needed, remove
fsagbuya marked this conversation as resolved
sb10q reviewed 2024-02-23 17:05:23 +08:00
flake.nix Outdated
@ -26,0 +69,4 @@
vivado = pkgs.buildFHSEnv {
name = "vivado";
targetPkgs = vivadoDeps;
Owner

inline

inline
fsagbuya marked this conversation as resolved
sb10q reviewed 2024-02-23 17:06:06 +08:00
flake.nix Outdated
@ -26,0 +91,4 @@
]);
postPatch = ''
substituteInPlace setup.py \
--replace "\"pyrp3>=1.1.0,<2.0;platform_machine=='armv7l'\"," ""
Owner

As far as I can tell this is a required dependency. Removing it does not solve the problem.

As far as I can tell this is a required dependency. Removing it does not solve the problem.
fsagbuya marked this conversation as resolved
sb10q reviewed 2024-02-23 17:06:24 +08:00
flake.nix Outdated
@ -175,9 +265,33 @@
url = "https://download.qemu.org/qemu-${version}.tar.xz";
hash = "sha256-Q8wXaAQQVYb3T5A5jzTp+FeH3/QA07ZA2B93efviZbs=";
};
# temporary fix to build qemu with recent nixpkgs
Owner

Did you confirm that you can't just use the latest QEMU?

Did you confirm that you can't just use the latest QEMU?
Author
Collaborator

Latest qemu can be use now, however some packages break with the update. Specifically the kernel. Will solve this in the next PR

Latest qemu can be use now, however some packages break with the update. Specifically the kernel. Will solve this in the next PR
fsagbuya marked this conversation as resolved
sb10q reviewed 2024-02-23 17:06:53 +08:00
flake.nix Outdated
@ -178,3 +270,4 @@
});
board-package-set = { board }: let
# gateware only builds fast-servo for now
Owner

"for now"?
What else? We're not going to run linien on zc706.

"for now"? What else? We're not going to run linien on zc706.
Author
Collaborator

Will update the script to be only exclusive for fast-servo.

Will update the script to be only exclusive for fast-servo.
fsagbuya marked this conversation as resolved
Owner

Split gateware and linien-server into two PRs.

Split gateware and linien-server into two PRs.
sb10q reviewed 2024-02-23 17:18:29 +08:00
@ -0,0 +26,4 @@
- cmd = f"bootgen -image {build_name}.bif -arch zynq -process_bitstream bin -w on".split(" ")
- subprocess.run(cmd)
+ # cmd = f"bootgen -image {build_name}.bif -arch zynq -process_bitstream bin -w on".split(" ")
Owner

Doesn't make sense to comment out lines in a patch. Just remove them.

Doesn't make sense to comment out lines in a patch. Just remove them.
Owner

(Unless the justification is you want the code to be exactly like elhep/linien upstream)

(Unless the justification is you want the code to be exactly like elhep/linien upstream)
fsagbuya marked this conversation as resolved
fsagbuya force-pushed linien-server from f5bf776510 to 89becc9873 2024-02-23 17:58:55 +08:00 Compare
fsagbuya changed title from Add support for fast-servo linien port to Add linien-server package 2024-02-23 17:59:26 +08:00
sb10q reviewed 2024-02-23 21:14:20 +08:00
flake.lock Outdated
@ -23,3 +23,3 @@
"owner": "NixOS",
"repo": "nixpkgs",
"rev": "b0d36bd0a420ecee3bc916c91886caca87c894e9",
"rev": "6fd7935607b68a96949b51ff08f12109e99ffd1f",
Owner

Why is this update part of this PR?

Why is this update part of this PR?
Author
Collaborator

Updated to fetch the fix for scipy dependency build and also to fetch the pinned rpyc4 version.

Updated to fetch the fix for `scipy` dependency build and also to fetch the pinned [rpyc4 version](https://github.com/NixOS/nixpkgs/commit/6fd7935607b68a96949b51ff08f12109e99ffd1f).
fsagbuya marked this conversation as resolved
sb10q reviewed 2024-02-23 21:14:35 +08:00
flake.nix Outdated
@ -25,0 +26,4 @@
pname = "pyrp3";
version = "1.2.0";
pyproject = true;
src = pkgs.python3Packages.fetchPypi {
Owner

I already told you that you need to use the linien fork.

I already told you that you need to use the linien fork.
fsagbuya marked this conversation as resolved
fsagbuya force-pushed linien-server from 89becc9873 to 568500f0e1 2024-02-26 11:28:22 +08:00 Compare
fsagbuya force-pushed linien-server from 568500f0e1 to f84ae926ca 2024-03-01 16:49:04 +08:00 Compare
sb10q reviewed 2024-03-01 16:50:04 +08:00
flake.nix Outdated
@ -73,2 +110,4 @@
extraModules = [
"${patched-not-os}/zynq_image.nix"
({ config, pkgs, ... }: {
environment.systemPackages = [ linien-server ];
Owner

We only want it on fast-servo. It won't work on zc706.

We only want it on fast-servo. It won't work on zc706.
Owner

Maybe don't do the not-os integration for now? It's not done for the gateware anyway. Could be two separate PRs.

Maybe don't do the not-os integration for now? It's not done for the gateware anyway. Could be two separate PRs.
fsagbuya marked this conversation as resolved
fsagbuya changed title from Add linien-server package to WIP: Add linien-server package 2024-03-01 17:04:55 +08:00
sb10q reviewed 2024-03-01 17:25:14 +08:00
flake.nix Outdated
@ -71,0 +80,4 @@
};
nativeBuildInputs = with pkgs.python3Packages; [ setuptools wheel setuptools-scm ];
propagatedBuildInputs = with pkgs.python3Packages; [
myhdl
Owner

Looks wrong.

Looks wrong.
Author
Collaborator

It seems the myhdl dependency has not been dropped in any version in the linien fork.

It seems the [myhdl dependency](https://github.com/linien-org/pyrp3/blob/v1.2.0/setup.py#L30) has not been dropped in any version in the linien fork.
Owner

Ah okay. It's used only for intbv in a few places (which could be replaced with simple bitwise arithmetic) and also in the original (non-linien) one. Looks like physicist code syndrome strikes again.

Ah okay. It's used only for intbv in a few places (which could be replaced with simple bitwise arithmetic) and also in the original (non-linien) one. Looks like physicist code syndrome strikes again.
Author
Collaborator

Will implement as suggested.

Will implement as suggested.
fsagbuya force-pushed linien-server from f84ae926ca to 9a5924ee81 2024-03-05 17:33:42 +08:00 Compare
fsagbuya changed title from WIP: Add linien-server package to Add linien-server package 2024-03-05 17:34:00 +08:00
Owner

Submit the bitwise patch upstream and keep myhdl here for now, one thing at a time.

Submit the bitwise patch upstream and keep myhdl here for now, one thing at a time.
fsagbuya force-pushed linien-server from 9a5924ee81 to 2f729643e1 2024-03-06 11:23:33 +08:00 Compare
Author
Collaborator

Submit the bitwise patch upstream and keep myhdl here for now, one thing at a time.

Submitted a PR upstream https://github.com/linien-org/pyrp3/pull/12.

> Submit the bitwise patch upstream and keep myhdl here for now, one thing at a time. Submitted a PR upstream https://github.com/linien-org/pyrp3/pull/12.
sb10q reviewed 2024-03-06 11:32:23 +08:00
flake.nix Outdated
@ -419,3 +456,3 @@
IMGDIR=$(mktemp -d /tmp/not-os-qemu-XXXXXX)
BASE=$(realpath $(dirname $0))
qemu-img create -F raw -f qcow2 -b $BASE/sd-image.img $IMGDIR/sd-overlay.qcow2 512M
qemu-img create -F raw -f qcow2 -b $BASE/sd-image.img $IMGDIR/sd-overlay.qcow2 2G
Owner

Since we are not including linien-server in the SD image yet, I think this should not be in this PR?

Since we are not including linien-server in the SD image yet, I think this should not be in this PR?
Author
Collaborator

Ah yes, thanks for noticing. Will omit this part for this PR.

Ah yes, thanks for noticing. Will omit this part for this PR.
Author
Collaborator

Hmm, the new sd-image now after the latest update from linux 6.1 to linux 6.6 have increased in size.

total 534M
 4.0K -r-xr-xr-x 3 root root   718 Jan  1  1970 qemu-script
 533M -r--r--r-- 2 root root  533M Jan  1  1970 sd-image.img
1016K -r-xr-xr-x 2 root root 1014K Jan  1  1970 u-boot.elf

Previously it is around 391M https://nixbld.m-labs.hk/build/157718. Will check for what have changed and resolve this on the next PR.

Hmm, the new sd-image now after the latest update from `linux 6.1` to `linux 6.6` have increased in size. ``` total 534M 4.0K -r-xr-xr-x 3 root root 718 Jan 1 1970 qemu-script 533M -r--r--r-- 2 root root 533M Jan 1 1970 sd-image.img 1016K -r-xr-xr-x 2 root root 1014K Jan 1 1970 u-boot.elf ``` Previously it is around `391M` https://nixbld.m-labs.hk/build/157718. Will check for what have changed and resolve this on the next PR.
fsagbuya force-pushed linien-server from 2f729643e1 to 3618a1f17d 2024-03-06 11:48:21 +08:00 Compare
sb10q merged commit 3618a1f17d into master 2024-03-06 11:58:17 +08:00
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: M-Labs/nix-servo#36
No description provided.