librepcb-unstable: init at 2017-12-29 #33630

Luz · 2018-01-08T22:26:31Z

Motivation for this change

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
- 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/)
Fits CONTRIBUTING.md.

das-g · 2018-01-08T23:30:27Z

pkgs/applications/science/electronics/librepcb/default.nix

+	meta = with stdenv.lib; {
+		description = "A free EDA software to develop printed circuit boards";
+		homepage = http://librepcb.org/;
+		maintainers = with maintainers; [ luz ];


You'll have to add yourself to lib/maintainers.nix for that to work.

andir

Thank you for PR!

I've left some comments.

During build I am receiving a few lines like this:

g++ -c -pipe -Wextra -O2 -fPIC -std=gnu++11 -Wall -W -D_REENTRANT -DQUAZIP_STATIC -DAPP_VERSION=\"0.1.0\" -DFIp
/nix/store/nkq0n2m4shlbdvdq0qijib5zyzgmn0vq-bash-4.4-p12/bin/bash: git: command not found
g++ -c -pipe -Wextra -O2 -fPIC -std=gnu++11 -Wall -W -D_REENTRANT -DQUAZIP_STATIC -DAPP_VERSION=\"0.1.0\" -DFIp

Should we provide git to extract the current treeish/tag to be displayed during runtime?

When trying to execute the application I am getting the following error:

This application failed to start because it could not find or load the Qt platform plugin "xcb"
in "".

andir · 2018-01-08T23:48:10Z

pkgs/applications/science/electronics/librepcb/default.nix

+	patches = [ ./fix-2017-12.patch ];
+
+	#recursive qmake building is required -> therefore this line is required (qmake -r)
+	preBuild = "mkdir build && cd build && qmake -r ../librepcb.pro PREFIX=$out QMAKE_LRELEASE=${stdenv.lib.getDev qttools.dev}/bin/lrelease";


You can probably just do qmakeFlags = [ "-r" ]; instead of your preBuild line. There are qmke hooks in-place that take care of the PREFIX and QMAKE_LRELEASE etc...

andir · 2018-01-08T23:50:47Z

pkgs/applications/science/electronics/librepcb/default.nix

+		sha256 = "0r33fm1djqpy0dzvnf5gv2dfh5nj2acaxb7w4cn8yxdgrazjf7ak";
+	};
+
+	enableParallelBuilding=true;


That should be the default for all qmake based projects. See: https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/libraries/qt-5/hooks/qmake-hook.sh#L13

andir · 2018-01-08T23:52:06Z

pkgs/applications/science/electronics/librepcb/default.nix

+
+	enableParallelBuilding=true;
+
+	nativeBuildInputs = [ qmake qttools qtbase ];


Is qtbase really a nativeBuildInput? Wouldn't that be a regular buildInput since it is most likely only used for linking/headers?

andir · 2018-01-08T23:59:15Z

pkgs/applications/science/electronics/librepcb/default.nix

