horizon-eda: init at 1.1.1 #86694
horizon-eda: init at 1.1.1 #86694
Conversation
| @@ -0,0 +1,50 @@ | |||
| { stdenv, fetchFromGitHub, pkgconfig, sqlite, libyamlcpp, libuuid, gnome3, epoxy, librsvg, zeromq, cppzmq, glm, libgit2, curl, boost, python3, opencascade, libzip, podofo, wrapGAppsHook, coreutils }: | |||
There was a problem hiding this comment.
Overly long line; should be split into multiple lines and preferably include lib directly rather than through stdenv; I personally prefer one package per line like this:
-{ stdenv, fetchFromGitHub, pkgconfig, sqlite, libyamlcpp, libuuid, gnome3, epoxy, librsvg, zeromq, cppzmq, glm, libgit2, curl, boost, python3, opencascade, libzip, podofo, wrapGAppsHook, coreutils }:
+{ stdenv
+, lib
+, fetchFromGitHub
+, pkgconfig
+, sqlite
+, libyamlcpp
+, libuuid
+, gnome3
+, epoxy
+, librsvg
+, zeromq
+, cppzmq
+, glm
+, libgit2
+, curl
+, boost
+, python3
+, opencascade
+, libzip
+, podofo
+, wrapGAppsHook
+, coreutils
+}:| version = "1.1.0"; | ||
|
|
||
| src = fetchFromGitHub { | ||
| owner = "horizon-eda"; | ||
| repo = "horizon"; | ||
| rev = "v${version}"; | ||
| sha256 = "1p8msmpxr5m2037yri3ml5h737n6cdgz2g817805z9kgilxdc56a"; | ||
| }; |
There was a problem hiding this comment.
New upstream release:
- version = "1.1.0";
+ version = "1.1.1";
- src = fetchFromGitHub {
- owner = "horizon-eda";
- repo = "horizon";
- rev = "v${version}";
- sha256 = "1p8msmpxr5m2037yri3ml5h737n6cdgz2g817805z9kgilxdc56a";
- };
+ src = fetchFromGitHub {
+ owner = "horizon-eda";
+ repo = "horizon";
+ rev = "v${version}";
+ hash = "sha256-gfCD//eiW7zmvVSeIVWos34lOTHqef0k96i4lJ1EzSg=";
+ };| zeromq | ||
| ]; | ||
|
|
||
| nativeBuildInputs = [ pkgconfig wrapGAppsHook coreutils ]; |
There was a problem hiding this comment.
No need for coreutils here. It's implicitly present for every build, and you don't need to reference things in the build inputs to refer to them directly with a ${coreutils}/... path like in the installPhase; Nix tracks the dependencies automatically.
- nativeBuildInputs = [ pkgconfig wrapGAppsHook coreutils ];
+ nativeBuildInputs = [ pkgconfig wrapGAppsHook ];|
|
||
| enableParallelBuilding = true; | ||
|
|
||
| meta = with stdenv.lib; { |
There was a problem hiding this comment.
If depending on lib directly:
- meta = with stdenv.lib; {
+ meta = with lib; {
|
@emilazy I believe I addressed your feedback in the updated commit. |
There was a problem hiding this comment.
Thanks for addressing these! A couple more formatting nitpicks, but other than that this looks good to go to me.
The build failed on 64-bit ARM due to freeimage's vendored libpng; I tried to fix it but got lost in the weeds. It's already tracked in #77653, so I opened #86694 to mark freeimage as broken on AArch64 for now.
| buildInputs = [ | ||
| boost | ||
| cppzmq | ||
| curl | ||
| epoxy | ||
| glm | ||
| gnome3.gtkmm | ||
| libgit2 | ||
| librsvg | ||
| libuuid | ||
| libyamlcpp | ||
| libzip | ||
| opencascade | ||
| podofo | ||
| python3 | ||
| sqlite | ||
| zeromq | ||
| ]; |
There was a problem hiding this comment.
Incorrect indentation and trailing whitespace at the end of lines:
- buildInputs = [
- boost
- cppzmq
- curl
- epoxy
- glm
- gnome3.gtkmm
- libgit2
- librsvg
- libuuid
- libyamlcpp
- libzip
- opencascade
- podofo
- python3
- sqlite
- zeromq
- ];
+ buildInputs = [
+ boost
+ cppzmq
+ curl
+ epoxy
+ glm
+ gnome3.gtkmm
+ libgit2
+ librsvg
+ libuuid
+ libyamlcpp
+ libzip
+ opencascade
+ podofo
+ python3
+ sqlite
+ zeromq
+ ];| installPhase = '' | ||
| make install INSTALL=${coreutils}/bin/install DESTDIR=$out PREFIX= | ||
| ''; |
There was a problem hiding this comment.
Incorrect indentation:
installPhase = ''
- make install INSTALL=${coreutils}/bin/install DESTDIR=$out PREFIX=
+ make install INSTALL=${coreutils}/bin/install DESTDIR=$out PREFIX=
'';
Mostly based on NixOS#56497 by @Luz
|
@emilazy addressed your indentation remarks |
|
Looks good, thanks! |
Mostly based on NixOS#86694 by yrashk
| zeromq | ||
| ]; | ||
|
|
||
| nativeBuildInputs = [ pkgconfig wrapGAppsHook ]; |
There was a problem hiding this comment.
| nativeBuildInputs = [ pkgconfig wrapGAppsHook ]; | |
| nativeBuildInputs = [ pkg-config wrapGAppsHook ]; |
| { stdenv | ||
| , lib | ||
| , fetchFromGitHub | ||
| , pkgconfig |
There was a problem hiding this comment.
| , pkgconfig | |
| , pkg-config |
| { stdenv | ||
| , lib |
There was a problem hiding this comment.
| { stdenv | |
| , lib | |
| { stdenv |
|
|
||
| enableParallelBuilding = true; | ||
|
|
||
| meta = with lib; { |
There was a problem hiding this comment.
| meta = with lib; { | |
| meta = with stdenv.lib; { |
|
Already newer version in nixpkgs. |
Mostly based on #56497 by @Luz
Motivation for this change
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)