Add btcpayserver #95104
Add btcpayserver #95104
Conversation
kcalvinalvin
force-pushed
the
add-btcpayserver
branch
from
e3d161d
to
cc7658f
Compare
| # 0. May need to run `dotnet nuget locals all --clear` if the user has built | ||
| # the package with dotnet previously | ||
| # 1. create a log with `dotnet restore -v n MyPackage.sln > mypackage-restore.log | ||
| # 2. then call ./create-deps.sh mypackage-restore.log |
There was a problem hiding this comment.
The script might be useful for other packages, I wonder if it makes sense to move it to e.g. maintainers/scripts?
There was a problem hiding this comment.
There are a lot of dotnet packages using a variation of this script. I took it and adjusted it slightly from one of them. Not sure where it originated though.
|
|
||
| installPhase = '' | ||
| mkdir -p $out/bin/ | ||
| echo -e '#!/usr/bin/env bash \n#launch BTCpayserver \nscriptdir=`dirname $(readlink $(which btcpayserver))` \ndotnet run --no-launch-profile --no-build -c Release -p "$scriptdir/../BTCPayServer/BTCPayServer.csproj" -- $@ |
There was a problem hiding this comment.
Suggestion: use writeScript (or makeWrapper?)
There was a problem hiding this comment.
Also using readlink makes the scriptdir dependent on $PATH. I.e. if btcpayserver is not in $PATH it won't start:
$ nix-build -A btcpayserver
...
$ ./result/bin/btcpayserver
which: no btcpayserver in (/run/wrappers/bin:/home/mmilata/.nix-profile/bin:/etc/profiles/per-user/mmilata/bin:/nix/var/nix/profiles/default/bin:/run/current-system/sw/bin)
readlink: missing operand
Try 'readlink --help' for more information.
dirname: missing operand
Try 'dirname --help' for more information.
./result/bin/btcpayserver: line 4: dotnet: command not found
Can you use the value of $out instead?
There was a problem hiding this comment.
Still getting the same error:
$ ./result/bin/btcpayserver
./result/bin/btcpayserver: line 3: dotnet: command not found
There was a problem hiding this comment.
:/ my bad, the script was calling the dotnet binary in my environment. Fix is pushed now (with writeScript as well)
| dontStrip = true; | ||
|
|
||
| meta = with lib; { | ||
| description = "BTCPay Server is a self-hosted, open-source cryptocurrency payment processor. It's secure, private, censorship-resistant and free"; |
There was a problem hiding this comment.
Description should not start with a package name.
kcalvinalvin
force-pushed
the
add-btcpayserver
branch
from
cc7658f
to
ca4cf12
Compare
There was a problem hiding this comment.
Thanks for the package! I am not qualified to review dotnet packaging but is there an existing similar package already? Fwiw, I can build it on archlinux with sandbox and I can successfully play around with btcpayserver after installing dotnet-sdk_3. Is that dotnet-sdk_3 runtime dependency necessary? The wasabibackend package doesn't seem to require this.
Is it true that the nbxplorer package is independent and is not a dependency of btcpayserver?
| script='#!/usr/bin/env bash\n#launch BTCpayserver\ndotnet run --no-launch-profile --no-build -c Release -p "REPLACE/BTCPayServer/BTCPayServer.csproj" -- $@' | ||
| echo -e $script | sed "s@REPLACE@$out@g" > $out/bin/run.sh | ||
| chmod +x $out/bin/run.sh | ||
| ln -s $out/bin/run.sh $out/bin/btcpayserver |
There was a problem hiding this comment.
why do we need both run.sh and btcpayserver?
There was a problem hiding this comment.
It is not needed. Removed in the latest commit
kcalvinalvin
force-pushed
the
add-btcpayserver
branch
from
ca4cf12
to
c101ae0
Compare
I think it is since this happened #95104 (comment)
It's possible to run nbxplorer without btcpayserver as stated in the nbxplorer readme though I don't think there are many that are doing so. |
Nice, the fix works for me. |
| mkdir home | ||
| export HOME=$PWD/home | ||
|
|
||
| export DOTNET_CLI_TELEMETRY_OPTOUT=1 |
There was a problem hiding this comment.
Can you add this to the bin-script? Or something like DOTNET_CLI_TELEMETRY_OPTOUT=${DOTNET_CLI_TELEMETRY_OPTOUT:-1}?
There was a problem hiding this comment.
I've added DOTNET_CLI_TELEMETRY_OPTOUT=1
|
Module: fort-nix/nix-bitcoin#221 |
kcalvinalvin
force-pushed
the
add-btcpayserver
branch
from
c101ae0
to
47a8e94
Compare
There was a problem hiding this comment.
Please add the following changes: https://github.com/erikarvstedt/nixpkgs/commits/add-btcpayserver
See the individual commit messages for details. Highlights:
- the script can now update the pkgs to the lastest release
- ~10x faster pkg build time
- decreased pkg output size : 830M -> 6.6M (nbxplorer), 1.5G -> 77M (btcpayserver)
- much faster
deps.nixgeneration
Here's the squashed branch including these changes: https://github.com/erikarvstedt/nixpkgs/commits/add-btcpayserver-squashed
Note the updated version in the nbxplorer commit message.
This includes the latest fixups from NixOS/nixpkgs#95104
|
Very nice improvements @erikarvstedt. Agree that they should be added to this PR. I also like that you kept the common-updater very general and simplified the build phase. My generated deps.nix differs in the order of the listed dependencies because this sort on my machine puts |
|
The order was produced by |
|
@erikarvstedt I'll just cherry-pick your squashed commits here if that's ok |
|
@kcalvinalvin, sure, that's what I had prepared the squashed branch for 🐱 Could you also add the latest changes? Just pull in the squashed branch again. |
Fix works for me. I can reproduce exactly the same deps.nix now. |
|
I rebased my module on @erikarvstedt's changes. The speed and size improvements are great. Everything compiles fine, but when I try to connect to the BTCPayServer URL I get the following error: Could this have to do with |
|
@nixbitcoin I am also getting the same issue. Was able to fix the issue by reverting the In the btcpayserver repo, they just have a 3 line shell script which I just copied. I think this should just be followed? |
|
Pushed new commits that are based off of @erikarvstedt's work |
There was a problem hiding this comment.
Sorry for not testing the HTTP server. I've filed an upstream bug report.
@kcalvinalvin, please add these changes. They work around the build bug while keeping the reduced output size (89M for btcpayserver) of the dotnet publish approach.
kcalvinalvin
force-pushed
the
add-btcpayserver
branch
2 times, most recently
from
807e089
to
4fe6bf6
Compare
|
Merged the new commits and checked that the webfront works |
|
@kcalvinalvin, thanks! |
|
@erikarvstedt done :) |
kcalvinalvin
force-pushed
the
add-btcpayserver
branch
from
4fe6bf6
to
a04a87b
Compare
kcalvinalvin
force-pushed
the
add-btcpayserver
branch
2 times, most recently
from
1f125cd
to
9959488
Compare
There was a problem hiding this comment.
Sorry, there were two more small fixes I asked @kcalvinalvin to pull in.
This looks great now. 🚀
There was a problem hiding this comment.
We missed adding the version to the package names.
@kcalvinalvin, as before, please update from here.
|
@erikarvstedt I can do that. But I'm not seeing any version changes with |
|
Yes, these changes are correct. |
kcalvinalvin
force-pushed
the
add-btcpayserver
branch
from
9959488
to
2f55d6f
Compare
TIL. Changes applied |
|
Thank you, great work! |
Motivation for this change
Adds package btcpayserver and its dependency, nbxplorer
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)