Skip to content

Add teos package (The Eye of Satoshi) with clightning watchtower plugin - #195782

Merged
prusnak merged 2 commits into
NixOS:masterfrom
seberm:add-rust-teos
Oct 14, 2022
Merged

prusnak merged 2 commits into
NixOS:masterfrom
seberm:add-rust-teos

Conversation

@seberm

@seberm seberm commented Oct 13, 2022 •

Copy link
Copy Markdown
Contributor
Description of changes

I would like to add teos package (implementation of lightning watchtower) into nixpkgs:

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ofborg ofborg Bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Oct 13, 2022
@seberm seberm changed the title Add teos package (The Eye of Satoshi) witch clightning watchtower plugin Add teos package (The Eye of Satoshi) with clightning watchtower plugin Oct 13, 2022
@seberm
seberm marked this pull request as ready for review October 13, 2022 10:01
@prusnak

prusnak commented Oct 13, 2022

Copy link
Copy Markdown
Member

Don't we want to add teos-watchtower-plugin to all-packages.nix?

@seberm

seberm commented Oct 13, 2022 •

Copy link
Copy Markdown
Contributor Author

Hello Pavol,

Don't we want to add teos-watchtower-plugin to all-packages.nix?

Sure we want. I've made the changes. Thanks!

@ofborg ofborg Bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Oct 13, 2022
@prusnak

prusnak commented Oct 13, 2022 •

Copy link
Copy Markdown
Member

Build error on darwin:

ld: framework not found Security

Please add Security conditionally for darwin to buildInputs and add the following to all-packages.nix:

inherit (darwin.apple_sdk.frameworks) Security;

@prusnak

prusnak commented Oct 13, 2022 •

Copy link
Copy Markdown
Member

@seberm

seberm commented Oct 13, 2022

Copy link
Copy Markdown
Contributor Author

Hm, Darwin still fails :-/ https://logs.nix.ci/?key=nixos/nixpkgs.195782&attempt_id=6274d55b-bc05-4147-bd1b-872de81c7dc7

Hm, strange :/. Is it somehow possible to reproduce this issue on my (x86_64) machine so I can debug it more easily?

@prusnak

prusnak commented Oct 14, 2022 •

Copy link
Copy Markdown
Member

Tried this on aarch64-darwin and got lucky. My compilation output contained more info:

  cargo:warning=curl/lib/hostip.c:72:10: fatal error: 'SystemConfiguration/SCDynamicStoreCopySpecific.h' file not found
  cargo:warning=#include <SystemConfiguration/SCDynamicStoreCopySpecific.h>
  cargo:warning=         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  cargo:warning=1 error generated.
  exit status: 1

leading to the following fix:

diff --git a/pkgs/applications/blockchains/teos/default.nix b/pkgs/applications/blockchains/teos/default.nix
index c92b2ebfa34..0fbb968a66e 100644
--- a/pkgs/applications/blockchains/teos/default.nix
+++ b/pkgs/applications/blockchains/teos/default.nix
@@ -8,6 +8,7 @@
 , protobuf
 , rustfmt
 , Security
+, SystemConfiguration
 }:
 
 let
@@ -31,7 +32,7 @@ let
 
   buildInputs = [
     openssl
-  ] ++ lib.optionals stdenv.isDarwin [ Security ];
+  ] ++ lib.optionals stdenv.isDarwin [ Security SystemConfiguration ];
 
   nativeBuildInputs = [
     perl  # used by openssl-sys to configure
diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix
index ffbf0271ac6..e0edf27318e 100644
--- a/pkgs/top-level/all-packages.nix
+++ b/pkgs/top-level/all-packages.nix
@@ -33234,7 +33234,7 @@ with pkgs;
   taro = callPackage ../applications/blockchains/taro { };
 
   inherit (callPackages ../applications/blockchains/teos {
-    inherit (darwin.apple_sdk.frameworks) Security;
+    inherit (darwin.apple_sdk.frameworks) Security SystemConfiguration;
   })
     teos
     teos-watchtower-plugin;

So the build now works fine, however, the tests fail with:

...
test retrier::tests::test_manage_retry_unreachable ... FAILED
...
---- retrier::tests::test_manage_retry_unreachable stdout ----
thread 'retrier::tests::test_manage_retry_unreachable' panicked at 'assertion failed:
wt_client.lock().unwrap().towers.get(&tower_id).unwrap().status.is_unreachable()', watchtower-plugin/src/retrier.rs:518:9

Is there a way how to skip a single test in buildRustPackage?

If not, let's just skip the tests for aarch64-darwin with doCheck = !(stdenv.isDarwin && stdenv.isAarch64); (tests on x86_64-darwin pass)

In both cases of skipping the test, let's add the error output above as a comment to indicate why we skipped the tests.

@prusnak

prusnak commented Oct 14, 2022

Copy link
Copy Markdown
Member

@GrahamcOfBorg build teos teos-watchtower-plugin

@prusnak

prusnak commented Oct 14, 2022 •

Copy link
Copy Markdown
Member

Tested 2f4f8e6 on {aarch64,x86_64}-darwin locally - it builds and works. Let's wait for the CI, then I'll merge. 👍

@seberm

seberm commented Oct 14, 2022

Copy link
Copy Markdown
Contributor Author

Thank you for helping me with the testing!

I have contacted teos developers via their discord channel and I asked them to take a look at the failing test.

CC: @sr-gi

@sr-gi

sr-gi commented Oct 14, 2022

Copy link
Copy Markdown

Hey guys

...
test retrier::tests::test_manage_retry_unreachable ... FAILED
...
---- retrier::tests::test_manage_retry_unreachable stdout ----
thread 'retrier::tests::test_manage_retry_unreachable' panicked at 'assertion failed:
wt_client.lock().unwrap().towers.get(&tower_id).unwrap().status.is_unreachable()', watchtower-plugin/src/retrier.rs:518:9

That check is performed after a sleep to externally check the state of the retrier. It could be the case that the sleep is not big enough depending on the architecture and the check is performed before the state of the retrier has changed :S

Do you have any good way of reproducing this? I'd love to test this myself, and even add darwin+aarch64 to our test matrix

@prusnak

prusnak commented Oct 14, 2022 •

Copy link
Copy Markdown
Member

Do you have any good way of reproducing this?

I am not able to reproduce on aarch64-darwin with the official cargo/rust binaries. Only with the nixpkgs ones. Let's merge this PR as is and we can investigate the issue later.

@prusnak
prusnak merged commit 401e3d5 into NixOS:master Oct 14, 2022
@prusnak

prusnak commented Oct 14, 2022 •

Copy link
Copy Markdown
Member

Hm, I must have missed something earlier. Now I can reproduce by simply running the following on an Apple Silicon hardware (Macbook Air M1):

cd rust-teos
cargo build
cargo test
$ cargo --version
cargo 1.64.0 (387270bc7 2022-09-16)
$ rustc --version
rustc 1.64.0 (a55dd71d5 2022-09-19)

@sr-gi

sr-gi commented Oct 17, 2022

Copy link
Copy Markdown

Hm, I must have missed something earlier. Now I can reproduce by simply running the following on an Apple Silicon hardware (Macbook Air M1):

cd rust-teos
cargo build
cargo test
$ cargo --version
cargo 1.64.0 (387270bc7 2022-09-16)
$ rustc --version
rustc 1.64.0 (a55dd71d5 2022-09-19)

Any clue on how to run this without physically having an Apple Silicon mac? Looks like Githbub Actions do not support it yet and I don't have one myself to test it out :S

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants