korrect: init at 0.1.3

[dwt]

Things done

Added the tool korrect which helps with always using the correct version of kubectl (and downloading that version when necessary) when talking to kubernetes clusters. This is necessary, because there can be at most a 3 version split between between the version of kubectl (on your system) and the version of the kubernetes cluster.

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested basic functionality of all binary files (usually in ./result/bin/)
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[dwt]

We're seeing some race conditions in the test suite of the project. Currently working with the maintainer to fix those and then promoting this from a draft to a real pull request.

[dwt]

This is ready for review. I'm still in conversation with upstream to investigate the issue on x86_64-darwin, but we've decided that this really does not need to prevent releasing this, as there will likely be very few users on that platform.

Will do an update as soon as we have figured out why it fails there.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/5444

[afh]

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 403417

---

aarch64-darwin

✅ 1 package built:

• korrect

[afh]

Overall looks good, thanks for the contribution and your willingness to become maintainer of this new package, @dwt, very much appreciated.

I've left a few nit comments inline for you to accept or disregard.
Apart from that please double check that:

• the commit adding you to the maintainers list comes before the commit that init's korrect
• the capitalisation of korrect on the commit marking it broken on x86_64 is correct

[nicolas-goudry]

Reviewed points

• package path fits guidelines
• package name fits guidelines
• package version fits guidelines
• package builds on x86_64-linux
• executables tested on x86_64-linux
• meta.description is set and fits guidelines
• meta.license fits upstream license
• meta.platforms is set
• meta.maintainers is set
• meta.mainProgram is set, if applicable.
• source is fetched using the appropriate function
• the list of phases is not overridden

Comments

1. Make sure that the commit adding yourself as a maintainer comes first in the commits list

2. Make sure to only include two commits in your PR: one adding yourself to the maintainers list and the other adding the package.

3. Using nix-init, the following is added to buildInputs:

  buildInputs = lib.optionals stdenv.isDarwin [
    darwin.apple_sdk.frameworks.Security
  ];

Not sure if and why this is needed, just wanted to mention it.

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review

---

x86_64-linux

✅ 1 package built:

• korrect

[nicolas-goudry]

Consider auto-installing shell completions.

[dwt]

@nicolas-goudry Regarding

buildInputs = lib.optionals stdenv.isDarwin [
  darwin.apple_sdk.frameworks.Security
];

In our testing this wasn't necessary, so I am a bit hesitant to just add it. I will keep this in mind should the need come up though.

[nicolas-goudry]

1. Make sure that the commit adding yourself as a maintainer comes first in the commits list

2. Make sure to only include two commits in your PR: one adding yourself to the maintainers list and the other adding the package.

Otherwise LGTM.

[dwt]

@nicolas-goudry I did squash the commits as you suggested, hope this makes this ready to merge.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/2369

[dwt]

0.1.3 finally unbroke x86-darwin. Yeah. :)

[cromulentbanana]

0.1.3 finally unbroke x86-darwin. Yeah. :)

I think the checkbox needs to be checked now :-)

[Aleksanaa]

lgtm except mainProgram

[dwt]

@Aleksanaa

lgtm except mainProgram

Why do you value that entry? The app is named the same as the package, so the default works fine to allow

❯ nix run .#korrect -- --help
A kubectl version managing shim that invokes the correct kubectl version ☸

Usage: korrect [COMMAND]

Commands:
  completions  Generates shell completions
  setup        Installs the korrect-shim and creates the cache
  list         Lists the installed components
  help         Print this message or the help of the given subcommand(s)

Options:
  -h, --help     Print help
  -V, --version  Print version

What is the value to you to additionally hardcode the name of the main program again?

[Aleksanaa]

The documentation has explicitly said that mainProgram has to be set whenever there is a main executable:
https://github.com/NixOS/nixpkgs/tree/master/pkgs#meta-attributes

lib.getExe will raise a warning when mainProgram isn't specified explicitly:
https://github.com/NixOS/nixpkgs/blob/master/lib/meta.nix#L404-L444

This best describes why we don't want to rely on pname, although it is correct from time to time:
https://discourse.nixos.org/t/lib-getexe-best-practices-discussion-thread/31290/5

[dwt]

@Aleksanaa Many thanks for educating me. [x] fixed.

[matthiasbeyer]

@dwt it appears that your commit in this PR was made with a bogus author setting in git, at least that's what I see in my git-log output. I don't want to be nitpicking here, but could you please double check that the setting is correct for you?
For example, your commit fails to render properly in the tig graph overview, which kinda makes me suspicious 👀

Here's a screenshot:

[Aleksanaa]

https://github.com/NixOS/nixpkgs/commit/7a550016fe7a24fab5f38cacc2ad7ffc4d93f083.patch

=E2=80=AE is a right-to-left override, so the name is reversed after that. If this is language design, we should respect it, but it seems that there is no other function here except to cause trouble for other programs 😅

[dwt]

That is correct, My username has this unicode direction changer in it, which I use to check which programs have good unicode handling - and which don't. If you tool fails with this, I would like to encourage you to file bug reports upstream. If this is causing just trouble for you, then I am sorry. But this has already shown me many bugs in open source programs, so I would like to continue using it.

[wolfgangwalther]

Am I the only one who finds it funny that tig, which is git reversed, has a problem with unicode direction changing? :D

If this is causing just trouble for you

For the record, lazygit that I am using, also has trouble displaying this correctly. Although it doesn't break the layout for me, just yet.

But this has already shown me many bugs in open source programs, so I would like to continue using it.

While I think it's really smart to use it yourself that way, I don't think it's acceptable to push this onto others as well. This reversal literally serves no purpose other than trying to break stuff and in a project of the scale of nixpkgs, you can be sure that quite a few people's tools will have trouble with that.

I suggest you use a git config override for the nixpkgs repo, to use a simpler name with less unicode magic. You can still use this in your other git repos..