chore!: remove `allow-unsafe-native-code-during-evaluation`

kfearsoff commented

2025-04-09 14:03:54 +00:00

Member

There's a very cursed setting called allow-unsafe-native-code-during-evaluation which allows calling external programs or C symbols during evaluation.

It was added 10 years ago and was only documented very recently. The use case largely overlaps with plugins, which are a little more involved for adding new builtins, but otherwise work better.

Furthermore, the entire mechanism is extremely unsafe and depends on internal C++ API, which we don't guarantee to be stable. It is probably best to remove it to make our codebase a little less cursed.

Additional Context

Need to remove relevant C++ functions: https://git.lix.systems/lix-project/lix/src/branch/main/lix/libexpr/primops.cc#L281-L348

Also in builtins init: https://git.lix.systems/lix-project/lix/src/branch/main/lix/libexpr/primops.cc#L2848-L2860

Also in headers: https://git.lix.systems/lix-project/lix/src/branch/main/lix/libexpr/primops.hh#L40-L52

Remove the setting: https://git.lix.systems/lix-project/lix/src/branch/main/lix/libexpr/settings/allow-unsafe-native-code-during-evaluation.md

There's a very cursed setting called [allow-unsafe-native-code-during-evaluation](https://docs.lix.systems/manual/lix/stable/command-ref/conf-file.html#conf-allow-unsafe-native-code-during-evaluation) which allows calling external programs or C symbols during evaluation. It was added [10 years ago](https://github.com/NixOS/nix/pull/270) and was only documented [very recently](https://github.com/NixOS/nix/pull/10803). The use case largely overlaps with [plugins](https://nix.dev/manual/nix/2.24/command-ref/conf-file.html#conf-plugin-files), which are a little [more involved](https://github.com/NixOS/nix/issues/8171#issuecomment-1499085774) for adding new builtins, but otherwise work better. Furthermore, the entire mechanism is extremely unsafe and depends on internal C++ API, which we don't guarantee to be stable. It is probably best to remove it to make our codebase a little less cursed. ## Additional Context Need to remove relevant C++ functions: https://git.lix.systems/lix-project/lix/src/branch/main/lix/libexpr/primops.cc#L281-L348 Also in builtins init: https://git.lix.systems/lix-project/lix/src/branch/main/lix/libexpr/primops.cc#L2848-L2860 Also in headers: https://git.lix.systems/lix-project/lix/src/branch/main/lix/libexpr/primops.hh#L40-L52 Remove the setting: https://git.lix.systems/lix-project/lix/src/branch/main/lix/libexpr/settings/allow-unsafe-native-code-during-evaluation.md

kfearsoff added the

labels

2025-04-09 14:03:54 +00:00

kfearsoff commented

2025-04-09 14:04:39 +00:00

Author

Member

If maintainers think this is ok to nuke, please change the Context label to Context/drive-by!

If maintainers think this is ok to nuke, please change the Context label to `Context/drive-by`!

jade commented

2025-04-09 16:07:55 +00:00

Owner

This feature is hilarious and very half baked. I doubt anyone uses it, but you'd be somewhat surprised sometimes. I would recommend surveying GitHub to see if anyone is using it.

kfearsoff commented

2025-04-09 16:40:16 +00:00

Author

Member

Right. Great idea. This query seems pretty accurate: https://github.com/search?q=%22allow-unsafe-native-code-during-evaluation%22+NOT+repo%3ANixOS%2Fnix+NOT+repo%3Alix-project%2Flix+NOT+is%3Afork&type=code

Don't feel qualified to say if it's fine to break that. My gut instinct says "yes", but I'd like maintainer confirmation.

Right. Great idea. This query seems pretty accurate: https://github.com/search?q=%22allow-unsafe-native-code-during-evaluation%22+NOT+repo%3ANixOS%2Fnix+NOT+repo%3Alix-project%2Flix+NOT+is%3Afork&type=code Don't feel qualified to say if it's fine to break that. My gut instinct says "yes", but I'd like maintainer confirmation.

jade commented

2025-04-09 21:49:48 +00:00

Owner

Ah, okay, I just looked at the documentation and found some funny stuff on GitHub we don't care about: f2016883bc/exec-bootstrap.nix (L2). We can remove builtins.importNative, but we can't remove builtins.exec. At the very least @bacchanalia (i think this is their @) uses it at work iirc and this is because of use cases in large private monorepos that critically require it, to get around limitations of source filtering or similar.

Ah, okay, I just looked at the documentation and found some funny stuff on GitHub we don't care about: https://github.com/shlevy/nixpkgs-development-support/blob/f2016883bc73eea2fff3a7c6faf0a37f83b48674/exec-bootstrap.nix#L2. We *can* remove builtins.importNative, but we *can't* remove builtins.exec. At the very least @bacchanalia (i think this is their @) uses it at work iirc and this is because of use cases in large private monorepos that critically require it, to get around limitations of source filtering or similar.

kfearsoff commented

2025-04-09 22:56:30 +00:00

Author

Member

I think someone from Target corporation initially requested the feature to be added for monorepo usage, too...

I thought Lix wouldn't be used like that, but oh well. How reasonable would it be to rewrite this to be used by a plugin? Or, what needs to be done in code to support their use case natively?

I think someone from Target corporation initially requested the feature to be added for monorepo usage, too... I thought Lix wouldn't be used like that, but oh well. How reasonable would it be to rewrite this to be used by a plugin? Or, what needs to be done in code to support their use case natively?

bacchanalia commented

2025-04-09 22:59:43 +00:00

Member

Correct. I've built out machinery that if you squint at it looks a bit like a large monorepo friendly flake alternative, that relies on builtins.exec under the hood. I think semantically it all could be done using IFD, but IFD is too slow.

Correct. I've built out machinery that if you squint at it looks a bit like a large monorepo friendly flake alternative, that relies on `builtins.exec` under the hood. I think semantically it all could be done using IFD, but IFD is too slow.

jade commented

2025-04-09 23:16:59 +00:00

Owner

@kfearsoff wrote in #795 (comment):

I think someone from Target corporation initially requested the feature to be added for monorepo usage, too...

I thought Lix wouldn't be used like that, but oh well. How reasonable would it be to rewrite this to be used by a plugin? Or, what needs to be done in code to support their use case natively?

Lix is intended to be a usable nix implementation, and many of its goals are aligned with using it in huge installations like NixOS Hydra (evaluating a huge monorepo). It is used by some very large companies today. My own work has a large monorepo, though it's currently using flakes, to the detriment of fast evaluations.

Improving the monorepo use case at very low maintenance cost (builtins.exec is not the same order of magnitude as cursed as recursive-nix e.g.!) is in line with the goals of the project, which absolutely include production use.

Removing the builtins.exec that's been gated behind a setting for years and which has worked the same for years with basically nobody touching it and basically no maintenance cost, in favour of a plugin which will break randomly because plugins have poor interactions with different Lix versions, does not make any sense to me.

@kfearsoff wrote in https://git.lix.systems/lix-project/lix/issues/795#issuecomment-10213: > I think someone from Target corporation initially requested the feature to be added for monorepo usage, too... > > I thought Lix wouldn't be used like that, but oh well. How reasonable would it be to rewrite this to be used by a plugin? Or, what needs to be done in code to support their use case natively? Lix is intended to be a usable nix implementation, and many of its goals are aligned with using it in huge installations like NixOS Hydra (evaluating a huge monorepo). It is used by some very large companies today. My own work has a large monorepo, though it's currently using flakes, to the detriment of fast evaluations. Improving the monorepo use case at very low maintenance cost (builtins.exec is not the same order of magnitude as cursed as recursive-nix e.g.!) is in line with the goals of the project, which absolutely include production use. Removing the builtins.exec that's been gated behind a setting for years and which has worked the same for years with basically nobody touching it and basically no maintenance cost, in favour of a plugin which will break randomly because plugins have poor interactions with different Lix versions, does not make any sense to me.

kfearsoff commented

2025-04-10 00:15:18 +00:00

Author

Member

I see. That sounds reasonable.

Do we postpone it until very very later? Or just WONTFIX?

I see. That sounds reasonable. Do we postpone it until very very later? Or just WONTFIX?

jade commented

2025-04-10 01:44:14 +00:00

Owner

importNative can still be removed. i am voting WONTFIX on exec.

👍 1

alois31 commented

2025-04-10 08:27:31 +00:00

Member

Do the existing use cases for exec require it to be available in pure evaluation mode? It is quite obviously impure.

kfearsoff commented

2025-04-10 08:57:56 +00:00

Author

Member

For context, CppNix considers it pure eval compatible by design: https://github.com/NixOS/nix/issues/4212

pennae commented

2025-04-10 12:52:10 +00:00

Owner

WONTFIX on exec and "add a warning for a few cycles first" on importNative. there's no maintenance burden here, unlike with recursive nix or ca derivations or much other nonsense. changing whether they're allowed in pure mode is a breaking change and shouldn't be done without warning either, and the whole thing being gated behind an unsafe feature makes it pretty clear what you're getting into when using it.

WONTFIX on exec and "add a warning for a few cycles first" on importNative. there's no maintenance burden here, unlike with recursive nix or ca derivations or much other nonsense. changing whether they're allowed in pure mode is a breaking change and shouldn't be done without warning either, and the whole thing being gated behind an *unsafe* feature makes it pretty clear what you're getting into when using it.

jade commented

2025-04-10 16:24:50 +00:00

Owner

@alois31 wrote in #795 (comment):

Do the existing use cases for exec require it to be available in pure evaluation mode? It is quite obviously impure.

i can very much see it! the use cases here are hacking around slow path filtering, and ideally you'd prefer to be able to be pure-eval if you can, in order to reduce the amount of shenanigans people are allowed to do. i would leave it alone.

@alois31 wrote in https://git.lix.systems/lix-project/lix/issues/795#issuecomment-10219: > Do the existing use cases for exec require it to be available in pure evaluation mode? It is quite obviously impure. i can very much see it! the use cases here are hacking around slow path filtering, and ideally you'd prefer to be able to be pure-eval if you can, in order to reduce the amount of shenanigans people are allowed to do. i would leave it alone.

bacchanalia commented

2025-04-10 17:48:18 +00:00

Member

The way I think about builtins.exec, is that by enabling allow-unsafe-native-code-during-evaluation you're taking responsibility for using it in a way that doesn't break the constraints that nix usually enforces by itself, and that includes both uses that fall within pure eval and those that don't. That said, for my use cases specifically, not having it available in pure eval would not be a big loss.

The way I think about `builtins.exec`, is that by enabling `allow-unsafe-native-code-during-evaluation` you're taking responsibility for using it in a way that doesn't break the constraints that nix usually enforces by itself, and that includes both uses that fall within pure eval and those that don't. That said, for my use cases specifically, not having it available in pure eval would not be a big loss.

👍 1

1

jade commented

2025-04-10 19:49:59 +00:00

Owner

I agree with that; the people who are using it know what they're doing and won't let incorrect usages through code review.

We should probably deprecate and later remove importNative, it's basically impossible to use correctly and I don't think anyone uses it.

I agree with that; the people who are using it know what they're doing and won't let incorrect usages through code review. We should probably deprecate and later remove importNative, it's basically impossible to use correctly and I don't think anyone uses it.

alois31 commented

2025-04-11 07:34:39 +00:00

Member

@jade wrote in #795 (comment):

i can very much see it! the use cases here are hacking around slow path filtering, and ideally you'd prefer to be able to be pure-eval if you can, in order to reduce the amount of shenanigans people are allowed to do. i would leave it alone.

I would generally agree, but the way I see it builtins.exec is even more blatantly impure than impure evaluation. That said, I could see how builtins.exec usage in such use cases can be treated as a warning where potential impurities can be found (similar to unsafe in Rust).

@jade wrote in https://git.lix.systems/lix-project/lix/issues/795#issuecomment-10231: > i can very much see it! the use cases here are hacking around slow path filtering, and ideally you'd prefer to be able to be pure-eval if you can, in order to reduce the amount of shenanigans people are allowed to do. i would leave it alone. I would generally agree, but the way I see it `builtins.exec` is even more blatantly impure than impure evaluation. That said, I could see how `builtins.exec` usage in such use cases can be treated as a warning where potential impurities can be found (similar to `unsafe` in Rust).

lix-bot commented

2025-04-19 23:41:18 +00:00

Member

This issue was mentioned on Gerrit on the following CLs:

commit message in cl/3025 ("libexpr: add a deprecation warning for builtins.importNative")
comment in cl/3025 ("libexpr: add a deprecation warning for builtins.importNative")

This issue was mentioned on Gerrit on the following CLs: * commit message in [cl/3025](https://gerrit.lix.systems/c/lix/+/3025) ("libexpr: add a deprecation warning for `builtins.importNative`") * comment in [cl/3025](https://gerrit.lix.systems/c/lix/+/3025) ("libexpr: add a deprecation warning for `builtins.importNative`")

just1602 commented

2025-04-19 23:42:30 +00:00

Member

Hey 👋

I create the CL above to add the deprecation warning. In the warning I put lix 2.96 as the deprecation version, but I was wondering what people were thinking about it?

Is it too late or too soon?

Hey 👋 I create the CL above to add the deprecation warning. In the warning I put `lix 2.96` as the deprecation version, but I was wondering what people were thinking about it? Is it too late or too soon?

Rows
Columns

chore!: remove allow-unsafe-native-code-during-evaluation #795

Additional Context

chore!: remove `allow-unsafe-native-code-during-evaluation` #795