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

nix-env to nix profile: maintain priorities #8956

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

nix-env to nix profile: maintain priorities #8956

wants to merge 9 commits into from
+5 −1

Conversation

elikoga
Copy link
Contributor

@elikoga elikoga commented Sep 9, 2023

nix profile does not set the original priority when converting from manifest.nix to manifest.json

It does now

Fixes #8903

@elikoga elikoga requested a review from edolstra as a code owner September 9, 2023 11:53
@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 9, 2023
edolstra
edolstra approved these changes Sep 26, 2023
View reviewed changes
src/nix/profile.cc Outdated Show resolved Hide resolved
@fricklerhandwerk
Copy link
Contributor

It seems this slipped through the cracks. @iFreilicht you touched the tests last, could you help with resolving the conflict?

@iFreilicht
Copy link
Contributor

I did my best, see iFreilicht@ed9aac6.

Unfortunately, it seems packageInfo.queryMetaInt("priority", defaultPriority) always returns 5, so the test fails. For some reason, debugging artifacts aren't built anymore when I run make, so I can't step through the code to find out where the issue lies.

@elikoga elikoga force-pushed the maintain-priorities-nix-env-nix-profile branch from 79801c7 to d77a5d3 Compare January 29, 2025 10:36
@elikoga elikoga force-pushed the maintain-priorities-nix-env-nix-profile branch from 82e11bd to 664d21d Compare January 29, 2025 10:46
@elikoga elikoga mentioned this pull request Jan 29, 2025
Open
@elikoga
Copy link
Contributor Author

elikoga commented Jan 29, 2025

Cannot rebase due to #12380 but on a happy note, we pass
grep '"priority":10' $TEST_HOME/.nix-profile/manifest.json
in the tests so the code did not need to be fundamentally changed at all.

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

@roberth roberth roberth 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.

bug: nix profile does not set the original priority when converting from manifest.nix to manifest.json
5 participants
@elikoga @fricklerhandwerk @iFreilicht @roberth @edolstra