endgame-singularity: init at 0.30c #45057

fgaz · 2018-08-15T10:09:20Z

Motivation for this change

A game about strong AI

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

fgaz · 2018-08-15T10:10:18Z

pkgs/games/endgame-singularity/default.nix

+  # Tell it where to find python libraries
+  # Also cd to the same directory as the code, since it uses relative paths
+  postFixup = ''
+  makeWrapper "$out/share/singularity.py" "$out/bin/singularity" \


Is the exe name ok or should I change it to endgame-singularity? Most distros and the source itself use just singularity

Since you already add a wrapper here, you could just merge this with the shebang change.

I think the name is fine, especially if the official docs and other distros do the same.

We do already have one other package with the same name: https://github.com/singularityware/singularity

But I guess conflicts can't be entirely avoided. Its your call. Changing the name from the upstream default might cause confusion.

fgaz · 2018-08-15T10:13:09Z

pkgs/games/endgame-singularity/default.nix

+  postPatch = ''
+    echo '#!${python2Packages.python}/bin/python'\
+         $'\n'$(cat singularity-${version}/singularity.py)\
+         > singularity-${version}/singularity.py


Will this cause problems? Should I use sponge?

I don't know sponge. Maybe it would be easier and nicer to just add a wrapper script endgame-singularity that just calls ${python2Packages.python}/bin/python $out/bin/singularity.py or something?

I managed to do it nicely with --add-flags

timokau · 2018-08-15T10:38:12Z

pkgs/games/endgame-singularity/default.nix

+  format = "other";
+
+  srcs =
+    [


Small style nitpick: I'd prefer if you included the opening ´[in the line before. Sosrcs = [`. But since we don't have an official style guide and pretty much anybody just does whatever they want, you can keep it the way it is if you prefer.

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

pkgs/games/endgame-singularity/default.nix

+    ];
+  sourceRoot = ".";
+
+  buildInputs = [ unzip ]; # The music is zipped


This should be nativeBuildInputs. The difference is that nativeBuildInputs are used exclusively at build time. That is important for cross-compilation.

Similar to above comment, my style preference is one line per item for most lists. It makes for cleaner diffs when dependencies change and easier reordering.

Also better for adding comments to individual items.

timokau · 2018-08-15T10:41:01Z

pkgs/games/endgame-singularity/default.nix

+  sourceRoot = ".";
+
+  buildInputs = [ unzip ]; # The music is zipped
+  propagatedBuildInputs = with python2Packages; [ python pygame numpy ];


I don't think python should be in this list.

timokau · 2018-08-15T10:43:09Z

pkgs/games/endgame-singularity/default.nix

+  postPatch = ''
+    echo '#!${python2Packages.python}/bin/python'\
+         $'\n'$(cat singularity-${version}/singularity.py)\
+         > singularity-${version}/singularity.py


I don't know sponge. Maybe it would be easier and nicer to just add a wrapper script endgame-singularity that just calls ${python2Packages.python}/bin/python $out/bin/singularity.py or something?

timokau · 2018-08-15T10:43:58Z

pkgs/games/endgame-singularity/default.nix

+  '';
+
+  installPhase = ''
+    install -Dm755 singularity-${version}/singularity.py  $out/share/singularity.py


Please quote all shell variables, including $out in these couple of lines.

timokau · 2018-08-15T10:46:33Z

pkgs/games/endgame-singularity/default.nix

+  # Tell it where to find python libraries
+  # Also cd to the same directory as the code, since it uses relative paths
+  postFixup = ''
+  makeWrapper "$out/share/singularity.py" "$out/bin/singularity" \


Since you already add a wrapper here, you could just merge this with the shebang change.

I think the name is fine, especially if the official docs and other distros do the same.

timokau · 2018-08-15T10:48:34Z

pkgs/games/endgame-singularity/default.nix

+  # Tell it where to find python libraries
+  # Also cd to the same directory as the code, since it uses relative paths
+  postFixup = ''
+  makeWrapper "$out/share/singularity.py" "$out/bin/singularity" \


We do already have one other package with the same name: https://github.com/singularityware/singularity

But I guess conflicts can't be entirely avoided. Its your call. Changing the name from the upstream default might cause confusion.

timokau · 2018-08-15T10:50:23Z

pkgs/games/endgame-singularity/default.nix

+
+  meta = {
+    homepage = "http://www.emhsoft.com/singularity/";
+    description = "A simulation of a true AI. Go from computer to computer, pursued by the entire world. Keep hidden, and you might have a chance";


This is a bit too long for description. Please split it into description and longDescription. It looks like the first sentence may be a good description, the rest may be longDescription. description should not have a dot at the end. Maybe use A game about a true AI to make clear it is a game.

timokau · 2018-08-15T10:50:39Z

pkgs/games/endgame-singularity/default.nix

+  '';
+
+  meta = {
+    homepage = "http://www.emhsoft.com/singularity/";


The quoting is unnecessary here, nix has a URL type.

fgaz · 2018-08-15T14:12:56Z

@timokau all done

timokau

Thanks for making the changes :)

timokau · 2018-08-15T22:28:29Z

pkgs/games/endgame-singularity/default.nix

+  # Also cd to the same directory as the code, since it uses relative paths
+  postFixup = ''
+  makeWrapper "${python}/bin/python" "$out/bin/endgame-singularity" \
+        --set PYTHONPATH "$PYTHONPATH" \


Setting pythonpath should not be necessary with buildPythonApplication. Does it fail without that?

buildPythonApplication wraps exes only in certain cases. This is not one of them, since the package type is other and we don't have anything in $out/bin

Maybe it would be a little more beautiful to just symlink $out/share/singularity.py to $out/bin/endgame-singularity in the installPhase and then add --run ... to makeWrapperArgs. Would that work? However, I think it's also ok as it is.

This use of makeWrapper is officially documented in the nixpkgs manual. The symlink does not work since imports are relative

timokau · 2018-08-15T22:29:16Z

pkgs/games/endgame-singularity/default.nix

+  makeWrapper "${python}/bin/python" "$out/bin/endgame-singularity" \
+        --set PYTHONPATH "$PYTHONPATH" \
+        --run "cd \"$out/share\"" \
+        --add-flags "$out/share/singularity.py"


Out of curiosity (not familiar with that flag): What does ´--add-flags` actually do?

Well... it adds a flag :)

Note that the wrapped exe isn't anymore singularity.py. Now I wrap the python exe and tell it what to execute using --add-flag.

Oh, I didn't notice that. But it is unfortunate if that defeats buildPythonPkgs automatic patching. PYTHONPATH is also less performant than buildPythonEnv, although that probably doesn't matter much with a small amount of dependencies.

Well, not that much of a loss since we have to wrap anyway

timokau · 2018-08-15T22:29:38Z

pkgs/top-level/all-packages.nix

@@ -19578,6 +19578,8 @@ with pkgs;

  EmptyEpsilon = callPackage ../games/empty-epsilon { };

+  endgame-singularity = callPackage ../games/endgame-singularity { };
+


Same here: @domenkozar should this be in python-packages?

Same here too :)

edit: I'll copy it here for reference too: Nope, since it isn't a library (see the nixpkgs docs, python section)

Actually I thought the same applied for the python2Packages←→python.pkgs+disabled=!isPy3 thing, but apparently I was wrong?

Very possible I'm the one who is wrong, @domenkozar should know better.

Same here, I actually meant to ping you @dotlambda. Somehow the matching first two letters got me confused 😁

@fgaz is right.

Is there a reason not to call this singularity as most distros are doing? https://repology.org/metapackage/singularity/versions
The executable should probably also be renamed: https://www.archlinux.org/packages/community/any/singularity/files/

Singularity is taken:

Attribute name: nixpkgs.singularity Package name: singularity Version: 2.5.1 Description: Designed around the notion of extreme mobility of compute and reproducible science, Singularity enables users to have full control of their operating system environment

For the same reason I also changed the exe name (not that bad since an exe name equal to the pkg name is not surprising). Should I revert it to singularity?

No, seems reasonable.

dotlambda · 2018-08-26T01:13:57Z

pkgs/games/endgame-singularity/default.nix

+  # Tell it where to find python libraries
+  # Also cd to the same directory as the code, since it uses relative paths
+  postFixup = ''
+  makeWrapper "${python}/bin/python" "$out/bin/endgame-singularity" \


indentation

and please use python.interpreter

dotlambda · 2018-08-26T01:26:40Z

pkgs/games/endgame-singularity/default.nix

@@ -0,0 +1,57 @@
+{ stdenv, fetchurl, unzip, python }:


Please make this use python2 explicitly.

dotlambda · 2018-08-26T01:27:08Z

pkgs/games/endgame-singularity/default.nix

+  pname = "endgame-singularity";
+  version = "0.30c";
+  format = "other";
+  disabled = !(python.isPy3 or true);


GrahamcOfBorg · 2018-08-26T12:34:36Z

Success on aarch64-linux (full log)

Attempted: endgame-singularity

Partial log (click to expand)

Compiling singularity-0.30c/utils/make-tree.py ...
Compiling singularity-0.30c/utils/reorder.py ...
Listing singularity-0.30c/utils/traduko ...
installing
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/gk6060qh6l8j5j1vl4v11k7407n89vff-endgame-singularity-0.30c
strip is /nix/store/y4ymnvgxygpq05h03kyzbj572zmh6zla-binutils-2.30/bin/strip
patching script interpreter paths in /nix/store/gk6060qh6l8j5j1vl4v11k7407n89vff-endgame-singularity-0.30c
checking for references to /build in /nix/store/gk6060qh6l8j5j1vl4v11k7407n89vff-endgame-singularity-0.30c...
/nix/store/gk6060qh6l8j5j1vl4v11k7407n89vff-endgame-singularity-0.30c

GrahamcOfBorg · 2018-08-26T12:53:59Z

Success on x86_64-linux (full log)

Attempted: endgame-singularity

Partial log (click to expand)

Compiling singularity-0.30c/utils/make-tree.py ...
Compiling singularity-0.30c/utils/reorder.py ...
Listing singularity-0.30c/utils/traduko ...
installing
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/vpq8bcb5iibskim6q638fhr8nak71425-endgame-singularity-0.30c
strip is /nix/store/h0lbngpv6ln56hjj59i6l77vxq25flbz-binutils-2.30/bin/strip
patching script interpreter paths in /nix/store/vpq8bcb5iibskim6q638fhr8nak71425-endgame-singularity-0.30c
checking for references to /build in /nix/store/vpq8bcb5iibskim6q638fhr8nak71425-endgame-singularity-0.30c...
/nix/store/vpq8bcb5iibskim6q638fhr8nak71425-endgame-singularity-0.30c

dotlambda · 2018-08-26T16:37:42Z

pkgs/games/endgame-singularity/default.nix

+  # This is not an error: it needs both compilation rounds
+  buildPhase = ''
+    python -m compileall "singularity-${version}"
+    python -O -m compileall "singularity-${version}"


python2.interpreter

GrahamcOfBorg · 2018-08-26T17:34:44Z

Success on aarch64-linux (full log)

Attempted: endgame-singularity

Partial log (click to expand)

Compiling singularity-0.30c/utils/make-tree.py ...
Compiling singularity-0.30c/utils/reorder.py ...
Listing singularity-0.30c/utils/traduko ...
installing
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/rjc9aqfgkz8asfc8mn4x0wj8ww4z6rzf-endgame-singularity-0.30c
strip is /nix/store/y4ymnvgxygpq05h03kyzbj572zmh6zla-binutils-2.30/bin/strip
patching script interpreter paths in /nix/store/rjc9aqfgkz8asfc8mn4x0wj8ww4z6rzf-endgame-singularity-0.30c
checking for references to /build in /nix/store/rjc9aqfgkz8asfc8mn4x0wj8ww4z6rzf-endgame-singularity-0.30c...
/nix/store/rjc9aqfgkz8asfc8mn4x0wj8ww4z6rzf-endgame-singularity-0.30c

GrahamcOfBorg · 2018-08-26T17:34:53Z

Success on x86_64-linux (full log)

Attempted: endgame-singularity

Partial log (click to expand)

Compiling singularity-0.30c/utils/make-tree.py ...
Compiling singularity-0.30c/utils/reorder.py ...
Listing singularity-0.30c/utils/traduko ...
installing
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/vbx3zbiq1lmdnvs0ra7366s3n68vakfi-endgame-singularity-0.30c
strip is /nix/store/h0lbngpv6ln56hjj59i6l77vxq25flbz-binutils-2.30/bin/strip
patching script interpreter paths in /nix/store/vbx3zbiq1lmdnvs0ra7366s3n68vakfi-endgame-singularity-0.30c
checking for references to /build in /nix/store/vbx3zbiq1lmdnvs0ra7366s3n68vakfi-endgame-singularity-0.30c...
/nix/store/vbx3zbiq1lmdnvs0ra7366s3n68vakfi-endgame-singularity-0.30c

fgaz commented Aug 15, 2018

View reviewed changes

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

timokau mentioned this pull request Aug 15, 2018

20kly: init at 1.4 #45056

Merged

9 tasks

fgaz force-pushed the endgame-singularity branch 2 times, most recently from 141a1b8 to 93116db Compare August 15, 2018 14:11

timokau reviewed Aug 15, 2018

View reviewed changes

dotlambda reviewed Aug 26, 2018

View reviewed changes

fgaz force-pushed the endgame-singularity branch from 93116db to c97e7e8 Compare August 26, 2018 12:21

dotlambda reviewed Aug 26, 2018

View reviewed changes

endgame-singularity: init at 0.30c

46fcc11

fgaz force-pushed the endgame-singularity branch from c97e7e8 to 46fcc11 Compare August 26, 2018 17:20

dotlambda merged commit ff583d8 into NixOS:master Aug 26, 2018

fgaz deleted the endgame-singularity branch August 26, 2018 17:42

		@@ -19578,6 +19578,8 @@ with pkgs;

		EmptyEpsilon = callPackage ../games/empty-epsilon { };

		endgame-singularity = callPackage ../games/endgame-singularity { };

endgame-singularity: init at 0.30c #45057

endgame-singularity: init at 0.30c #45057

Conversation

fgaz commented Aug 15, 2018

Motivation for this change

Things done

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

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

timokau 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

fgaz Aug 16, 2018 • edited

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

dotlambda Aug 26, 2018 • edited

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

GrahamcOfBorg commented Aug 26, 2018

GrahamcOfBorg commented Aug 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GrahamcOfBorg commented Aug 26, 2018

GrahamcOfBorg commented Aug 26, 2018

fgaz Aug 16, 2018 •

edited

dotlambda Aug 26, 2018 •

edited