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.

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

lxc: fix postInstall substitutions #371051

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

BSFishy
Copy link

@BSFishy BSFishy commented Jan 5, 2025

Fix the order of substitutions in postInstall hook of lxc package. They seem to have been implemented in reverse order, resulting in path issues.

Also changed some of the substitutions to --replace-warn. In my testing, leaving it on --replace-fail caused it to false-positive fail.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@nix-owners nix-owners bot requested a review from adamcstephens January 5, 2025 03:45
@NixOSInfra NixOSInfra added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jan 5, 2025
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 11-100 labels Jan 5, 2025
Copy link
Contributor

@adamcstephens adamcstephens left a comment

Choose a reason for hiding this comment

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

Can you explain the behavior you're trying to fix?

I think you're actually reversing the correct order of operations, and changing fail to warn is obscuring this reversal.

@wegank wegank added 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package 12.approvals: 1 This PR was reviewed and approved by one reputable person labels Jan 5, 2025
@BSFishy
Copy link
Author

BSFishy commented Jan 5, 2025

Sorry for not including more details in the original description! I'm trying to fix the file paths in some of the configuration files. Grabbing LXC from unstable gives me this:

matt on orion in ~ took 1m8s
❯ nix-shell -p lxc

matt on orion in ~ via  impure
❯ which lxc
/nix/store/6ysbxkxc87ya1iasxjhsnyjzsnmjpxaj-lxc-6.0.3/bin/lxc

matt on orion in ~ via  impure
❯ cat /nix/store/6ysbxkxc87ya1iasxjhsnyjzsnmjpxaj-lxc-6.0.3/share/lxc/config/common.conf | rg /run/current-system/sw/share
lxc.hook.clone = /run/current-system/sw/share/lxc/hooks/clonehostname
lxc.seccomp.profile = /run/current-system/sw/share/lxc/config/common.seccomp
# Lastly, include all the configs from /run/current-system/sw/share/lxc/config/common.conf.d/
lxc.include = /run/current-system/sw/share/lxc/config/common.conf.d/

And even just trying to create a container fails:

❯ sudo $(which lxc) create -n nix-sample -t download -- -d ubuntu -r oracular -a amd64
Using image from local cache
Unpacking the rootfs

---
You just created an Ubuntu oracular amd64 (20241230_07:42) container.

To enable SSH, run: apt install openssh-server
No default root or user password are set by LXC.
lxc-create: nix-sample: ../src/lxc/parse.c: lxc_file_for_each_line_mmap: 78 No such file or directory - Failed to open file "/run/current-system/sw/share/lxc/config/common.conf"
lxc-create: nix-sample: ../src/lxc/parse.c: lxc_file_for_each_line_mmap: 129 Failed to parse config file "/var/lib/lxc/nix-sample/config" at line "lxc.include = /run/current-system/sw/share/lxc/config/common.conf"
lxc-create: nix-sample: ../src/lxc/tools/lxc_create.c: lxc_create_main: 318 Failed to create container nix-sample

This has been a particular issue as I've been trying to create containers and LXC starts looking for these configuration files at the wrong paths. I manually patched these for my development environment and with these changes it seems to be working fine, i.e. I'm not getting "file not found" errors anymore and I can successfully create containers.

I agree that changing fail to warn could obscure errors, but when I just reversed them initially, ran the build, got the error, and checked the files that were being substituted, the files were empty. I'm not familiar enough with Nix build to know why this might be happening, but my assumption was that these files are filled in some sort of 2-stage system where they are created and then filled later, but if these can be left as fail instead of warn, please let me know and I'll update the PR!

@BSFishy
Copy link
Author

BSFishy commented Jan 5, 2025

I'm still digging, but it seems like the substitutions for $out/etc/lxc/lxc and $out/libexec/lxc/lxc-net were fine, but the ones in all the other files break things in a pure installation. I think they would require that a bunch of LXC files need to be installed to /run/current-system/sw/share for it to function properly

@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 11-100 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants