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

disallowedRequisites: Show forbidden chain(s) to forbidden path #10877

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

roberth
Copy link
Member

@roberth roberth commented Jun 9, 2024

Motivation

When a build fails because of disallowedRequisites, the error message now includes the chain of references that led to the failure. This makes it easier to see in which derivations the chain can be broken, to resolve the problem.

Example:

$ nix build --no-link /nix/store/1fsjsd98awcf3sgny9zfq4wa7i5dci4n-hercules-ci-agent-0.10.3.drv^out
error: output '/nix/store/xg01gga2qsi9v2qm45r2gqrkk46wvrkd-hercules-ci-agent-0.10.3' is not allowed to refer to the following paths.
       Shown below are chains that lead to the forbidden path(s). More may exist.
         { /nix/store/xg01gga2qsi9v2qm45r2gqrkk46wvrkd-hercules-ci-agent-0.10.3 -> /nix/store/mfzkjrwv8ckqqbsbj5yj24ri9dsgs30l-hercules-ci-cnix-expr-0.3.6.3 -> /nix/store/vxlw2igrncif77fh2qg1jg02bd455mbl-ghc-9.6.5 }

Context

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@github-actions github-actions bot added documentation with-tests Issues related to testing. PRs with tests have some priority labels Jun 9, 2024
@roberth roberth force-pushed the report-forbidden-path-as-chain branch from 3c6e18a to 131974a Compare June 9, 2024 22:29
$ nix build --no-link /nix/store/1fsjsd98awcf3sgny9zfq4wa7i5dci4n-hercules-ci-agent-0.10.3.drv^out
error: output '/nix/store/xg01gga2qsi9v2qm45r2gqrkk46wvrkd-hercules-ci-agent-0.10.3' is not allowed to refer to the following paths.
Shown below are chains that lead to the forbidden path(s). More may exist.
{ /nix/store/xg01gga2qsi9v2qm45r2gqrkk46wvrkd-hercules-ci-agent-0.10.3 -> /nix/store/mfzkjrwv8ckqqbsbj5yj24ri9dsgs30l-hercules-ci-cnix-expr-0.3.6.3 -> /nix/store/vxlw2igrncif77fh2qg1jg02bd455mbl-ghc-9.6.5 }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if a more readable output would be something similar to why-depends -- the tree-like output is a lot easier to read than a super-long chain of [store path] -> [store path] -> [store path] -> .....

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the chain is short enough, it will look better, and I expect the chains to be short.

I did consider it, but was non-trivial to achieve, because that code can't simply be reused, and when we use the tree syntax, we also need a proper tree data model in order to render it properly. So it's:

  • gather all chains up into a tree
  • refactor the tree rendering code to take a more abstract tree
    • or maybe only factor out the very surface level rendering bits, insofar that makes sense
  • render it with that

I've already had to do a much deeper dive for this unplanned thing, which should have been quick. That's why I had already added the comment:

// Consider working with paths in a more stable order (https://github.com/NixOS/nix/issues/10875)
// and/or get all possible chains and present them in the `nix-store -q --tree` format.

I'd like to save that improvement for later and focus on more urgent things, but PRs are welcome.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or do something very ad hoc, and not bother deduplicating common paths so that it forms a tree, but it's worth noting that it also requires more test setup to check a multi-line output, fwiw.

@roberth roberth force-pushed the report-forbidden-path-as-chain branch from 131974a to d1911e1 Compare June 10, 2024 10:07
@roberth roberth added the error-messages Confusing messages and better diagnostics label Jun 11, 2024
@fricklerhandwerk fricklerhandwerk removed their request for review June 13, 2024 09:46
@edolstra edolstra requested review from Ericson2314 and removed request for Ericson2314 July 17, 2024 19:13
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2024-07-17-nix-team-meeting-minutes-162/49255/1

@roberth roberth force-pushed the report-forbidden-path-as-chain branch from d1911e1 to 06fad10 Compare July 24, 2024 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation error-messages Confusing messages and better diagnostics with-tests Issues related to testing. PRs with tests have some priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants