sympow: init at 1.018.1

[timokau]

Motivation for this change

Package sympow.

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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/)
• Fits CONTRIBUTING.md.

---

[peterhoeg]

You don't need pname if you're only using it to set the name.

[timokau]

Its more of a style choice, I prefer to have separate attributes for name and version. But I can understand that inconsistency might be a negative here. Do you want me to remove pname?

[peterhoeg]

The standard way of doing this is not to define pname unless it's actively used for something.

[peterhoeg]

please add runHook preConfigure and runHook postConfigure.

[timokau]

Isn't that only needed when preConfigure or postConfigure are actually used? Hooks don't use those right?

[peterhoeg]

If someone needs to override something in a package, that can be done easily through the pre/post hooks. If you don't run them when you override a phase, they cannot be used.

[timokau]

Alright that makes sense. Is there a reason configurePhase etc. are not defined somewhat like this:

configurePhase() { 
	runHook preConfigure
	if [[ -n "$configurePhase" ]]; then
		eval "$configurePhase";
	else
		# what the default configure phase does
	fi
	runHook postConfigure
}

That would remove this necessary boiler plate and would remove confusion about how to execut phases when using `nix-shell.

[peterhoeg]

I'm sure there's a reason - I just don't know what it is! Your suggestion makes perfect sense to me but @vcunat and/or @Ericson2314 might be able to shed some light on the current state of affairs.

[vcunat]

I'm not sure. It's probably just because of history; I suspect it comes from beginnings of nixpkgs. In time it turned out you usually only want to override the inside and not pre/post, and now it's much more difficult to change such basic things.

[peterhoeg]

runHook preInstall and runHook postInstall

[peterhoeg]

you can use install directly instead of mkdir and cp

[peterhoeg]

Instead of a comment, you can pass name = "do_not_hardcode_paths.patch to fetchpatch which also makes it easier to work with.

[timokau]

I added names (but also kept the comments, as they offer more freedom for explanation than a filename).

[peterhoeg]

Documentation is always good!

[infinisil]

Please don't use placeholder in nixpkgs. We need to stay compatible with people using Nix 1.11 for a while. I'm not sure why but this single instance apparently breaks nix-env -i nix-repl, as reported on IRC. This should be fixed ASAP.

[LnL7]

Fixed in bae15c8

[timokau]

Sorry, I didn't know we're still maintaining compatibility. Is that documented somewhere? How long?

[infinisil]

No idea for how long, but ther is a fil sthat specifies the minimum required nix version: https://github.com/NixOS/nixpkgs/blob/master/.version

Edit: whoops, meant https://github.com/NixOS/nixpkgs/blob/master/lib/minver.nix

[LnL7]

Yeah, there's no date for it AFAIK. I personally think we shouldn't until 18.09 unless there's a good reason not to, while placeholders are nice it's generally pretty straightforward to work around the recursion in other ways.