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

Fix flakes follow symlinks #12286

Merged
merged 1 commit into from
Feb 2, 2025
Merged

Conversation

stevalkr
Copy link
Contributor

@stevalkr stevalkr commented Jan 17, 2025

Motivation

This is a port of #9499 by @con-f-use with tests requested by @roberth

  • symlink points to flake
  • symlink points to directory with a subdir that's a flake (ie the symlink isn't the final component but in between)
  • symlink points to a directory in a repo
  • symlink points from a repo to another directory. Needs --impure? Also test the error.

The last point should be discussed. The current behavior is nix respects the original flake that symlink is pointing to, no --impure needed. This is IMO the desired behavior that I can store links of all the flakes in each project in one place, like how nix manages profiles.

But if someone wants to have one general flake that applies to multiple projects, and for example, uses relative path to files, it's not the case. But I think path: should be the solution.

Context

Closes: #9463
Closes: #9499


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@stevalkr stevalkr requested a review from edolstra as a code owner January 17, 2025 14:39
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Jan 17, 2025
@con-f-use
Copy link

con-f-use commented Jan 17, 2025

You could have added that as a commit to my PR, but I'm fine with yours being merged. If my changes weren't a one-liner, I'd care more about proper attribution, just fyi.

@stevalkr
Copy link
Contributor Author

Hey @con-f-use, sorry I’m not so familiar with GitHub. It's off topic but do you mean I can clone your fork and push directly to it? There were a lot of commits between then and now, so I’d have to rebase master and force push a commit to your branch.

@con-f-use
Copy link

con-f-use commented Jan 18, 2025

Depends, if you're a maintainer of nix, you can just push to my branch, because I have "Allow edits by maintainers" checked.

image

It's off topic but do you mean I can clone your fork and push directly to it? There were a lot of commits between then and now, so I’d have to rebase master and force push a commit to your branch.

Yes, kinda. You can make a PR to my forked branch. You don't have to force push or care about the commits that have happened in the mean time if you simply add a new commit that just contains the tests. I.e. just adding the meson.build and symlink-paths.sh changes in a separate commit. You'll have to fork my fork, I think and then do a PR to that.

@roberth
Copy link
Member

roberth commented Jan 18, 2025

Alternatively, if you include the tip of @con-f-use's branch in this PR's history, and we merge this, then GitHub will correctly consider both PRs to be merged.
You could for example rebase your branch onto theirs to achieve this.

@stevalkr
Copy link
Contributor Author

Thanks a bunch, that’s cool to know!

Co-authored-by: Jan Christoph Bischko <[email protected]>
@Mic92 Mic92 force-pushed the flakes_symlink_path branch from 3f6b6c0 to 453e8dc Compare February 2, 2025 01:30
@Mic92
Copy link
Member

Mic92 commented Feb 2, 2025

@mergify queue

Copy link
Contributor

mergify bot commented Feb 2, 2025

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 24d11d2

@Mic92 Mic92 mentioned this pull request Feb 2, 2025
mergify bot added a commit that referenced this pull request Feb 2, 2025
mergify bot added a commit that referenced this pull request Feb 2, 2025
@mergify mergify bot merged commit 24d11d2 into NixOS:master Feb 2, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
with-tests Issues related to testing. PRs with tests have some priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: nix commands do not follow flake-url symlinks
4 participants