Skip to content
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.

Sign up for GitHub

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

dogecoin: restore #341302

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

dogecoin: restore #341302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
06967c2
Revert "dogecoin, dogecoind: remove"
cleverca22 Sep 12, 2024
0df90e8
dogecoind: update to 1.14.8 and fix building without gui
cleverca22 Sep 12, 2024
c256b5c
dogecoind: apply nixfmt
cleverca22 Sep 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 104 additions & 0 deletions pkgs/applications/blockchains/dogecoin/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
{
lib,
stdenv,
fetchFromGitHub,
pkg-config,
autoreconfHook,
db5,
openssl,
boost,
zlib,
miniupnpc,
libevent,
protobuf,
qtbase ? null,
wrapQtAppsHook ? null,
qttools ? null,
qmake ? null,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

qmake is unused.

qrencode,
withGui,
withUpnp ? false,
withUtils ? true,
withWallet ? true,
withZmq ? true,
zeromq,
util-linux ? null,
Cocoa ? null,
}:

stdenv.mkDerivation rec {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use the finalAttrs pattern here.

The finalAttrs pattern will let you remove the rec keyword (see its implementation in Nix).

Why is this pattern preferred to rec ?

Let's take this simple code example:

mkDerivation rec {
   foo = 1;
   bar = foo + 1;
}

and then .overrideAttrs(old: { foo = 2; }), you'll get { foo = 2; bar = 2; } while with finalAttrs pattern, it would work correctly because it's a real fixed point.

Let me share a couple of useful links regarding the finalAttrs pattern:

  1. History: https://discourse.nixos.org/t/avoid-rec-expresions-in-nixpkgs/8293
  2. Documentation: https://nixos.org/manual/nixpkgs/unstable/#mkderivation-recursive-attributes
  3. Recent example of implementation: https://github.com/NixOS/nixpkgs/compare/17f96f7b978e61576cfe16136eb418f74fab9952..9e6ea843e473d34d4f379b8b0d8ef0425a06defe

Feel free to reach out if you need some assistance.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i didnt really write that rec part, and i do personally try to avoid it

if you look at each commit seperately, youll see the first one is a revert of somebody deleting dogecoin entirely, which is where the rec came from
and the next commit is just updating the version, and disabling upnp

Copy link
Contributor

@oxalica oxalica Sep 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i didnt really write that rec part, and i do personally try to avoid it
if you look at each commit seperately, youll see the first one is a revert of somebody deleting dogecoin entirel

You could argue that rec is not worse than finalAttrs (which I might say). But your reason does not seem valid to me. "It is written by someone else" or "it used to be in nixpkgs" does not imply it meets the current practice. Legacy means legacy for a reason.

This should be reviewed as a new-package PR, because it adds a package which did not exist. You could reference the previous commit, but now that code is under your contribution and you are responsible for it.

But anyway, I think rec or finalAttrs are both acceptable for this PR.

pname = "dogecoin" + lib.optionalString (!withGui) "d";
version = "1.14.8";

src = fetchFromGitHub {
owner = "dogecoin";
repo = "dogecoin";
rev = "v${version}";
sha256 = "sha256-Jmdxaona9bI9mw+WDGnrFU2ETgVTTQ7HHaWRodK/c4k=";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
sha256 = "sha256-Jmdxaona9bI9mw+WDGnrFU2ETgVTTQ7HHaWRodK/c4k=";
hash = "sha256-Jmdxaona9bI9mw+WDGnrFU2ETgVTTQ7HHaWRodK/c4k=";

};

preConfigure = lib.optionalString withGui ''
export LRELEASE=${lib.getDev qttools}/bin/lrelease
'';

nativeBuildInputs =
[
pkg-config
autoreconfHook
util-linux
]
++ lib.optionals withGui [
wrapQtAppsHook
qttools
];

buildInputs =
[
openssl
protobuf
boost
zlib
libevent
]
++ lib.optionals withGui [
qtbase
qrencode
]
++ lib.optionals withUpnp [ miniupnpc ]
++ lib.optionals withWallet [ db5 ]
++ lib.optionals withZmq [ zeromq ]
++ lib.optionals stdenv.isDarwin [ Cocoa ];

configureFlags =
[
"--with-incompatible-bdb"
"--with-boost-libdir=${boost.out}/lib"
]
++ lib.optionals (!withGui) [ "--with-gui=no" ]
++ lib.optionals (!withUpnp) [ "--without-miniupnpc" ]
++ lib.optionals (!withUtils) [ "--without-utils" ]
++ lib.optionals (!withWallet) [ "--disable-wallet" ]
++ lib.optionals (!withZmq) [ "--disable-zmq" ];

enableParallelBuilding = true;

meta = with lib; {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing meta.mainProgram

description = "Wow, such coin, much shiba, very rich";
longDescription = ''
Dogecoin is a decentralized, peer-to-peer digital currency that
enables you to easily send money online. Think of it as "the
internet currency."
It is named after a famous Internet meme, the "Doge" - a Shiba Inu dog.
'';
homepage = "https://www.dogecoin.com/";
license = licenses.mit;
maintainers = with maintainers; [
edwtjo
offline
cleverca22
craigem
];
Comment on lines +95 to +100
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be best to remove the maintainers who did not explicitly agree, even if this is just a revert.

platforms = platforms.unix;
broken = withGui;
};
}
2 changes: 0 additions & 2 deletions pkgs/top-level/aliases.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,8 +341,6 @@ mapAliases ({
docker-machine-kvm = throw "'docker-machine-kvm' has been removed, because 'docker-machine' was archived upstream and removed"; # Added 2023-12-27
docker-machine-xhyve = throw "'docker-machine-xhyve' has been removed, because 'docker-machine' was archived upstream and removed"; # Added 2023-12-27
docker-proxy = throw "`docker-proxy` has been merged to the main repo of Moby since Docker 22.06"; # Added 2024-03-14
dogecoin = throw "'dogecoin' has been removed, as it was broken and unmaintained"; # Added 2024-03-11
dogecoind = throw "'dogecoind' has been removed, as it was broken and unmaintained"; # Added 2024-03-11
dolphin-emu-beta = dolphin-emu; # Added 2023-02-11
dolphinEmu = dolphin-emu; # Added 2021-11-10
dolphinEmuMaster = dolphin-emu-beta; # Added 2021-11-10
Expand Down
7 changes: 7 additions & 0 deletions pkgs/top-level/all-packages.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34821,6 +34821,13 @@ with pkgs;
dcrd = callPackage ../applications/blockchains/dcrd { };
dcrwallet = callPackage ../applications/blockchains/dcrwallet { };

dogecoin = libsForQt5.callPackage ../applications/blockchains/dogecoin {
withGui = true;
};
dogecoind = callPackage ../applications/blockchains/dogecoin {
withGui = false;
};

eclair = callPackage ../applications/blockchains/eclair { };

electrs = callPackage ../applications/blockchains/electrs {
Expand Down