20kly: init at 1.4 #45056

fgaz · 2018-08-15T09:17:43Z

Motivation for this change

The classic "20,000 Light-Years Into Space" game

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)
- +1.3M
Fits CONTRIBUTING.md.

mpickering · 2018-08-15T10:44:38Z

@GrahamcOfBorg build 20kly

GrahamcOfBorg · 2018-08-15T10:44:45Z

No attempt on x86_64-linux (full log)

The following builds were skipped because they don't evaluate on x86_64-linux: 20kly

Partial log (click to expand)

Cannot nix-instantiate `20kly' because:
error: attribute '20kly' in selection path '20kly' not found

GrahamcOfBorg · 2018-08-15T10:44:57Z

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: 20kly

Partial log (click to expand)

Cannot nix-instantiate `20kly' because:
error: attribute '20kly' in selection path '20kly' not found

GrahamcOfBorg · 2018-08-15T10:45:10Z

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: 20kly

Partial log (click to expand)

Cannot nix-instantiate `20kly' because:
error: attribute '20kly' in selection path '20kly' not found

timokau · 2018-08-15T10:54:17Z

pkgs/games/20kly/default.nix

@@ -0,0 +1,37 @@
+{ stdenv, fetchurl, python2Packages }:


This should be python instead of python2Packages. The packages can then be accessed with python.pkgs. If it is not python3 compatible, you should add disabled = !isPy3.

Another one of my style preferences: I'd format the argument list like this:

{ stdenv , fetchurl , python2Packages }:

Because of the same reasons as the list formatting in #45057.

timokau · 2018-08-15T10:56:37Z

pkgs/games/20kly/default.nix

+        "LIGHTYEARS_DIR = \"$out/share\""
+  '';
+
+  propagatedBuildInputs = with python2Packages; [ python pygame ];


Again, I don't think python belongs here.

timokau · 2018-08-15T10:57:00Z

pkgs/games/20kly/default.nix

+  buildPhase = "python -O -m compileall .";
+
+  installPhase = ''
+    mkdir -p $out/share


Please quote shell variables.

timokau · 2018-08-15T10:57:25Z

pkgs/games/20kly/default.nix

+
+  meta = with stdenv.lib; {
+    description = "A steampunk-themed strategy game where you have to manage a steam supply network";
+    homepage = "http://jwhitham.org.uk/20kly/";


Quoting unnecessary

fgaz · 2018-08-15T13:31:57Z

@timokau all done

@mpickering it's _20kly, since it starts with a number

timokau · 2018-08-15T22:18:39Z

pkgs/games/20kly/default.nix

+  pname = "20kly";
+  version = "1.4";
+  format = "other";
+  disabled = !(python.isPy3 or true);


Any reason for the or true here?

On python2 it's null

x or y should be sugar for if x==null then y else x, but I can't find where it's documented, if it's documented at all

On python2 it's null

Oh? That doesn't seem right.

And what do you want to express? Shouldn't it be disabled = !isPy2, since you want to disable for everything that isn't python 2?

I don't see anybody else using or null. Does it make a difference if you add isPy2 to the argument list instead of using python.isPy2?

Did you try using isPyX from the argument list instead?

isn't that a callPythonPackage-specific param?

Maybe. Why aren't you using that?

Sorry, that doesn't even exist 😅. I meant python-packages.nix-specific, which I don't use because this is an application

Well there is python.pkgs.callPackage (which is used in python-packages.nix). But outside of python-packages you'd have to choose which python to use. Which makes the point about disabled kind of moot.

TLDR: I'm probably just not qualified to review a python application, sorry for the noise. I hope @domenkozar will add his opinion.

timokau · 2018-08-15T22:19:26Z

@GrahamcOfBorg build _20kly

GrahamcOfBorg · 2018-08-15T22:19:45Z

Success on x86_64-linux (full log)

Attempted: _20kly

Partial log (click to expand)

post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/gxwg46gyh4430vamqs9vql3q6q452i8d-20kly-1.4
strip is /nix/store/gpc2wld1s0c6qzx9326cwn1wcx29xzsj-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/gxwg46gyh4430vamqs9vql3q6q452i8d-20kly-1.4/bin
patching script interpreter paths in /nix/store/gxwg46gyh4430vamqs9vql3q6q452i8d-20kly-1.4
/nix/store/gxwg46gyh4430vamqs9vql3q6q452i8d-20kly-1.4/share/lightyears: interpreter directive changed from "/usr/bin/python" to "/nix/store/9qjc0d5557h3dkpsvfq98b2k96lnb6pw-python-2.7.15/bin/python"
/nix/store/gxwg46gyh4430vamqs9vql3q6q452i8d-20kly-1.4/bin/lightyears: interpreter directive changed from "/usr/bin/python" to "/nix/store/9qjc0d5557h3dkpsvfq98b2k96lnb6pw-python-2.7.15/bin/python"
checking for references to /build in /nix/store/gxwg46gyh4430vamqs9vql3q6q452i8d-20kly-1.4...
wrapping `/nix/store/gxwg46gyh4430vamqs9vql3q6q452i8d-20kly-1.4/bin/lightyears'...
/nix/store/gxwg46gyh4430vamqs9vql3q6q452i8d-20kly-1.4

