nginx-config-formatter: Init at 2016-06-16 #22179
Conversation
| ${pkgs.python3}/bin/python $src/nginxfmt.py "\$@" | ||
| EOF | ||
| chmod a+x $out/bin/nginxfmt | ||
| ''; |
There was a problem hiding this comment.
You can indent this block and it will be dedented automatically in nix.
There was a problem hiding this comment.
or use makeWrapper.
BTW, won't this create runtime dependency on source derivation? Better would be to copy the sole .py file and wrap it.
There was a problem hiding this comment.
If the source derivation only has one file then it's not really a problem to depend on it.
There was a problem hiding this comment.
I agree, consider this as a nitpick.
There was a problem hiding this comment.
makeWrapper, of course. D'oh.
But my original attempt was indented, and I ended up with EOF in the file. Not sure why; possibly tab/space confusion.
Don't have access to the machine this is on right now, though. I'll have another version up tomorrow.
There was a problem hiding this comment.
makeWrapper doesn't seem to let me specify a custom interpreter.
But that's fine. Specifying python3 in buildInputs makes the shebang patching work.
| version = "2016-06-16"; | ||
| name = "nginx-config-formatter-${version}"; | ||
|
|
||
| src = pkgs.fetchgit { |
There was a problem hiding this comment.
fetchFromGitHub would probably be better here.
| @@ -4697,6 +4697,25 @@ in { | |||
| }; | |||
| }; | |||
|
|
|||
| nginx-config-formatter = stdenv.mkDerivation rec { | |||
There was a problem hiding this comment.
If this is a tool and not a general Python library then its expression should be placed into its own directory. For example, pkgs/tools/system/nginx-config-formatter or pkgs/tools/misc/nginx-config-formatter.
|
@Baughn BTW, I really like this idea. I agree the nginx config files are rough. |
|
The commit log currently reflects the PR's history. Want me to squash it, or will you do that? |
|
yes, squash it |
Baughn
force-pushed
the
nginx
branch
2 times, most recently
from
59210da
to
d2cf7ff
Compare
|
Done. |
|
|
||
| stdenv.mkDerivation rec { | ||
| version = "2016-06-16"; | ||
| name = "nginx-config-formatter-${version}"; |
There was a problem hiding this comment.
Sorry, I didn't notice this earlier. If this is not an official release then the name should be nginx-config-formatter-unstable-${version} as per https://nixos.org/nixpkgs/manual/#sec-package-naming.
There was a problem hiding this comment.
I'm of the opinion that if there isn't any stable release that we just drop the unstable. As soon as there will be a release we would likely use that instead of this version. In that case the name remains the same. If you would now append -unstable to the name part, the name changes and it would break nix-env -u.
There was a problem hiding this comment.
Yeah, but later when, e.g., version 1.0 is released I believe nix-env -u will break anyway since 1.0 < 2016-06-16 and the user would have to manually force a package update.
There was a problem hiding this comment.
@rycee you're right
https://nixos.org/nix/manual/#versions
I thought Nix didn't version comparisons but it does.
There was a problem hiding this comment.
If I understand correctly it will check the version if you installed the package using the package name (nix-env -i xyz) and not the attribute name (nix-env -iA xyz).
I'm not sure, though. I don't use the nix-env -u mechanism at all so I've never been bitten by this.
In any case I think it is best to change the package name now when very few will be affected. It'll be more painful over time.
Motivation for this change
I'd like to automatically format the nginx config file, when generated using structured config. At present the result is quite unreadable.
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandboxinnix.confon non-NixOS)
nix-shell -p nox --run "nox-review wip"./result/bin/)