update and fix build with latest Lix #14

qyriad · 2024-10-25T20:32:00Z

qyriad commented

2024-10-25 20:32:00 +00:00

Fixes #13. I am not certain that I've used setDefault() vs override() in the right places, but I think it shouldn't matter too much and it seems to work?

Fixes #13. I am not certain that I've used `setDefault()` vs `override()` in the right places, but I think it shouldn't matter too much and it seems to work?

qyriad added 2 commits 2024-10-25 20:32:05 +00:00

flake.lock: Update 50a1455953

Flake lock file updates:

• Updated input 'flake-parts':
    'github:hercules-ci/flake-parts/8471fe90ad337a8074e957b69ca4d0089218391d' (2024-08-01)
  → 'github:hercules-ci/flake-parts/3d04084d54bedc3d6b8b736c70ef449225c361b1' (2024-10-01)
• Updated input 'lix':
    'b016eb0895.tar.gz?narHash=sha256-kOpGI9WPmte1L4QWHviuXsr8jxmGn27zwi82jtzYObM%3D&rev=b016eb0895bb6714a4f6530d9a2bb6577ac6c3cf' (2024-08-13)
  → '2734a9cf94.tar.gz?narHash=sha256-XME7TzBvjK6GEmZqPLK%2B2%2BWk0qnwc7DCwYH434hMcOM%3D&rev=2734a9cf94debc6baef4e7d4d9fa28cc28f5b31d' (2024-10-23)
• Updated input 'lix/nix2container':
    'github:nlewo/nix2container/3853e5caf9ad24103b13aa6e0e8bcebb47649fe4' (2024-07-10)
  → 'github:nlewo/nix2container/fa6bb0a1159f55d071ba99331355955ae30b3401' (2024-08-30)
• Updated input 'lix/pre-commit-hooks':
    'github:cachix/git-hooks.nix/f451c19376071a90d8c58ab1a953c6e9840527fd' (2024-07-15)
  → 'github:cachix/git-hooks.nix/4e743a6920eab45e8ba0fbe49dc459f1423a4b74' (2024-09-19)
• Updated input 'nix-github-actions':
    'github:nix-community/nix-github-actions/622f829f5fe69310a866c8a6cd07e747c44ef820' (2024-07-04)
  → 'github:nix-community/nix-github-actions/e04df33f62cdcf93d73e9a04142464753a16db67' (2024-10-24)
• Updated input 'nixpkgs':
    'github:NixOS/nixpkgs/fb81cec9eda2a6b5365ad723995f0329d9e356fd' (2024-08-13)
  → 'github:NixOS/nixpkgs/45e5197248e59e92e88956c5aa12553a7f62337f' (2024-10-25)
• Updated input 'treefmt-nix':
    'github:numtide/treefmt-nix/349de7bc435bdff37785c2466f054ed1766173be' (2024-08-12)
  → 'github:numtide/treefmt-nix/aac86347fb5063960eccb19493e0cadcdb4205ca' (2024-10-22)

fix build with latest Lix 43aaa943bf

Lix commit 4dbbd721e[1] changed the way settings are changed, removing
operator= in the process. This commit changes the places where we use
operator= to using either setDefault(), or override(). I *believe* I
have used the correct ones for each changed setting.

Fixes #13.

[1]: 4dbbd721eb9db75d4968a624b8cb9e75e979a144

alois31 requested changes 2024-10-26 10:03:19 +00:00

src/nix-eval-jobs.cc

					
				@ -351,3 +351,3 @@

				        /* FIXME: The build hook in conjunction with import-from-derivation is

				         * causing "unexpected EOF" during eval */

				        settings.builders = "";

				        settings.builders.setDefault("");

alois31 commented

2024-10-26 09:58:08 +00:00

Not sure if this makes sense, if it breaks stuff it should be overridden, and if it doesn't why do it at all?

src/nix-eval-jobs.cc

					
				@ -355,3 +355,3 @@

				        /* Prevent access to paths outside of the Nix search path and

				           to the environment. */

				        evalSettings.restrictEval = false;

				        evalSettings.restrictEval.setDefault(false);

alois31 commented

2024-10-26 10:01:30 +00:00

This does not make sense at all, as the default is false already. You either want to override it or delete this entirely. Or judging by the comment, false is a typo to begin with and this should set the default to true (but still allowing the user to override it)?

This does not make sense at all, as the default is `false` already. You either want to override it or delete this entirely. Or judging by the comment, `false` is a typo to begin with and this should set the default to `true` (but still allowing the user to override it)?

qyriad commented

2024-10-27 20:47:25 +00:00

Oh that comment makes this very confusing then. The CLI doesn't seem to have any way to specify this in the first place? Should we just leave it as false since that's what it's been, and add a FIXME?

Oh that comment makes this very confusing then. The CLI doesn't seem to have any way to specify this in the first place? Should we just leave it as `false` since that's what it's been, and add a FIXME?

alois31 commented

2024-10-29 17:34:20 +00:00

Looking at the blame of this code, it seems that the comment was added along with explicitly setting this to true, and when it was later set to false some time later (presumably because breakage in some situations was noticed), the comment was left. I'm not really qualified to comment on this because I'm not familiar with this codebase, but my gut feeling is that this code should be deleted entirely and the user can pass --option restrict-eval true if they really want to enable restricted eval mode.

Looking at the blame of this code, it seems that the comment was added along with explicitly setting this to `true`, and when it was later set to `false` some time later (presumably because breakage in some situations was noticed), the comment was left. I'm not really qualified to comment on this because I'm not familiar with this codebase, but my gut feeling is that this code should be deleted entirely and the user can pass `--option restrict-eval true` if they really want to enable restricted eval mode.

src/nix-eval-jobs.cc

					
				@ -360,3 +360,3 @@

				           'getEnv', 'currentSystem' etc. */

				        if (myArgs.impure) {

				            evalSettings.pureEval = false;

				            evalSettings.pureEval.setDefault(false);