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.

Sign up for GitHub

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

Jump to bottom

Don't print unknown locations unless requested for dev purposes #8905

Merged
merged 3 commits into from
Oct 6, 2023
Merged

Don't print unknown locations unless requested for dev purposes #8905

merged 3 commits into from
Oct 6, 2023

Conversation

roberth
Copy link
Member

@roberth roberth commented Sep 3, 2023

Motivation

This does add environment variable _NIX_DEVELOPER_SHOW_UNKNOWN_LOCATIONS as it serves a purpose for improving error messages.

Context

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

  • agreed on idea
  • agreed on implementation strategy
  • tests, as appropriate
    • functional tests - tests/**.sh
    • unit tests - src/*/tests
    • integration tests - tests/nixos/*
  • documentation in the manual
  • documentation in the internal API docs
  • code and comments are self-explanatory
  • commit message explains why the change was made
  • new feature or incompatible change: updated release notes

Priorities

Add 👍 to pull requests you find important.

@roberth
traces: Do not print unknown location
4c50f5d 
Solves 1/3 of the infinite recursion at unknown location meme.
See NixOS#8879 for ensuring we always have a trace (for stack overflows)
We might want to re-add this for finding missing location info
*while hacking on that problem only*.
@roberth
traces: DRY printPosMaybe
f1aeeea
@roberth
traces: Add _NIX_DEVELOPER_SHOW_UNKNOWN_LOCATIONS
477bc61
@roberth roberth added error-messages Confusing messages and better diagnostics language The Nix expression language; parser, interpreter, primops, evaluation, etc labels Sep 3, 2023
@roberth roberth requested a review from edolstra as a code owner September 3, 2023 12:13
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Sep 3, 2023
Artturin
Artturin approved these changes Sep 18, 2023
View reviewed changes
Copy link
Member

@Artturin Artturin left a comment •
edited
Loading

Choose a reason for hiding this comment

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

To my untrained cpp eye it looks OK and the tests pass. The idea is good.

@edolstra edolstra merged commit 3dd4475 into NixOS:master Oct 6, 2023
@fricklerhandwerk fricklerhandwerk mentioned this pull request Oct 9, 2023
Merged
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2023-10-06-nix-team-meeting-minutes-92/34001/1

@roberth roberth mentioned this pull request Dec 9, 2023
Merged
tebowy pushed a commit to tebowy/nix that referenced this pull request Jul 11, 2024
Merge pull request NixOS#8905 from hercules-ci/no-unknown-location
4517de0 
Don't print unknown locations unless requested for dev purposes

(cherry picked from commit 3dd4475)
Change-Id: I04a91277d1d9d09f5c1bf4a28fc99f0702b161e5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Artturin Artturin Artturin approved these changes

@edolstra edolstra Awaiting requested review from edolstra edolstra is a code owner

@thufschmitt thufschmitt Awaiting requested review from thufschmitt

Assignees
No one assigned
Labels
error-messages Confusing messages and better diagnostics language The Nix expression language; parser, interpreter, primops, evaluation, etc with-tests Issues related to testing. PRs with tests have some priority
Projects
Nix team
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@roberth @nixos-discourse @Artturin @edolstra