Skip to content

fix: Resolve symlink before FlakeRefFromEnv#107

Merged
water-sucks merged 1 commit intonix-community:mainfrom
Darkness9724:resolve-symlink
Sep 28, 2025
Merged

fix: Resolve symlink before FlakeRefFromEnv#107
water-sucks merged 1 commit intonix-community:mainfrom
Darkness9724:resolve-symlink

Conversation

@Darkness9724
Copy link
Copy Markdown
Contributor

@Darkness9724 Darkness9724 commented Sep 26, 2025

Greetings!

Currently, nixos-cli subcommands that execute the nix command under the hood fail to find flake.nix inside symlinked directories (for example, ln -s ~/.config/nixos /etc; nixos apply -dv). However, nixos-rebuild{,-ng} work fine in the same setup.

This change ensures that the path is resolved to the real directory before being passed to FlakeRefFromEnv using filepath.EvalSymlinks, allowing symlinked configuration directories to work transparently.

Copy link
Copy Markdown
Collaborator

@water-sucks water-sucks left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I was not aware of this issue.

This PR currently only does this for the generic FindConfiguration() function, which only resolves the issue for implicit configuration discovery.

However, many commands accept explicit command-line flake ref parameters, so this change may need to be applied to all those places, or the same problem may arise.

In our case, since this is flake-specific behavior, I think it would make sense to move this to internal/configuration/configuration.go, and only evaluate symlinks if the provided path exists on disk (i.e. verified with os.Stat()). I'm willing to hear alternative ways of implementing this though :}

@Darkness9724 Darkness9724 force-pushed the resolve-symlink branch 2 times, most recently from f7a2a54 to e65fb60 Compare September 26, 2025 17:26
@Darkness9724
Copy link
Copy Markdown
Contributor Author

Darkness9724 commented Sep 26, 2025

In our case, since this is flake-specific behavior, I think it would make sense to move this to internal/configuration/configuration.go, and only evaluate symlinks if the provided path exists on disk (i.e. verified with os.Stat()). I'm willing to hear alternative ways of implementing this though :}

I believe you meant internal/configuration/flake.go? Because the previous change was already applied to internal/configuration/configuration.go.

I’ve gone ahead and moved the symlink resolution logic to internal/configuration/flake.go as suggested. The new ResolvePath function uses os.Stat() to verify path existence before calling filepath.EvalSymlinks. It’s now should be integrated into both FlakeRefFromEnv for implicit configuration discovery and FlakeRefFromString for explicit flake references.

I’ve also tested cases like nixos apply git+https://path/to/repo/ to ensure non-local URIs are handled correctly. I wanted to add unit tests as well, but I’m not very experienced with Go yet :)

Currently, nixos-cli subcommands that execute the nix command under the hood
fail to find flake.nix inside symlinked directories (ex: ln -s ~/.config/nixos /etc; nixos apply -dv).
However, nixos-rebuild{,-ng} work fine in the same setup.

This change ensures that the path is resolved to the real directory
before being passed to FlakeRefFromEnv using filepath.EvalSymlinks,
allowing symlinked configuration directories to work transparently.

Signed-off-by: Maxim Mikhailov <darkness9724@gmail.com>
Copy link
Copy Markdown
Collaborator

@water-sucks water-sucks left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! Works during my own (rudimentary) testing as well.

Appreciate you for taking the time to address the requested changes too :}

At some point I want to take a crack at resolving symlinks in upstream Nix as well, since that can be useful for behavior for other systems, i.e. nix-darwin, but this will do for now.

@water-sucks water-sucks merged commit 44568f7 into nix-community:main Sep 28, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants