kisslicer: init at 1.6.2 #30595
kisslicer: init at 1.6.2 #30595
Conversation
| homepage = http://www.kisslicer.com; | ||
| license = licenses.unfree; | ||
| maintainers = [ maintainers.cransom ]; | ||
| platforms = platforms.linux; |
There was a problem hiding this comment.
Based on the download URL I guess this should be platforms = "x86_64-linux";.
| , makeWrapper | ||
| , stdenv | ||
| , unzip | ||
| , inidir ? "\\\$HOME/.config/kisslicer" |
There was a problem hiding this comment.
I'd suggest to either change the default here to ${XDG_CONFIG_HOME:-$HOME/.config}/kisslicer or to always use this, i.e., not to have an inidir parameter.
| in stdenv.mkDerivation rec { | ||
| inherit version name; | ||
|
|
||
| src = fetchurl { |
There was a problem hiding this comment.
I'd suggest to use
src = fetchzip {
url = "http://www.kisslicer.com/uploads/1/5/3/8/15381852/kisslicer_linux64_1.6.2_release.zip";
sha256 = "blahblahblah";
stripRoot = false;
};
since then you can remove the custom unpackPhase.
There was a problem hiding this comment.
I've switched to it. I'm not a big fan of the behavior as I have to unpack it in order to get the SHA (though yes, there's nix-prefetch-url -A) but it drops the unpackphase.
There was a problem hiding this comment.
Hmm, I don't think you'll need to worry about the unpack phase since fetchzip extracts the archive after the fetch. Try removing the phases = [ "unpackPhase" "installPhase" "fixupPhase" ]; line. It should still work fine.
There was a problem hiding this comment.
It does work without it, but is there value in skipping phases that are a noop? There isn't a configure script or Makefile so it cuts down a small amount of logging that isn't pertinent to the build.
There was a problem hiding this comment.
Generally speaking it is best not to override the phases field since it is controlling fairly low level stuff within stdenv and it is easy to miss something. The recommendation I've seen is to instead disable the phases that are not needed. In this case to have
configurePhase = ":";
dontBuild = true;
instead of phases = [ … ];. Don't ask me why there is no dontConfigure.
In any case, I believe the phases you have set is OK so I'll merge as is. Can also sort it out later if any problem ever arises.
| ]; | ||
|
|
||
| in stdenv.mkDerivation rec { | ||
| inherit version name; |
There was a problem hiding this comment.
The name should include the version. Best to replace this line with
name = "kisslicer-1.6.2";
No need for a version field since it's not used anywhere.
| makeWrapper | ||
| unzip | ||
| ]; | ||
| installPhase = '' |
There was a problem hiding this comment.
The convention is to have empty lines between the attributes.
|
Rebased to master in b62992a. Thanks for the good work! |
|
Thanks for the pointers.
…On Thu, Nov 2, 2017 at 1:01 PM, Robert Helgesson ***@***.***> wrote:
Rebased to master in b62992a
<b62992a>.
Thanks for the good work!
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#30595 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AB3drWxMpSrTMhM4D7oGqtcUGTqZ2fbaks5syfV_gaJpZM4P_-gJ>
.
|
Motivation for this change
I'd like to use my favorite 3d printing slicer under NixOS
Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)