GrahamcOfBorg · 2018-08-15T22:19:59Z

Success on aarch64-linux (full log)

Attempted: _20kly

Partial log (click to expand)

post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/0yr9r6h8qnspdb0nvnphy0l0gnbcs8la-20kly-1.4
strip is /nix/store/ah0va6j4cnwj9nx4j6rwcfc8nh785jwm-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/0yr9r6h8qnspdb0nvnphy0l0gnbcs8la-20kly-1.4/bin
patching script interpreter paths in /nix/store/0yr9r6h8qnspdb0nvnphy0l0gnbcs8la-20kly-1.4
/nix/store/0yr9r6h8qnspdb0nvnphy0l0gnbcs8la-20kly-1.4/bin/lightyears: interpreter directive changed from "/usr/bin/python" to "/nix/store/9gfk6klv3zajmibdsvn4yhz20z6vg5gw-python-2.7.15/bin/python"
/nix/store/0yr9r6h8qnspdb0nvnphy0l0gnbcs8la-20kly-1.4/share/lightyears: interpreter directive changed from "/usr/bin/python" to "/nix/store/9gfk6klv3zajmibdsvn4yhz20z6vg5gw-python-2.7.15/bin/python"
checking for references to /build in /nix/store/0yr9r6h8qnspdb0nvnphy0l0gnbcs8la-20kly-1.4...
wrapping `/nix/store/0yr9r6h8qnspdb0nvnphy0l0gnbcs8la-20kly-1.4/bin/lightyears'...
/nix/store/0yr9r6h8qnspdb0nvnphy0l0gnbcs8la-20kly-1.4

timokau · 2018-08-15T22:21:31Z

Thanks for the changes!

Actually, should this be in python-packages? @domenkozar

GrahamcOfBorg · 2018-08-15T23:17:20Z

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: _20kly

Partial log (click to expand)


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

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

fgaz · 2018-08-16T09:18:17Z

@timokau Nope, since it isn't a library (see the nixpkgs docs, python section)

timokau · 2018-08-16T20:49:51Z

I just noticed that I pinged the wrong person, I meant @dotlambda.

GrahamcOfBorg added 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Aug 15, 2018

timokau requested changes Aug 15, 2018

View reviewed changes

fgaz force-pushed the 20kly branch 2 times, most recently from 0db6543 to 40e6745 Compare August 15, 2018 13:30

timokau reviewed Aug 15, 2018

View reviewed changes

timokau approved these changes Aug 15, 2018

View reviewed changes

20kly: init at 1.4

de5c986

fgaz force-pushed the 20kly branch from 40e6745 to de5c986 Compare August 16, 2018 20:23

xeji merged commit 0fd58ed into NixOS:master Aug 26, 2018

fgaz deleted the 20kly branch March 27, 2021 17:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

20kly: init at 1.4 #45056

20kly: init at 1.4 #45056

fgaz commented Aug 15, 2018

mpickering commented Aug 15, 2018

GrahamcOfBorg commented Aug 15, 2018

GrahamcOfBorg commented Aug 15, 2018

GrahamcOfBorg commented Aug 15, 2018

timokau Aug 15, 2018

timokau Aug 15, 2018

timokau Aug 15, 2018

timokau Aug 15, 2018

timokau Aug 15, 2018

fgaz commented Aug 15, 2018

timokau Aug 15, 2018

fgaz Aug 16, 2018

fgaz Aug 16, 2018

timokau Aug 16, 2018

timokau Aug 16, 2018

timokau Aug 16, 2018

fgaz Aug 16, 2018

timokau Aug 16, 2018

fgaz Aug 16, 2018

timokau Aug 16, 2018

timokau commented Aug 15, 2018

GrahamcOfBorg commented Aug 15, 2018

GrahamcOfBorg commented Aug 15, 2018

timokau commented Aug 15, 2018

GrahamcOfBorg commented Aug 15, 2018

fgaz commented Aug 16, 2018

timokau commented Aug 16, 2018

20kly: init at 1.4 #45056

20kly: init at 1.4 #45056

Conversation

fgaz commented Aug 15, 2018

Motivation for this change

Things done

mpickering commented Aug 15, 2018

GrahamcOfBorg commented Aug 15, 2018

GrahamcOfBorg commented Aug 15, 2018

GrahamcOfBorg commented Aug 15, 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

fgaz commented Aug 15, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timokau commented Aug 15, 2018

GrahamcOfBorg commented Aug 15, 2018

GrahamcOfBorg commented Aug 15, 2018

timokau commented Aug 15, 2018

GrahamcOfBorg commented Aug 15, 2018

fgaz commented Aug 16, 2018

timokau commented Aug 16, 2018