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

Impl: Propagate Flake inputs #6550

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

ysndr
Copy link
Member

@ysndr ysndr commented May 19, 2022

MVP implementation of #6549

@MagicRB
Copy link
Contributor

MagicRB commented May 24, 2022

This kind of relates to #610 and #5497 and any other similar issue

@thufschmitt
Copy link
Member

I've mixed feelings about that. I see the problem that it's solving, but I'm not entirely convinced by the solution. It would probably be useful for some use-cases like a multi-flakes repository, but would potentially be fairly confusing when used with external flakes ‑ because of the fact that it can or override the lockfile depending on whether the flake has an explicit input specification.

Maybe restricting it to lockless flakes in a first time like you suggest in #6549 would be better until we can come up with a good solution.

@thufschmitt
Copy link
Member

Maybe restricting it to lockless flakes in a first time like you suggest in #6549 would be better until we can come up with a good solution

An attribute to explicitely ignore a potential existing lockfile might be an idea, although that's a big hammer for this kind of precise thing. And that wouldn't work really nicely with transitive flakes either 🤔

@charlesbaynham
Copy link

Can I vote against

Maybe restricting it to lockless flakes in a first time like you suggest in #6549 would be better until we can come up with a good solution.

? I think a lot of the use of this will be overriding locks in input flakes which are locked in a way you don't like. An attribute to ignore lock files would be nice indeed. A big hammer would do lots of jobs, though not all 😁

@ysndr
Copy link
Member Author

ysndr commented Jun 2, 2022

I agree with @charlesbaynham. I think restricting this to only lockless flakes leaves too little use for it. I dont know if it would even work for local subflakes as their inputs are eventually locked as well by the importing flake.
Therefore, it would need to be constrained to completely lockless nix e.g. requiring --no-write-lockfile which has pretty limited application.

I agree people may be surprised if the nixpkgs that is locked with their flake is incompatible with the propagated / registered one. However I think that's acceptable as long as we make clear that you need explicitly add a nixpkgs input to keep it.
For third party flakes maybe its better to have an attribute to ignore the registered input instead (a shield from the hammer if you will 🛡):

(works better if we'd call it propagate )

inputs.nixpkgs.url = ...
inputs.nixpkgs.register = true;

inputs.someflake.url = ...; # <-- has registered nixpkgs

inputs.otherflake.url = ...;
inputs.otherflake.no-registered = true;

@thufschmitt
Copy link
Member

I agree people may be surprised if the nixpkgs that is locked with their flake is incompatible with the propagated / registered one. However I think that's acceptable as long as we make clear that you need explicitly add a nixpkgs input to keep it.

That's not really my fear. My issue on the contrary is that this creates an incentive for publishing “incomplete” flakes (in that their inputs won't be defined) since that would be the only way to have their input transparently overriden by a register when imported from a parent flake. And that would be very limiting.

For example, if I'm the author of a public flake that is developped using nixpkgs/nixos-22.05, should I set inputs.nixpkgs.url = "nixpkgs/nixos-22.05" (in which case downstream users can't use the register mechanism to override it), or should I leave it blank (in which case I can't easily lock it to the right version since it's gonna pick the nixpkgs from the registry which might not be what I want)? Neither case is fully satsifying.

That's why I thought about limiting this to lockless flakes since these have that implicit contract that… well, they aren't locked 🙃

I dont know if it would even work for local subflakes as their inputs are eventually locked as well by the importing flake.

Eventually they are, ofc, but afaik Nix distinguishes between a flake that's locked “by itself” and locked as part of the parent flake

@tomberek
Copy link
Contributor

tomberek commented Jun 2, 2022

From discussion:

  • can we limit this to lockless flakes? that and an experimental flag allows this to be easier to explore the design
  • interacts with local flakes
  • RFC?

@charlesbaynham
Copy link

For example, if I'm the author of a public flake that is developped using nixpkgs/nixos-22.05, should I set inputs.nixpkgs.url = "nixpkgs/nixos-22.05" (in which case downstream users can't use the register mechanism to override it), or should I leave it blank (in which case I can't easily lock it to the right version since it's gonna pick the nixpkgs from the registry which might not be what I want)? Neither case is fully satsifying.

Well true, but if my flake is picky enough about its inputs that using the latest nixpkgs will break it then specifying "22.05" helps communicate that to the user. If, on the other hand, I'm pinning my dependencies out of good practice but I don't expect severe compatibility problems in the future, I use "nixpkgs" so that later users can override my locked version with theirs.

@nrdxp
Copy link

nrdxp commented Jun 16, 2022

That's not really my fear. My issue on the contrary is that this creates an incentive for publishing “incomplete” flakes

FWIW, I agree with this sentiment, but my desired solution would be for the implementation to go even further by having register act on any input with the same name. So if I have a flake, and I set inputs.nixpkgs.register = true, I would want it to override every other input in the chain for any input named nixpkgs regardless if it's been specified or not. Maybe the change in semantic would require a change in name, so maybe something like inputs.nixpkgs.override = true?

In any case, something needs to be done here or there isn't much to do about #6626, assuming you do not have the authority to modify every upstream flake's input section without forking.

Perhaps one way to have both behaviors would be to add another flag to nix flake lock. Something like --only-registies which ignores the inputs section of a flake and expects every input to come from a registry value, or maybe make register accept 3 values: true, false or "force".

@roberth
Copy link
Member

roberth commented Jun 22, 2022

It seems that Nix has to finally incur a lot of the language-level dependency management complexity. These probably aren't problems that are solved by single PRs, but rather by collecting requirements and designing a more comprehensive solution. Though it's just a thought.

@blaggacao
Copy link
Contributor

blaggacao commented Jun 29, 2022

In the aforementioned spirit, I dropped this comment maybe in the wrong place: #6549 (comment)

It is worth noting that the described goal is probably not feasible without static code analysis.

Otoh, we have string contexts. Could we do identifier contexts that trace corresponding flake inputs reliably?

@stale stale bot added the stale label Jan 7, 2023
@stale stale bot removed the stale label Jun 14, 2023
@edolstra edolstra self-requested a review as a code owner November 12, 2024 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Friday Hacking Extravaganza
Development

Successfully merging this pull request may close these issues.

9 participants