+	name = "librepcb-unstable";
+	version = "2017-12-29";
+	src = fetchgit {
+		url = "git://github.com/LibrePCB/LibrePCB";


use fetchfromGitHub instead

andir · 2018-01-09T00:03:26Z

pkgs/applications/science/electronics/librepcb/default.nix

+
+stdenv.mkDerivation rec {
+	name = "librepcb-unstable";
+	version = "2017-12-29";


usually - should not be part of the version. Everything after the last - denotes the package version.

Also version is not a (formal?) required attribute. Often it is just used as convenient way to share the version string across multiple place within a package.

You should probably rewrite the beginning of your deriviation to look like this:

stdenv.mkDerivation rec { version = "0.git.20171229"; name = "librepcb-${version}"; … }

Mic92 · 2018-01-08T22:45:58Z

pkgs/applications/science/electronics/librepcb/default.nix

+	meta = with stdenv.lib; {
+		description = "A free EDA software to develop printed circuit boards";
+		homepage = http://librepcb.org/;
+		maintainers = with maintainers; [ luz ];


Needs to be added to lib/maintainers.nix as well.

Mic92 · 2018-01-08T22:46:54Z

pkgs/applications/science/electronics/librepcb/default.nix

+	name = "librepcb-unstable";
+	version = "2017-12-29";
+	src = fetchgit {
+		url = "git://github.com/LibrePCB/LibrePCB";


Using https for cloning is the preferred way. I switched this to fetchFromGitHub, which does this.

Mic92 · 2018-01-09T00:05:35Z

pkgs/applications/science/electronics/librepcb/default.nix

+	patches = [ ./fix-2017-12.patch ];
+
+	#recursive qmake building is required -> therefore this line is required (qmake -r)
+	preBuild = "mkdir build && cd build && qmake -r ../librepcb.pro PREFIX=$out QMAKE_LRELEASE=${stdenv.lib.getDev qttools.dev}/bin/lrelease";


this can be simplified by:

qmakeFlags = ["-r"];

Mic92 · 2018-01-09T00:07:10Z

pkgs/applications/science/electronics/librepcb/default.nix

+
+	nativeBuildInputs = [ qmake qttools qtbase ];
+
+	patches = [ ./fix-2017-12.patch ];


Can you rename the patch to make it self-explaining (in example what the patch fixes).

Mic92 · 2018-01-09T00:12:05Z

pkgs/applications/science/electronics/librepcb/default.nix

+{ stdenv, fetchgit, qtbase, qttools, qmake, mesa, openssl, zlib }:
+
+stdenv.mkDerivation rec {
+	name = "librepcb-unstable";


the name should contain the version or a date of last commit, if a software project does make releases

ubruhin · 2018-01-09T17:58:54Z

Should we provide git to extract the current treeish/tag to be displayed during runtime?

It's not that important, but yes, if it's easy to provide the git command at build time, it would be useful because the revision information is displayed at runtime 👍

Mic92 · 2018-01-11T01:29:59Z

@GrahamcOfBorg build librepcb

GrahamcOfBorg

Failure for system: x86_64-darwin

Package ‘librepcb-20171229’ in /tmp/nix-ofborg/repo/38dca4e3aa6bca43ea96d2fcc04e8229/builder/lnl7-mac/pkgs/applications/science/electronics/librepcb/default.nix:27 is not supported on ‘x86_64-darwin’, refusing to evaluate.

a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowBroken = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowBroken = true; }
to ~/.config/nixpkgs/config.nix.

GrahamcOfBorg

Success for system: aarch64-linux

shrinking /nix/store/75v9x9r4kdqvj74ly3bvxbmxjc5rxd4j-librepcb-20171229/bin/librepcb
strip is /nix/store/c6qj0j45xizkrx58i65j75a5ysmqhgrs-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/75v9x9r4kdqvj74ly3bvxbmxjc5rxd4j-librepcb-20171229/bin
patching script interpreter paths in /nix/store/75v9x9r4kdqvj74ly3bvxbmxjc5rxd4j-librepcb-20171229
checking for references to /build in /nix/store/75v9x9r4kdqvj74ly3bvxbmxjc5rxd4j-librepcb-20171229...
postPatchMkspecs
postMoveQtStaticLibs
postPatchMkspecs
postMoveQtStaticLibs
/nix/store/75v9x9r4kdqvj74ly3bvxbmxjc5rxd4j-librepcb-20171229

GrahamcOfBorg

Success for system: x86_64-linux

shrinking /nix/store/cgkskzqk1ffmdr0g97y8cnlq73dk9fqc-librepcb-20171229/bin/librepcb
strip is /nix/store/wxn5gn8amxm1w0ikcx4gbs8a17wvss4j-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/cgkskzqk1ffmdr0g97y8cnlq73dk9fqc-librepcb-20171229/bin 
patching script interpreter paths in /nix/store/cgkskzqk1ffmdr0g97y8cnlq73dk9fqc-librepcb-20171229
checking for references to /tmp/nix-build-librepcb-20171229.drv-0 in /nix/store/cgkskzqk1ffmdr0g97y8cnlq73dk9fqc-librepcb-20171229...
postPatchMkspecs
postMoveQtStaticLibs
postPatchMkspecs
postMoveQtStaticLibs
/nix/store/cgkskzqk1ffmdr0g97y8cnlq73dk9fqc-librepcb-20171229

librepcb-unstable: init at 2017-12-29

ca758f8

Luz force-pushed the librepcb branch from fd4c3bd to ca758f8 Compare January 8, 2018 22:29

das-g reviewed Jan 8, 2018

View reviewed changes

andir requested changes Jan 9, 2018

View reviewed changes

librepcb: use fetchFromGitHub; set qmakeFlags

2c33238

andir added the 8.has: package (new) label Jan 9, 2018

Mic92 reviewed Jan 9, 2018

View reviewed changes

Luz added 3 commits January 11, 2018 00:15

Remove dashes from version

Verified

This commit was signed with the committer’s verified signature.

targos Michaël Zasso

GPG key ID: 770F7A9A5AE15600

Verified
Learn about vigilant mode

c4a6afb

Add a description about the patch

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

900e971

Add luz to maintainer list

d50c3d9

GrahamcOfBorg added 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Jan 10, 2018

GrahamcOfBorg reviewed Jan 11, 2018

View reviewed changes

Mic92 merged commit e056315 into NixOS:master Jan 11, 2018

Luz deleted the librepcb branch January 11, 2018 21:59

Janik-Haag added the 12. first-time contribution label Jun 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

librepcb-unstable: init at 2017-12-29 #33630

librepcb-unstable: init at 2017-12-29 #33630

Luz commented Jan 8, 2018 •

edited

Loading

das-g Jan 8, 2018

andir left a comment

andir Jan 8, 2018

andir Jan 8, 2018

andir Jan 8, 2018

andir Jan 8, 2018

andir Jan 9, 2018

Mic92 Jan 8, 2018

Mic92 Jan 8, 2018

Mic92 Jan 9, 2018

Mic92 Jan 9, 2018

Mic92 Jan 9, 2018

ubruhin commented Jan 9, 2018

Mic92 commented Jan 11, 2018

GrahamcOfBorg left a comment

GrahamcOfBorg left a comment

GrahamcOfBorg left a comment


		enableParallelBuilding=true;

		nativeBuildInputs = [ qmake qttools qtbase ];


		nativeBuildInputs = [ qmake qttools qtbase ];

		patches = [ ./fix-2017-12.patch ];

librepcb-unstable: init at 2017-12-29 #33630

librepcb-unstable: init at 2017-12-29 #33630

Conversation

Luz commented Jan 8, 2018 • edited Loading

Motivation for this change

Things done

Choose a reason for hiding this comment

andir left a 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

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

ubruhin commented Jan 9, 2018

Mic92 commented Jan 11, 2018

GrahamcOfBorg left a comment

Choose a reason for hiding this comment

GrahamcOfBorg left a comment

Choose a reason for hiding this comment

GrahamcOfBorg left a comment

Choose a reason for hiding this comment

Luz commented Jan 8, 2018 •

edited

Loading