Skip to content

nix-required-mounts: correctly handle relative paths in symlink_targets()#500772

Closed
SomeoneSerge wants to merge 3 commits intoNixOS:masterfrom
SomeoneSerge:fix/required-mounts
Closed

nix-required-mounts: correctly handle relative paths in symlink_targets()#500772
SomeoneSerge wants to merge 3 commits intoNixOS:masterfrom
SomeoneSerge:fix/required-mounts

Conversation

@SomeoneSerge
Copy link
Contributor

Cf. #497824 (comment)

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

As per NixOS#497824, symlink_parents() incorrectly handles
relative symlinks and produces non-existent paths. Fix and add unit test
Equivalent/dual view but probably more consistent and intuitive
@SomeoneSerge SomeoneSerge requested review from kmein and tfc March 17, 2026 17:42
p = target
else:
p = parent
p = p.parent / target
Copy link
Contributor

Choose a reason for hiding this comment

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

this will not work. i have a branch with tests and other fixes online. i suggest we discuss that in a meeting this week to get on the same page because i have found multiple issues with the app. https://github.com/tfc/nixpkgs/tree/unwhack-nrm but i am currently still working on that, so expect changes in the next minutes

parsed_drv = parsed_drv[canon_drv_path]
required_features = get_required_system_features(parsed_drv)
required_features = list(filter(known_features.__contains__, required_features))
required_features = list(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uh, accidentally ran black, maybe let it be

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 2.status: merge-bot eligible This PR can be merged by commenting "@NixOS/nixpkgs-merge-bot merge". labels Mar 17, 2026
@SomeoneSerge
Copy link
Contributor Author

Closing in favour #500971

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.status: merge-bot eligible This PR can be merged by commenting "@NixOS/nixpkgs-merge-bot merge". 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants