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

monero-cli: update submodule version; disable aarch64-darwin #341980

Merged
merged 1 commit into from
Oct 9, 2024
+20 −7
Merged

monero-cli: update submodule version; disable aarch64-darwin #341980

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 20 additions & 7 deletions pkgs/applications/blockchains/monero-cli/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
}:

let
# submodules
# submodules; revs are taken from monero repo's `/external` at the given monero version tag.
supercop = fetchFromGitHub {
owner = "monero-project";
repo = "supercop";
Expand All @@ -37,12 +37,11 @@ let
trezor-common = fetchFromGitHub {
owner = "trezor";
repo = "trezor-common";
rev = "bc28c316d05bf1e9ebfe3d7df1ab25831d98d168";
hash = "sha256-F1Hf1WwHqXMd/5OWrdkpomszACTozDuC7DQXW3p6248=";
rev = "bff7fdfe436c727982cc553bdfb29a9021b423b0";
hash = "sha256-VNypeEz9AV0ts8X3vINwYMOgO8VpNmyUPC4iY3OOuZI=";
};

in

stdenv.mkDerivation rec {
pname = "monero-cli";
version = "0.18.3.4";
Expand Down Expand Up @@ -111,14 +110,28 @@ stdenv.mkDerivation rec {
"-DCMAKE_CXX_FLAGS=-fpermissive"
];

outputs = [ "out" "source" ];
outputs = [
philipmw marked this conversation as resolved.
Show resolved Hide resolved
"out"
"source"
];

meta = {
description = "Private, secure, untraceable currency";
homepage = "https://getmonero.org/";
license = lib.licenses.bsd3;
platforms = lib.platforms.all;
maintainers = with lib.maintainers; [ rnhmjoj ];

platforms = with lib.platforms; linux;

# macOS/ARM has a working `monerod` (at least), but `monero-wallet-cli`
# segfaults on start after entering the wallet password, when built in release mode.
# Building the same revision in debug mode to root-cause the above problem doesn't work
# because of https://github.com/monero-project/monero/issues/9486
badPlatforms = [ "aarch64-darwin" ];
Copy link
Member

Choose a reason for hiding this comment

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

Setting badPlatforms when platforms does not include it, is redundant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks; I wasn't sure about that. I was following the advice from an earlier review of this PR: #341980 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

@philipmw only now I noticed, that you change from platforms.all to platforms.linux

I think the best would be to change to platform.unix for both monero-cli and monero-gui - can you please send a PR to set this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@prusnak, I don't have any environment besides Linux and macOS to test, so I don't think it would be right for me to advertise a capability that I cannot validate. Can you validate?

Copy link
Member

Choose a reason for hiding this comment

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

@philipmw ok, use linux and darwin instead of unix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, can you clarify what you are asking me to change? Right now the list of supported platforms is linux and not darwin because aarch64-darwin is broken for me. I don't see a way to improve on this.

Copy link
Member

Choose a reason for hiding this comment

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

There is still x86_64-darwin

Your earlier change is a regression because now the package is not available for this platform

I suggest to change the platforms to [ linux darwin ] for both monero-cli and monero-gui.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, @prusnak. I do have an Intel-based MacBook Pro I can test with, so I tried nix-build -A monero-cli tonight on master. Unfortunately the build is failing:

Undefined symbols for architecture x86_64:
  "_pthread_jit_write_protect_np", referenced from:
      _allocMemoryPages in librandomx.a(virtual_memory.c.o)
      _setPagesRW in librandomx.a(virtual_memory.c.o)
      _setPagesRX in librandomx.a(virtual_memory.c.o)
ld: symbol(s) not found for architecture x86_64
clang-16: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [src/blockchain_utilities/CMakeFiles/blockchain_prune.dir/build.make:137: bin/monero-blockchain-prune] Error 1
make[1]: *** [CMakeFiles/Makefile2:3423: src/blockchain_utilities/CMakeFiles/blockchain_prune.dir/all] Error 2

I rewound the repo to the commit before my change (97b33aa) and tried building that. That fails also, though with a different error:

In file included from /tmp/nix-build-monero-cli-0.18.3.4.drv-0/source/src/device_trezor/device_trezor.cpp:30:
In file included from /tmp/nix-build-monero-cli-0.18.3.4.drv-0/source/src/device_trezor/device_trezor.hpp:46:
/tmp/nix-build-monero-cli-0.18.3.4.drv-0/source/src/device_trezor/device_trezor_base.hpp:232:33: error: cannot initialize a parameter of type '::hw::trezor::messages::monero::MoneroNetworkType' with an rvalue of type 'uint32_t' (aka 'unsigned int')
          msg->set_network_type(static_cast<uint32_t>(this->network_type));
                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/nix-build-monero-cli-0.18.3.4.drv-0/source/src/device_trezor/trezor/messages/messages-monero.pb.h:13204:110: note: passing argument to parameter 'value' here
inline void MoneroTransactionInitRequest::set_network_type(::hw::trezor::messages::monero::MoneroNetworkType value) {
                                                                                                             ^
12 errors generated.
make[2]: *** [src/device_trezor/CMakeFiles/obj_device_trezor.dir/build.make:132: src/device_trezor/CMakeFiles/obj_device_trezor.dir/device_trezor.cpp.o] Error 1

So, I think not including "darwin" is not a regression, since it didn't use to build anyway... and since it still doesn't build, it's still fine to not include it. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

OK


maintainers = with lib.maintainers; [
pmw
rnhmjoj
];
mainProgram = "monero-wallet-cli";
};
}