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

yaft: init at 0.2.9 #22261

Closed
wants to merge 2 commits into from
Closed

Conversation

matthiasbeyer
Copy link
Contributor

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.


meta = {
homepage = "https://github.com/uobikiemukot/yaft";
description = "yet another framebuffer terminal";
Copy link
Member

Choose a reason for hiding this comment

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

The "y" in "yet" should be capitalized.

@rycee
Copy link
Member

rycee commented Jan 29, 2017

Maybe you forgot to add an attribute in all-packages.nix?

@rycee
Copy link
Member

rycee commented Jan 29, 2017

Hmm, this didn't build very well for me. Well it built but never installed anything but a few empty directories. I changed the expression a little:

diff --git a/pkgs/applications/misc/yaft/default.nix b/pkgs/applications/misc/yaft/default.nix
index a1708c6..9d717e0 100644
--- a/pkgs/applications/misc/yaft/default.nix
+++ b/pkgs/applications/misc/yaft/default.nix
@@ -1,4 +1,4 @@
-{ stdenv, fetchFromGitHub, pkgconfig, ncurses, readline }:
+{ stdenv, fetchFromGitHub, ncurses }:
 
 stdenv.mkDerivation rec {
   version = "0.2.9";
@@ -11,10 +11,9 @@ stdenv.mkDerivation rec {
     sha256 = "0l1ig8wm545kpn4l7186rymny83jkahnjim290wsl7hsszfq1ckd";
   };
 
-  installPhase = ''
-    mkdir -p $out/{bin,share/man}
-    DESTDIR=$out PREFIX=$out/bin/ MANPREFIX=$out/share/man/
-  '';
+  buildInputs = [ ncurses ];
+
+  installFlags = [ "PREFIX=$(out)" "MANPREFIX=$(out)/share/man/" ];
 
   meta = {
     homepage = "https://github.com/uobikiemukot/yaft";

and with that it appears to build and install OK.

@matthiasbeyer
Copy link
Contributor Author

Would be way easier if you would have pushed a commit that I can cherry-pick. But well, fixed your suggestions.

@rasendubi
Copy link
Member

rasendubi commented Jan 30, 2017

@matthiasbeyer, you don't seem to apply the patch. I have opened a PR with the fixes: matthiasbeyer#5.

@matthiasbeyer
Copy link
Contributor Author

@rasendubi 10 hrs ago I was in bed sleeping, and I have a life besides my computer. But here you go!

@rycee
Copy link
Member

rycee commented Jan 31, 2017

Squashed and rebased into master in commit 904bcb3.

@rycee rycee closed this Jan 31, 2017
@matthiasbeyer matthiasbeyer deleted the add-yaft branch January 31, 2017 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants