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

[backport 2.18-maintenance] AttrCursor: Remove forceErrors #11445

Merged
merged 5 commits into from
Sep 9, 2024
Merged

[backport 2.18-maintenance] AttrCursor: Remove forceErrors #11445

merged 5 commits into from
Sep 9, 2024

Conversation

eclairevoyant
Copy link
Contributor

Motivation

backport of #10564

Context

fixes #3872 for those on nix 2.18

Priorities and Process

Add 👍 to pull requests you find important.

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

@edolstra @eclairevoyant
Add test for the evaluation cache
e3c55dd 
(cherry picked from commit 8b86f41)
@edolstra @eclairevoyant
AttrCursor: Remove forceErrors
8e5ec81 
Instead, force evaluation of the original value only if we need to
show the exception to the user.

(cherry picked from commit 2c88930)
@edolstra @eclairevoyant
Typo
fc14197 
(cherry picked from commit eeb4c40)
@github-actions github-actions bot added new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority labels Sep 8, 2024
@eclairevoyant eclairevoyant mentioned this pull request Sep 8, 2024
Closed
@eclairevoyant eclairevoyant marked this pull request as draft September 8, 2024 14:53
@eclairevoyant
Copy link
Contributor Author

state is defined in eval-cache.hh but not visible in eval-cache.cc, idk what do

@roberth
Store ref<EvalState> in CachedEvalError
5bcc706 
This makes the previous commits work.
Newer versions store it in all EvalErrors.
@roberth
Copy link
Member

roberth commented Sep 9, 2024

CI / installer (push) failure can be ignored. I'll try to backport its fix, but it's not a blocker afaic.

@eclairevoyant eclairevoyant marked this pull request as ready for review September 9, 2024 16:27
@eclairevoyant
Copy link
Contributor Author

Tested locally as well, seems to avoid the cached failure as expected.

@roberth roberth merged commit 6354f84 into NixOS:2.18-maintenance Sep 9, 2024
14 of 15 checks passed
@roberth
Copy link
Member

roberth commented Sep 9, 2024

Thanks!

@eclairevoyant eclairevoyant deleted the backport-10564-to-2.18 branch September 9, 2024 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edolstra edolstra edolstra approved these changes

Assignees
No one assigned
Labels
new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@eclairevoyant @roberth @edolstra