lib: add callLocklessFlake

[MatthewCroughan]

This is essentially a copy of the function of the same name, from
flake-compat. callLocklessFlake is useful when trying to utilise a
flake.nix without a lock file, often for when you want to create a
subflake from within a parent flake.

Co-authored-by: Tom Bereknyei tomberek@gmail.com

Taken from: https://github.com/tomberek/relative/blob/0943a113c19e17b5f51c054de5233f2d3c97ce35/flake.nix#L32-L36
Inspired by: https://github.com/edolstra/flake-compat/blob/64a525ee38886ab9028e6f61790de0832aa3ef03/default.nix#L84-L88

Description of changes

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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/)
• 22.05 Release Notes (or backporting 21.11 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
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[Artturin]

Should this be in flake-utils repo instead of here?

[MatthewCroughan]

@Artturin It would be a real shame to depend on a library like flake-utils in any flake, than to be able to simply say nixpkgs.lib.callLocklessFlake. If this isn't merged, I'll just be using it in the let block of my outputs, like is the case in all of the examples https://github.com/NixOS/templates/blob/6296caa969420047c3de8b336f481106cf7af6f8/c-hello/flake.nix#L8

[roberth]

Seems useful.

Some thoughts in the file comments +

• please move this to a new lib/flakes.nix file. Trivial is the worst name. Might as well call it utils ;)
• add one or two tests in lib/tests
• format the new file with nixpkgs-fmt

[roberth]

Suggested change

	r = {outPath = path;} //
	self = {outPath = path;} //

[roberth]

{outPath = path;}

Canonicalize outPath? Depending on the type, might need something like builtins.path { name = "source": ... }.

Not sure, maybe this should be the caller's responsibility. If they want to avoid putting intermediate paths in the store, perhaps we shouldn't.

[tomberek]

In most cases I'd see this used via a sub-flake in a monorepo or another flake input which would already be canonicalized.

[roberth]

I agree. Let's not make the mistake again of doing too much.

[roberth]

After seeing that the current behavior does not match user expectations, I'm not convinced that we should leave this as is.

outPath = builtins.path { name = "source"; inherit path; } matches what flakes do iirc, and it could still be made optional afterwards.

Let's not make the mistake again of doing too much.

In this case the mistake has already been made, and we better follow along.
I do not get the impression that non-store flake sources to be implemented. @tomberek, wdyt?

[tomberek]

I don't have a strong opinion either way at the moment.

I'm not sure what the behavior is with builtins.path in term of multiple copies. I'm thinking of cases like a monorepo where a subdirectory already exists in the Store somewhere, just not as a top-level store path.

[roberth]

After some thought, this scenario seems rather serious:

1. user sees that outPath isn't copied, uses it for some sources imports, etc
2. user applies callLocklessFlake to directory with confidential files, which is safe to do
3. we change callLocklessFlake to match other users' expectations of calling builtins.path
4. user updates Nixpkgs
5. user unknowingly puts confidential files in the store or binary cache

A similar scenario exists for the opposite direction, where a store path size (and therefore closure size) can increase unknowingly.

It seems like it's a matter of time before someone comes along to make it do the "right" thing and enact (3), so let's preempt that by adding a copy parameter without a default, fixing the problem at (1). We can assign a default once flakes stabilize and when we know how this function is used in practice.

I'm not sure what the behavior is with builtins.path in term of multiple copies.

Subflakes always call it, so outPath is always a top-level path. It may not always do this in the future, but for now it seems that we should replicate that behavior. (Maybe I've been holding it wrong, or maybe this changes)
There's no need for the flake to be a subflake and normal flakes also put outPath in the store on way or another.

[MatthewCroughan]

@roberth I've done as you requested. All that's left to do is add a test in lib/tests. I will do this shortly.

[MatthewCroughan]

I've added what I think is a decent test, I'm not sure how i could do it better, so please advise on a better way to do it if there is one.

[adisbladis]

I think this is a very bad idea and I have reverted it in #173093.

[roberth]

I figured users of this function would anticipate breaking changes, as Flakes are clearly advertised to be experimental.
I do not think preemptively removing it is particularly productive, but I respect that other maintainers don't want to be responsible for this code.

NixOS/rfcs#82 has been mentioned as a solution in the other thread. I've offered to shepherd it some time ago. If you're interested, please join.

[MatthewCroughan]

I do not think it's fair to say that Flakes are experimental, given how many users there are, and the plan to stabilize them. They may be unstable, in terms of API and technicality, but I do not agree that they are experimental.

[roberth]

Flakes are deeply flawed when it comes to user facing design/implementation choices. If anything, they should be considered more experimental than they currently are. But maybe I should have more faith in upstream's courage to make (very) breaking changes.

That said, I'll reiterate:
I do not think preemptively removing it is particularly productive.

[tomberek]

I don't mind the revert for now. I'm working on a better replacement for this that will be strictly speaking more compliant with flakes/subflakes as well as a bit more powerful (allow dynamically generated input + follows attrsets).

[7c6f434c]

Usually, Nixpkgs cares about a strict subset of stable features of (non-latest, because all the interwoven release cycles) stable Nix. There are useful things easily doable in Nix but not expected to be done in Nixpkgs.

[adisbladis]

I do not think preemptively removing it is particularly productive.

Merging it in the first place set a bad precedent with far reaching implications for the stability expectation for all of lib.

[zimbatm]

Since this got reverted, I made a PR to add it to flake-utils. What do you think?

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/url-of-flake-inside-flake/32939/2