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

Catch eval errors in hasContent #8109

Merged
merged 2 commits into from
Mar 27, 2023
Merged

Catch eval errors in hasContent #8109

merged 2 commits into from
Mar 27, 2023

Conversation

oxalica
Copy link
Contributor

@oxalica oxalica commented Mar 26, 2023 •
edited by roberth
Loading

Motivation

legacyPackages of nixpkgs trigger eval errors in hasContent, causing the whole legacyPackages being skipped. We should treat it as has-content in that case.

Context

See #7726 (comment)

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
  • 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.

@oxalica
Catch eval errors in hasContent
2941a59 
`legacyPackages` of nixpkgs trigger eval errors in `hasContent`, causing
the whole `legacyPackages` being skipped. We should treat it as
has-content in that case.
@oxalica oxalica requested a review from edolstra as a code owner March 26, 2023 15:10
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Mar 26, 2023
roberth
roberth reviewed Mar 27, 2023
View reviewed changes
tests/flakes/show.sh Outdated Show resolved Hide resolved
roberth
roberth approved these changes Mar 27, 2023
View reviewed changes
Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

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

Great work!

@roberth roberth enabled auto-merge March 27, 2023 03:11
@roberth roberth merged commit 9e7940e into NixOS:master Mar 27, 2023
@oxalica
Copy link
Contributor Author

oxalica commented Mar 27, 2023

Could we backport this to 2.14?

@roberth roberth added the backport 2.14-maintenance Automatically creates a PR against the branch label Mar 27, 2023
@github-actions github-actions bot mentioned this pull request Mar 27, 2023
Merged
@github-actions
Copy link

Successfully created backport PR for 2.14-maintenance:

@oxalica oxalica deleted the fix/show-legacy-packages branch March 28, 2023 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roberth roberth roberth approved these changes

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

Assignees
No one assigned
Labels
backport 2.14-maintenance Automatically creates a PR against the branch 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.
2 participants
@oxalica @roberth