command-not-found: drop perl dependency #74789

Mic92 · 2019-12-01T15:22:44Z

Faster startup and remove some perl packages from the default nixos closure.
Eventually we could remove perl completely from the default nixos closure.
It now also uses nix run when NIX_AUTO_RUN is set.

Motivation for this change

Things done

Notify maintainers

cc @

Mic92 · 2019-12-01T15:24:03Z

I tested export NIX_AUTO_INSTALL=1, export NIX_AUTO_RUN=1 and without any of those.

Mic92 · 2019-12-01T15:24:56Z

nixos/modules/programs/command-not-found/src/command-not-found.cpp

+        }
+      }
+    } else if (getenv("NIX_AUTO_RUN")) {
+      std::string attrname = std::string("nixpkgs.") + package;


Is nixpkgs. correct here or should it be nixos.?

nixpkgs., however it will become nixpkgs# in the future.

bjornfor · 2019-12-02T19:26:16Z

Small typo in commit subject.

Mic92 · 2019-12-03T10:51:38Z

fixed

edolstra · 2019-12-03T12:12:23Z

This to me is not really an improvement: it replaces a 51-line Perl script with a 141-line C++ program. As an aside, if we really want to get rid of Perl, then Rust is the way to go IMHO.

BTW, I think we can get rid of the auto-install feature, it was added as a bit of a joke and nobody uses it.

edolstra · 2019-12-03T12:13:09Z

nixos/modules/programs/command-not-found/src/command-not-found.cpp

+      }
+    } else if (getenv("NIX_AUTO_RUN")) {
+      std::string attrname = std::string("nixpkgs.") + package;
+      std::vector<const char *> args = {"nix", "run", &attrname[0],


nix run should be avoided, it's experimental and won't work in Nix 2.4 unless you pass --experimental-features nix-command.

@edolstra is this going away in future or when does it become stable?

FYI, nix run is used a few places in nixpkgs (and the manual) already.

Not only nix run also nix copy and nix build (for example in the installer).

Yeah, we should replace those by nix-build etc. (or add --experimental-features nix-command to those invocations, but that's tricky because it could override the user's experimental-features setting).

BTW, maybe we should get rid of NIX_AUTO_RUN as well, automatically downloading/running binaries is not a very secure thing to do...

Let's say not safe instead of secure.

edolstra · 2019-12-03T12:13:45Z

nixos/modules/programs/command-not-found/src/command-not-found.cpp

+  }
+};
+
+int queryPackages(const char *system, const char *program,


These char * are a bit unidiomatic C++ IMHO.

edolstra · 2019-12-03T12:14:10Z

nixos/modules/programs/command-not-found/src/command-not-found.cpp

+
+#ifndef NIX_SYSTEM
+#error "NIX_SYSTEM not defined"
+#endif


These are unnecessary, compilation will fail anyway if they're not defined.

Mic92 · 2019-12-03T20:05:26Z

This to me is not really an improvement: it replaces a 51-line Perl script with a 141-line C++ program. As an aside, if we really want to get rid of Perl, then Rust is the way to go IMHO.

BTW, I think we can get rid of the auto-install feature, it was added as a bit of a joke and nobody uses it.

I choose C++:

because it was proposed here: [WIP] Port switch-to-configuration script to Python #48378 (comment)
it was accepted here: nixos-option C++ rewrite with --all #68193
the sqlite interface is already in C, which made the building part quite simple and avoids having to cross-compile rust, when cross-compiling NixOS.

I am not sure the Rust version would be much shorter. The C++ is now 99 lines compared to 55 lines Perl. But it also generates more error messages and starts faster.
I guess I just wait for nix run to become stable - it is better suited for this use case since nix-shell exports all tons of unneeded environment variables and takes more time to execute as it spawns a shell with setup hooks.

Faster startup and remove some perl packages from the default nixos closure. Eventually we could remove perl completely from the default nixos closure.

Mic92 · 2019-12-06T10:43:25Z

Old closure size: 82.9M
New closure size: 33.6M

New one is faster but it probably hardly matters:

Old:

$ time command-not-found sqlite3
0.03s user 0.00s system 98% cpu 0.034 total

New:

$ time command-not-found sqlite3
0.00s user 0.00s system 87% cpu 0.004 total

The C++ program is now +28 lines longer.

Does this looks fine now?

bjornfor · 2019-12-06T12:02:45Z

-1 on removing NIX_AUTO_INSTALL, since I'm using it. Or at least remove it in a separate PR with a release note entry.

Mic92 · 2019-12-06T12:09:54Z

I am fine with whatever the consensus is.

doronbehar · 2020-05-21T06:22:30Z

BTW for the record, nix-index (Rust written) has a database and a "command-not-found" shell function which can be used:

https://github.com/bennofs/nix-index#usage-as-a-command-not-found-replacement

Though from my experience it feels somewhat slower to that of the original command-not-found implementation.

Mic92 · 2020-05-21T13:15:35Z

There is also a rust version alternative: #88517

Mic92 commented Dec 1, 2019

View reviewed changes

Mic92 force-pushed the command-not-found branch from 63c9a2e to e416c35 Compare December 1, 2019 15:27

ofborg bot added 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Dec 1, 2019

Mic92 force-pushed the command-not-found branch from e416c35 to 3233ca3 Compare December 1, 2019 17:01

Mic92 requested a review from edolstra December 1, 2019 17:05

Mic92 changed the title ~~commnand-not-found: drop perl dependency~~ command-not-found: drop perl dependency Dec 1, 2019

Mic92 force-pushed the command-not-found branch from 3233ca3 to d131ccc Compare December 3, 2019 10:51

edolstra reviewed Dec 3, 2019

View reviewed changes

Mic92 force-pushed the command-not-found branch from d131ccc to f012aa4 Compare December 3, 2019 19:57

FRidh mentioned this pull request Dec 4, 2019

command-not-found: suggest nix-shell as well #74993

Closed

10 tasks

command-not-found: drop perl dependency

2ccb9f5

Faster startup and remove some perl packages from the default nixos closure. Eventually we could remove perl completely from the default nixos closure.

Mic92 force-pushed the command-not-found branch from f012aa4 to 2ccb9f5 Compare December 6, 2019 10:34

edolstra approved these changes Dec 6, 2019

View reviewed changes

Mic92 mentioned this pull request May 21, 2020

command-not-found: rewrite in Rust #88517

Closed

10 tasks

Mic92 closed this Jun 23, 2020

Mic92 deleted the command-not-found branch November 1, 2022 16:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

command-not-found: drop perl dependency #74789

command-not-found: drop perl dependency #74789

Mic92 commented Dec 1, 2019 •

edited

Mic92 commented Dec 1, 2019

Mic92 Dec 1, 2019

edolstra Dec 5, 2019

bjornfor commented Dec 2, 2019

Mic92 commented Dec 3, 2019

edolstra commented Dec 3, 2019

edolstra Dec 3, 2019

Mic92 Dec 3, 2019 •

edited

bjornfor Dec 3, 2019

Mic92 Dec 3, 2019

edolstra Dec 5, 2019

edolstra Dec 5, 2019

Mic92 Dec 6, 2019

edolstra Dec 3, 2019

edolstra Dec 3, 2019

Mic92 commented Dec 3, 2019 •

edited

Mic92 commented Dec 6, 2019

bjornfor commented Dec 6, 2019

Mic92 commented Dec 6, 2019 •

edited

doronbehar commented May 21, 2020

Mic92 commented May 21, 2020

command-not-found: drop perl dependency #74789

command-not-found: drop perl dependency #74789

Conversation

Mic92 commented Dec 1, 2019 • edited

Motivation for this change

Things done

Notify maintainers

Mic92 commented Dec 1, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bjornfor commented Dec 2, 2019

Mic92 commented Dec 3, 2019

edolstra commented Dec 3, 2019

Choose a reason for hiding this comment

Mic92 Dec 3, 2019 • 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

Choose a reason for hiding this comment

Mic92 commented Dec 3, 2019 • edited

Mic92 commented Dec 6, 2019

bjornfor commented Dec 6, 2019

Mic92 commented Dec 6, 2019 • edited

doronbehar commented May 21, 2020

Mic92 commented May 21, 2020

Mic92 commented Dec 1, 2019 •

edited

Mic92 Dec 3, 2019 •

edited

Mic92 commented Dec 3, 2019 •

edited

Mic92 commented Dec 6, 2019 •

edited