Skip to content

nix: add nix flake root #8486

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

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

nix: add nix flake root #8486

wants to merge 1 commit into from

Conversation

lucasew
Copy link

@lucasew lucasew commented Jun 10, 2023

Motivation

Nix already has a logic to go up levels and find the flake.nix. This subcommand just exposes what it finds.

Context

Implementation guide for #8034
Consumes logic introduced in #5720

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
  • documentation in the internal API docs
  • 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.

@lucasew lucasew requested a review from edolstra as a code owner June 10, 2023 01:40
@github-actions github-actions bot added the new-cli Relating to the "nix" command label Jun 10, 2023
src/nix/flake.cc Outdated
addFlag({
.longName = "as-ref",
.shortName = 'r',
.description = "Show flake root as a normalized flake ref.",
Copy link
Member

Choose a reason for hiding this comment

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

What is a normalized flake ref?

Copy link
Author

Choose a reason for hiding this comment

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

Something like git+path:/path/to/flake/folder instead of /path/to/flake/folder

Copy link
Contributor

Choose a reason for hiding this comment

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

The word "normalized" never shows up in the manual at all, and flakeref is written without a space. Maybe "Show the root as a flakeref in URL-like representation." This way people can check in the manual what that actually means.

Copy link
Author

Choose a reason for hiding this comment

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

changed the flag description

src/nix/flake.cc Outdated

std::string description() override
{
return "get flake root";
Copy link
Member

Choose a reason for hiding this comment

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

Should this be something like "get the root directory of a flake"? Not entirely clear what "flake root" means.

Copy link
Author

Choose a reason for hiding this comment

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

yes

Copy link
Author

Choose a reason for hiding this comment

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

changed the description

Comment on lines +1368 to +1379
int slashIndex = rootRef.find('/');
while (rootRef[slashIndex + 1] == '/') {
slashIndex++;
}
rootRef = rootRef.substr(slashIndex);
Copy link
Member

Choose a reason for hiding this comment

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

Not clear to me what this does...

Copy link
Author

Choose a reason for hiding this comment

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

it just eats out that flake specific part

@roberth
Copy link
Member

roberth commented Jun 12, 2023

Does it look for the flake directory or for the flake root? (And did we settle on those terms?)
While these two file paths coincide for typical flakes, subflakes or flakes in a subdirectory of an input make the distinction relevant. Both should be supported, and I don't think either one should be the default or preferred behavior.

Implementation guide for #8034

To be clear this command is not a solution for that issue, but I appreciate the link.
The reason is that nix flake root, like any other command, can not know the location of the original nix develop flake. nix flake root can only automate the existing heuristics people use, but it is still guesswork and not a reliable link to the nix develop (or nix run) invocation.

Nix already has a logic to go up levels and find the flake.nix. This subcommand just exposes what it finds.

Signed-off-by: lucasew <[email protected]>
@fricklerhandwerk fricklerhandwerk removed their request for review June 4, 2024 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation flakes new-cli Relating to the "nix" command
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants