Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove builtins.exec #3355

Closed
wants to merge 1 commit into from
Closed

remove builtins.exec #3355

wants to merge 1 commit into from

Conversation

@andir
Copy link
Member

@andir andir commented Feb 12, 2020

IMO that feature is very dangerous to have in Nix and probably doesn't fit the entire model at all.

It currently isn't documented so there shouldn't be many users. Those that need it can probably add this via a plugin. The plugin example (tests/plugins/plugintest.c) already shows how to add such a plugin and the removed code can probably just pasted into that file to get started.

@shlevy
Copy link
Member

@shlevy shlevy commented Feb 12, 2020

builtins.exec is already hidden behind a very verbose flag, and allows for a much nicer extension experience without any native code. What is the concern?

@grahamc
Copy link
Member

@grahamc grahamc commented Feb 12, 2020

People see builtins.exec and think "ooooh!", and then nearly nothing persuades them against it.

@shlevy
Copy link
Member

@shlevy shlevy commented Feb 13, 2020

Are there real examples of people shooting themselves in the foot or failing to do things the right way due to this?

@flokli
Copy link
Contributor

@flokli flokli commented Feb 13, 2020

It's a massive opportunity to shoot yourself in the foot.

It's undocumented, 0bb8db2 mentioned it was meant as an intermediate thing, so I think if people want it, moving it to an optional plugin would be the right move.

@zimbatm
Copy link
Member

@zimbatm zimbatm commented Feb 13, 2020

Would removing it solve a actual current problem? This provides an escape hatch to do funny and crazy things. It's properly protected behind a scary flag.

@edolstra
Copy link
Member

@edolstra edolstra commented Feb 19, 2020

Closing since it doesn't really harm anybody as it's hidden behind a flag.

@edolstra edolstra closed this Feb 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.