AntTweakBar: init at 1.16 #34540
AntTweakBar: init at 1.16 #34540
Conversation
| buildInputs = [ unzip xlibs.libX11 mesa ]; | ||
|
|
||
| src = fetchurl { | ||
| url = "https://vorboss.dl.sourceforge.net/project/anttweakbar/AntTweakBar_116.zip"; |
There was a problem hiding this comment.
In general we use mirror://sourceforge/ instead of hardcoding individual SF mirrors.
There was a problem hiding this comment.
I ran into this issue: NixOS/nix#969 (comment)
There was a problem hiding this comment.
Oh well, the most fun case is with some tarballs where you can have a completely different name (but the same hash) and it will fetch from hash-addressed tarballs.nixos.org…
| (compatibility and core profiles), DirectX 9, DirectX 10 or DirectX 11 | ||
| to interactively tweak parameters on-screen | ||
| ''; | ||
| homepage = http://anttweakbar.sourceforge.net/doc/; |
There was a problem hiding this comment.
I would call http://anttweakbar.sourceforge.net/ the project homepage…
| to interactively tweak parameters on-screen | ||
| ''; | ||
| homepage = http://anttweakbar.sourceforge.net/doc/; | ||
| license = [ stdenv.lib.licenses.libpng stdenv.lib.licenses.zlib ]; |
There was a problem hiding this comment.
Maybe just call it zlib license? I have a feeling that license-as-a-list is not used consistently, and is confusing in some cases.
There was a problem hiding this comment.
I would prefer to keep both since that is what is specified by the author. But I'll remove it if you say it is ok.
There was a problem hiding this comment.
I think the point is that zlib and libpng is approximately the same license.
https://opensource.org/licenses/Zlib
In case where two licenses make sense, a comment to explain the relation (we don't have a real consensus if it is «portions like this, portions like that» or «at your option») would be nice. But here you can just pick one.
| postPatch = "cd src"; | ||
| installPhase = '' | ||
| mkdir -p $out/lib/ | ||
| cp -vi ../lib/{libAntTweakBar.so,libAntTweakBar.so.1,libAntTweakBar.a} $out/lib/ |
There was a problem hiding this comment.
Any reason for -vi? Verbose copying is generally not used in Nixpkgs for installation; and you are copying to an empty directory anyway.
There was a problem hiding this comment.
No reason. I was helped by others to create the nix expression and I didn't think it through enough. Will remove.
|
Could you please squash and force-push to the PR branch? |
|
@7c6f434c Will do |
|
By the way, I would leave the empty sandboxing checkbox. Also, almost noone does the cleanup, so many reviewers are used to having the checkboxes that are irrelevant to your case. (of course, not actually a problem for this PR, but in future you might want to know) |
|
@7c6f434c Squashed and force pushed. |
| ''; | ||
| homepage = http://anttweakbar.sourceforge.net/; | ||
| license = stdenv.lib.licenses.zlib; | ||
| maintainers = stdenv.lib.maintainers.razvan; |
There was a problem hiding this comment.
Oh right, borg is right: this should be a list of strings.
There was a problem hiding this comment.
@7c6f434c Darn, and I thought I was being smart about it by making it non-list :( Makes sense though to be a list
There was a problem hiding this comment.
@7c6f434c Ironically platforms does not have to be a list haha
There was a problem hiding this comment.
They do; but Linux is not a single platform, it is a list of at least three different ones for practical purposes (i686, x86_64, aarch64), and then there are a few 32-bit ARM ones and a 64-bit MIPS one.
There was a problem hiding this comment.
@7c6f434c So it is ok the way it is now, right?
| @@ -0,0 +1,33 @@ | |||
| { stdenv, fetchurl, unzip, xlibs, mesa }: | |||
There was a problem hiding this comment.
By the way, in general you can ask for libX11 and callPackage will find it in xlibs for you. Top-level callPackage is configured to search top-level and xlibs (but nowhere else)
Motivation for this change
This is a lib dependency for Lamdu IDE which I will try to package fully.
Things done
Thanks to takoa, cleverca22, puffnfresh and others from #nixos IRC for helping to package this.