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

improve handling of unsandboxed checks #53

Open
sellout opened this issue Dec 2, 2023 · 1 comment
Open

improve handling of unsandboxed checks #53

sellout opened this issue Dec 2, 2023 · 1 comment

Comments

@sellout
Copy link
Owner

sellout commented Dec 2, 2023

Some checks (e.g., Vale’s1) currently need to disable the chroot. The way it's currently handled, they are just regular checks, so you need to either manually exclude them from your flake or set sandbox = false if you want to nix flake check. Setting sandbox = false in the flake has already bit me on multiple occasions, so I want to get rid of that.

There are a few different thoughts I've had for managing this:

  • provide a way to add only sandboxed checks to the flake
  • add an option to include the unsandboxed checks as devShells instead
  • make use of the lax-checks devShell that sort of exists to bundle all of the unsandboxed checks similarly to nix flake check

All three is probably the way to go. Additionally, it'd be great to automatically generate GH or other CI jobs for unsandboxed derivations, since garnix can't run them.

Footnotes

  1. In the case of Vale, I want to add a valeWithPackages to Nixpkgs so that Vale packages are just derivations and don't break the sandbox. But this issue still holds more broadly.

sellout added a commit to sellout/flaky that referenced this issue Dec 2, 2023
Eldev likes to do a lot of things that don't get along with Nix. In this case,
because there are checks that can't run in the sandbox, it's easy to overlook
sandbox violations until garnix catches them.

Without `--external`, Eldev will try to download various packages when linting
and testing. This restores the `--external` that used to be there and still
needs to be.

Just one more instance of sellout/project-manager#53.
sellout added a commit to sellout/flaky that referenced this issue Dec 2, 2023
Eldev likes to do a lot of things that don't get along with Nix. In this
case,
because there are checks that can't run in the sandbox, it's easy to
overlook
sandbox violations until garnix catches them.

Without `--external`, Eldev will try to download various packages when
linting
and testing. This restores the `--external` that used to be there and
still
needs to be.

Just one more instance of sellout/project-manager#53.
@sellout
Copy link
Owner Author

sellout commented Dec 5, 2023

This is much improved by #55, but not quite finished. I think the changes in that PR are generally better than what’s suggested above. The first bullet point is the same, but the second is unnecessary, as they can still be checks and the last one is there but not quite in a satisfactory state.

I hadn’t known that sandbox = "relaxed" was a thing, so that helps a lot. But the thing that’s missing is being able to somehow run the unsandboxed checks when sandbox = true. It seems that CLI options like --option sandbox false don’t take precedence, which is surprising.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant