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.

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: 0.17.1.1 -> 0.17.1.3 #103208

Merged
merged 2 commits into from Nov 14, 2020
Merged

monero: 0.17.1.1 -> 0.17.1.3 #103208

merged 2 commits into from Nov 14, 2020

Conversation

prusnak
Copy link
Member

@prusnak prusnak commented Nov 9, 2020

Motivation for this change

Version update + update monero-gui to 0.17.1.4 (latest)

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@prusnak
Copy link
Member Author

prusnak commented Nov 9, 2020

@rnhmjoj Can you please have a look? When I build monero-gui it fails with File name too long because a very-very long path is passed to cc1plus. monero builds fine

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Nov 9, 2020

It looks like CMAKE_PREFIX_PATH became so large that the command line of gcc is geing truncated.

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Nov 9, 2020

They just released another update: 0.17.1.4. I would wait untile one is marked as "latest release" before updating the package here. In any case, I found out what's the cause of the crash but I don't really understand the issue.
Apply this patch and it should fix the build. For some reason this is duplicating every path in the CMAKE_PREFIX_PATH, resulting in a huge gcc command line.

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 9edb9c6b..3eb38aa9 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -221,10 +221,6 @@ if(APPLE)
 endif()
 
 if(UNIX)
-  if(NOT CMAKE_PREFIX_PATH AND DEFINED ENV{CMAKE_PREFIX_PATH})
-    message(STATUS "Using CMAKE_PREFIX_PATH environment variable: '$ENV{CMAKE_PREFIX_PATH}'")
-    set(CMAKE_PREFIX_PATH $ENV{CMAKE_PREFIX_PATH})
-  endif()
   if(APPLE AND NOT CMAKE_PREFIX_PATH)
     execute_process(COMMAND brew --prefix qt5 OUTPUT_VARIABLE QT5_DIR OUTPUT_STRIP_TRAILING_WHITESPACE)
     list(APPEND CMAKE_PREFIX_PATH ${QT5_DIR})

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Nov 9, 2020

Here's a better solution, but hopefully it will be fixed in the next release:

From b64f79a629fea0cffaf23609ec71d1460f90c2d6 Mon Sep 17 00:00:00 2001
From: rnhmjoj <rnhmjoj@inventati.org>
Date: Mon, 9 Nov 2020 23:01:21 +0100
Subject: [PATCH] monero-gui: fix build

---
 pkgs/applications/blockchains/monero-gui/default.nix   |  2 +-
 .../blockchains/monero-gui/duplicate-includes.patch    | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)
 create mode 100644 pkgs/applications/blockchains/monero-gui/duplicate-includes.patch

diff --git a/pkgs/applications/blockchains/monero-gui/default.nix b/pkgs/applications/blockchains/monero-gui/default.nix
index bd6aacc9a94..72936f1765f 100644
--- a/pkgs/applications/blockchains/monero-gui/default.nix
+++ b/pkgs/applications/blockchains/monero-gui/default.nix
@@ -58,7 +58,7 @@ stdenv.mkDerivation rec {
     chmod -R +w source/monero
   '';
 
-  patches = [ ./move-log-file.patch ];
+  patches = [ ./move-log-file.patch ./duplicate-includes.patch ];
 
   postPatch = ''
     # set monero-gui version
diff --git a/pkgs/applications/blockchains/monero-gui/duplicate-includes.patch b/pkgs/applications/blockchains/monero-gui/duplicate-includes.patch
new file mode 100644
index 00000000000..de045e36731
--- /dev/null
+++ b/pkgs/applications/blockchains/monero-gui/duplicate-includes.patch
@@ -0,0 +1,10 @@
+--- a/CMakeLists.txt	2020-11-09 22:38:42.948315032 +0100
++++ b/CMakeLists.txt	2020-11-09 22:56:07.411646638 +0100
+@@ -231,7 +231,6 @@
+   endif()
+ 
+   if(CMAKE_PREFIX_PATH)
+-    include_directories(${CMAKE_PREFIX_PATH}/include)
+     set(CMAKE_BUILD_RPATH "${CMAKE_PREFIX_PATH}/lib")
+   endif()
+ endif()
-- 
2.28.0


@prusnak
Copy link
Member Author

prusnak commented Nov 9, 2020

Here's a better solution, but hopefully it will be fixed in the next release:

Will you please send a PR to monero-gui, so the fix can appear in the next release?

@rnhmjoj
Copy link
Contributor

rnhmjoj commented Nov 9, 2020

In fact, I have been chatting with upstream on IRC and they have written a fix. It should be in the next patch release. For now, you can fetchpath this xiphon/monero-gui@d967ade.

@prusnak
Copy link
Member Author

prusnak commented Nov 10, 2020

monero-gui fixed in 52ee36a

Copy link
Contributor

@rnhmjoj rnhmjoj left a comment

Choose a reason for hiding this comment

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

Looks good, but I would still wait until the tag the release on github.

@prusnak prusnak marked this pull request as ready for review November 14, 2020 18:22
@prusnak prusnak requested a review from mmahut as a code owner November 14, 2020 18:22
@mmahut mmahut merged commit e43e173 into NixOS:master Nov 14, 2020
@prusnak prusnak deleted the monero branch November 14, 2020 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants