faustlive: init at 2017-12-05 #45493

ThysBallard · 2018-08-22T23:47:05Z

Motivation for this change

Added FaustLive package for fast prototyping with FAUST programming language

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
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 nox --run "nox-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)
Fits CONTRIBUTING.md.

AndersonTorres · 2018-08-23T18:53:08Z

Well, I think we need some hints of style here! Are you ready? :)

ThysBallard · 2018-08-23T19:27:11Z

@AndersonTorres I mean yeah, go for it.

AndersonTorres · 2018-08-23T19:04:09Z

pkgs/applications/audio/faust/faustlive.nix

+stdenv.mkDerivation rec {
+    name = "faustlive-${version}";
+    version = "2017-12-05";
+    src = fetchgit {


Here I will suggest to use fetchFromGitHub. It is a less general function than fetchgit. It is always a better idea to use specialized fetchers, it reduces the "mental load".

You can look at this code as an example (yes, it's mine!):

nixpkgs/pkgs/development/compilers/jwasm/default.nix

Lines 9 to 14 in 4a553dc

src = fetchFromGitHub {

owner = "JWasm";

repo = "JWasm";

rev = "26f97c8b5c9d9341ec45538701116fa3649b7766";

sha256 = "0m972pc8vk8s9yv1pi85fsjgm6hj24gab7nalw2q04l0359nqi7w";

};

AndersonTorres · 2018-08-23T19:12:16Z

pkgs/applications/audio/faust/faustlive.nix

+{ stdenv
+, fetchgit
+, llvm
+, gnumake


Wait. I am a little confused.

Isn't gnumake and gnused part of stdenv? If you called stdenv, automatically GNU Make is present!

Did you find some difficulty? The expression works without that?

Sorry, that was a leftover from an earlier version of the expression.

AndersonTorres · 2018-08-23T19:13:29Z

pkgs/applications/audio/faust/faustlive.nix

+, gnused
+, coreutils
+, which 
+}:


Wow. So much lines.

Try to organize them, putting various parameters in a same line (always respecting the magic 80-characters limit).

Also, try to organize them by, what I would say?, logic. Libraries as (libjack2, alsaLib, qt48Full) grouped in a line, build tools (as llvm and gnumake) in other, auxiliary utilities (coreutils, with, gnused) in another one... You don't need to be overly accurate here, I think you get the general idea...

AndersonTorres · 2018-08-23T19:15:33Z

pkgs/applications/audio/faust/faustlive.nix

+    };
+
+    meta = with stdenv.lib; {
+        description = "FaustLive is a standalone just-in-time Faust compiler";


No self-reference is needed in a small description. You can just rewrite it as

A standalone just-in-time Faust compiler

(No ending period, as you already did).

AndersonTorres · 2018-08-23T19:18:59Z

pkgs/applications/audio/faust/faustlive.nix

+        sha256 = "0sw44yd9928rid9ib0b5mx2x129m7zljrayfm6jz6hrwdc5q3k9a";
+    };
+
+    meta = with stdenv.lib; {


meta is supposed to be the last thing in the derivation.

AndersonTorres · 2018-08-23T19:24:41Z

pkgs/applications/audio/faust/faustlive.nix

+
+    meta = with stdenv.lib; {
+        description = "FaustLive is a standalone just-in-time Faust compiler";
+        longDescription = ''


Rewrite that longDescription, always respecting the magic 80-characters limit, and using two chars as indent.
Indeed, this line is so long I need to use the horizontal scrolling bars of my browser.

Another example (mine, again!):

nixpkgs/pkgs/applications/virtualization/bochs/default.nix

Lines 115 to 120 in 71f899c

description = "An open-source IA-32 (x86) PC emulator";

longDescription = ''

Bochs is an open-source (LGPL), highly portable IA-32 PC emulator, written

in C++, that runs on most popular platforms. It includes emulation of the

Intel x86 CPU, common I/O devices, and a custom BIOS.

'';

AndersonTorres · 2018-08-23T21:17:04Z

pkgs/applications/audio/faust/faustlive.nix

+    makeFlags = [ "PREFIX=$(out)" ];
+
+    buildPhase = '' 
+        substituteInPlace Build/Linux/buildversion --replace "#!/bin/bash" ""


It can be rewritten as

preBuild = "patchShebangs Build/Linux/buildversion";

ThysBallard · 2018-08-24T00:02:48Z

@AndersonTorres Thanks for the tips. Let me know if these changes did the job :)

AndersonTorres · 2018-08-24T01:53:13Z

pkgs/applications/audio/faust/faustlive.nix

+    homepage = http://faust.grame.fr/;
+    license = licenses.gpl3;
+  };
+


Useless newline. It can be removed.

AndersonTorres · 2018-08-24T01:57:20Z

pkgs/applications/audio/faust/faustlive.nix

+  };
+
+}
+


Double newline at the end of file. Leave only one here.

P.S.: there are some spurious whitespaces in your code. They pollute Git logs. Specifically, lines 3, 17 and 19 have endline whitespaces.

ThysBallard · 2018-08-24T18:56:27Z

Hopefully everything is clean and tidy now :)

AndersonTorres · 2018-08-25T15:23:33Z

I have tested it locally. It compíles and builds cleanly. If no one else objects, I will merge it tomorrow.

ThysBallard · 2018-08-25T18:38:51Z

@AndersonTorres thanks for all the help! This was my first pull request ever even though I've been programming for years so thanks for dealing with my crap

ThysBallard force-pushed the master branch from c1e4093 to 28486d7 Compare August 23, 2018 06:09

GrahamcOfBorg added 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 1-10 labels Aug 23, 2018

AndersonTorres self-requested a review August 23, 2018 18:52

AndersonTorres suggested changes Aug 23, 2018

View reviewed changes

ThysBallard force-pushed the master branch 2 times, most recently from ad9e916 to c2cc82a Compare August 23, 2018 23:47

AndersonTorres reviewed Aug 24, 2018

View reviewed changes

faustlive: init at 2017-12-05

7ef0a74

ThysBallard force-pushed the master branch from c2cc82a to 7ef0a74 Compare August 24, 2018 02:30

AndersonTorres approved these changes Aug 25, 2018

View reviewed changes

AndersonTorres merged commit 18006f8 into NixOS:master Aug 28, 2018

hrhino mentioned this pull request May 27, 2021

faust2: unstable-2020-08-27 -> 2.30.5, faustlive: unstable-dev-2020-08-03 -> 2.5.5 #124063

Merged

9 tasks

Janik-Haag added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

faustlive: init at 2017-12-05 #45493

faustlive: init at 2017-12-05 #45493

ThysBallard commented Aug 22, 2018 •

edited

Loading

AndersonTorres commented Aug 23, 2018

ThysBallard commented Aug 23, 2018

AndersonTorres Aug 23, 2018

AndersonTorres Aug 23, 2018

ThysBallard Aug 23, 2018

AndersonTorres Aug 23, 2018

AndersonTorres Aug 23, 2018

AndersonTorres Aug 23, 2018

AndersonTorres Aug 23, 2018

AndersonTorres Aug 23, 2018

ThysBallard commented Aug 24, 2018

AndersonTorres Aug 24, 2018

AndersonTorres Aug 24, 2018

ThysBallard commented Aug 24, 2018

AndersonTorres commented Aug 25, 2018

ThysBallard commented Aug 25, 2018

	src = fetchFromGitHub {
	owner = "JWasm";
	repo = "JWasm";
	rev = "26f97c8b5c9d9341ec45538701116fa3649b7766";
	sha256 = "0m972pc8vk8s9yv1pi85fsjgm6hj24gab7nalw2q04l0359nqi7w";
	};

	description = "An open-source IA-32 (x86) PC emulator";
	longDescription = ''
	Bochs is an open-source (LGPL), highly portable IA-32 PC emulator, written
	in C++, that runs on most popular platforms. It includes emulation of the
	Intel x86 CPU, common I/O devices, and a custom BIOS.
	'';

faustlive: init at 2017-12-05 #45493

faustlive: init at 2017-12-05 #45493

Conversation

ThysBallard commented Aug 22, 2018 • edited Loading

Motivation for this change

Things done

AndersonTorres commented Aug 23, 2018

ThysBallard commented Aug 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThysBallard commented Aug 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThysBallard commented Aug 24, 2018

AndersonTorres commented Aug 25, 2018

ThysBallard commented Aug 25, 2018

ThysBallard commented Aug 22, 2018 •

edited

Loading