-
Notifications
You must be signed in to change notification settings - Fork 108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Get electrs source tarball with gpg verified sha256 and corresponding helper script #156
Conversation
0862277
to
b47b785
Compare
Here are a few patches that I think are sensible. What was the reason for the |
f1e367a
to
b47b785
Compare
Wow, thank you. I learned so much from your commits. Truly much better with your changes. The reason for |
Re: 90f3f35 |
Regarding |
I get this when removing
|
I can't reproduce the error when removing |
I'm deploying from the When using your version without
I get error
|
Ah, the error appears when building the electrs pkg pinned to nixpkgs-unstable that's used in the modules:
This bug has been fixed in the current |
Specifically to advance fort-nix#156 without cargoDepsHook
LGTM. Can you squash the fixup commits and put the |
With
I tried changing clang to a newer version with -{ lib, rustPlatform, clang, llvmPackages, fetchurl, pkgs }:
+{ lib, rustPlatform, llvmPackages_10, fetchurl, pkgs }:
rustPlatform.buildRustPackage rec {
pname = "electrs";
version = "0.8.3";
@@ -10,8 +10,8 @@ rustPlatform.buildRustPackage rec {
};
# Needed for librocksdb-sys
- buildInputs = [ clang ];
- LIBCLANG_PATH = "${llvmPackages.libclang}/lib";
+ buildInputs = [ llvmPackages_10.clang ];
+ LIBCLANG_PATH = "${llvmPackages_10.libclang}/lib"; But still got the error I think it's somehow related to romanz/electrs#226 and NixOS/nixpkgs@dc3c338#diff-0c4100e0613807f5600eb55c5446b20e but can't figure out how. Maybe you have an idea. |
82e5d87
to
7f8e452
Compare
… helper script move script to pkg dir, add hint to script in pkg def remove unneeded script deps add extended bash error checking rename DIR -> TMPDIR remove TMPDIR on exit strip whitespace, simplify comments gpg2 -> gpg latesttagelectrs -> latest tmpdir: don't use XDG_RUNTIME_DIR XDG_RUNTIME_DIR is often in RAM and shouldn't be used for larger workloads like repo downlaods verify fingerprint of the imported key remove trailing '-' in output simplify output Hide --fetch-key output Output is not relevant to user, looks better without it More accurately describe ./get-sha256 function User might think that ./get-sha256 automatically updates sha256 in default.nix Fetch key from sks keyservers instead of keybase.io Using --recv-key simplifies getting the right key, and only the right key, greatly. I try to refrain from using sks keyservers, but the certificate spamming attack shouldn't be an issue in this case because we create a temporary keychain just for the verificaiton. remove unneeded cargoDepsHook Make clang nativeBuildInput instead of buildInput
7f8e452
to
1acb22a
Compare
I was finally able to fix the issue by making
Done |
The test currently times out because of the channel update. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test runs fine locally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 1acb22a
I pushed the branch to this repo to enable cachix pushes and fix travis.
I was finally able to fix the issue by making clang a nativeBuildInput instead of a buildInput.
Ouch, this probably took a while.
This PR changes the electrs expression to fetch sources from the releases archive with
fetchurl
instead offetchFromGitHub
. This allows us to calculate the sha256 ahead of time and compare it with Roman Zeyde's gpg-signed git tags. I have provided a helper script to automate the process.Overall this should improve expression integrity, and I plan to implement this for all our packages where gpg verification is possible.