incremental builds: add derivation override functions

[messemar]

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.

[roberth]

Intriguing!

Some thoughts:

• buildOut: "out" is a little redundant
• the intermediate files are more likely not to be reproducible than normal outputs
• most of all: Please write tests

[messemar]

Intriguing!

Some thoughts:

* buildOut: "out" is a little redundant

* the intermediate files are more likely not to be reproducible than normal outputs

* most of all: Please write tests

Thanks for your input.

I have changed the names and added a test. Let me know If there is more to do for testing.

To your second concern:
I guess the intermediate files should be as reproducible as normal outputs, as they depends on the input and the compiler, also.
But I can not guarantee that they will be reproducible at any time.
I run the same derivation several times, and the derivation would not be rebuilt if nothing in the source tree changes.

[tfc]

This is pretty cool.

Interface wise i think it might be nicer to use and it seems like nothing speaks against doing it like this maybe?

incrementalDrv = pkgs.buildIncremental drv;

this could be done by implementing build-incremental.nix like this:

{ pkgs }:
let
  prepareIncrementalBuild = drv: ...;
  mkIncrementalBuild = drv: ...;

in

drv: previousArtifacts:

mkIncrementalBuild (prepareIncrementalBuild drv) previousArtifacts

The tests would look slightly different but would show the same thing.

[messemar]

This is pretty cool.

Interface wise i think it might be nicer to use and it seems like nothing speaks against doing it like this maybe?

incrementalDrv = pkgs.buildIncremental drv;

this could be done by implementing build-incremental.nix like this:

{ pkgs }:
let
  prepareIncrementalBuild = drv: ...;
  mkIncrementalBuild = drv: ...;

in

drv: previousArtifacts:

mkIncrementalBuild (prepareIncrementalBuild drv) previousArtifacts

The tests would look slightly different but would show the same thing.

You can not handle it that way, because there is no possibility to detect an earlier build automaticly, as nix is lacking the corresponding Support.

[tfc]

You can not handle it that way, because there is no possibility to detect an earlier build automaticly, as nix is lacking the corresponding Support.

Ok sorry for the noise. Now i get how it works.

[roberth]

• What happens when a source file is removed in the new build? It seems unavoidable that the accompanying intermediate file will be present, which is a risk, but a decent build script will only glob for source files.
  This needs a test case.

• In the existing test case, could you check that unmodified files aren't recompiled?

[messemar]

• What happens when a source file is removed in the new build? It seems unavoidable that the accompanying intermediate file will be present, which is a risk, but a decent build script will only glob for source files.
  This needs a test case.
  Yes, good Idea, I will add it
  • In the existing test case, could you check that unmodified files aren't recompiled?

Yes.

[messemar]

I've added the requested tests.

[YellowOnion]

Does this require two copies of src? this doesn't seem to create genuine incremental builds, but require one "unaltered" source, that acts as a seed for the 2nd derivation. This can be problematic when you want to work on big projects like linux source code.

[roberth]

Does this require two copies of src?

Yes.

this doesn't seem to create genuine incremental builds

That is correct. "True" incremental builds are a cross-cutting concern that goes all the way down to the build tool used inside the sandbox. As such, Nix can not be solely responsible for managing the state, making such a solution fragile, unless the build tool can delegate the state management to recursive nix, which is, still, rather restrictive and an invasive requirement.
Incrementality can't be added in a transparent manner, so instead, this is an embodiment of an incremental build within the derivation paradigm, that makes the prior state explicit and does not provide any guarantees beyond the fact that the result is an incremental build from a specific version to another specific version.

@messemar Could you explain this in the Nixpkgs manual?

This can be problematic when you want to work on big projects like linux source code.

If you buy into Nix, you have to be ok with "wasting" some space for the benefits of reproducibility, atomicity, etc.
There's two kinds of space waste: the temporary and the permanent. The first just causes garbage collection to run more frequently, the latter should not be affected by this solution.
When you're rebuilding, say, linux you are already copying the src to the store for each new iteration (unless you only change the expression), so presumably the garbage produced isn't much of a problem.
I guess I'm skipping over some subtleties, but I think this explanation is already getting quite long.

[messemar]

@messemar Could you explain this in the Nixpkgs manual?

Yes I can, when I have some spare time again I will. As well as continuing the implementation, of course :D

[messemar]

Steps to un-draft: Add a section in the nixpkgs manual.

[dasJ]

Code looks good and functionality is lovely to have (would have helped me bisecting the kernel a couple of days ago :)

[messemar]

@roberth @dasJ Is there anything left, that I can do to get this over the finishing line?

[dasJ]

There isn't

[dasJ]

Ah there is: @infinisil is it okay to dismiss your review?

[phip1611]

nit:

Suggested change

	# from an earlier build and a later one we store the state of the build
	# from an earlier build and a later one we store the state of the build

[phip1611]

Suggested change

	# After the build the build directory is copied again
	# After the build, the build directory is copied again

[phip1611]

Suggested change

	# Build tools, building in the source directory, instead of
	# build tools, building in the source directory, instead of

[phip1611]

Suggested change

	# having a build root directory, e.G the Linux kernel.
	# having a build root directory, e.g., the Linux kernel.

or

Suggested change

	# having a build root directory, e.G the Linux kernel.
	# having a build root directory, such as the Linux kernel.

[phip1611]

Awesome! Left some nits.

Stale, everything appears to be fixed

[infinisil]

Ah there is: @infinisil is it okay to dismiss your review?

Oh yeah sorry missed this. It's okay. Generally this does seem a rather hacky, but it looks like it's good enough for a lot of use cases, so it's fine.

@messemar: Though it looks like #167670 (comment) wasn't fully addressed, and I can't find a comment explaining why. Not very important though.

[roberth]

rather hacky

I'm sure you think the same about it, but to be clear, the goal itself is hacky, as something non-hacky would have to integrate with something other than phases perhaps, in order for it to be able to give guarantees that can't be messed up by the user of this feature, but it's unclear to what that would be.
The implementation is not at all hacky in and of itself.

[messemar]

@infinisil and @roberth
I totally agree on the hackyness. But as checkpoints and incremental builds are not a first class concept for nix, it can not be enabled in a non hacky way, I think.

Regarding #167670 (comment) If we use dotglob wouldn't that mean that we also remove ´.´ and ´..´ as well ? Does not sound like a good Idea IMHO. But yes, we might miss dotfiles then.

[infinisil]

@messemar You do have dotglob enabled in the PR. I was thinking of the other point in that comment that allows avoiding the extglob for rm -r !("sourceDifference.patch").

Which would then also work if the source has a sourceDifference.patch file itself.

[messemar]

@infinisil Ah, now I get it. Is it a nit, or should I fix it in a follow up PR ?

[infinisil]

@messemar I'd classify it as a nice-to-have for correctness. Feel free to make a follow-up PR, but not necessarily :)

[asymmetric]

Would it make sense to mention this in the 2023 recap? Or is it not supposed to be seen by too many people? 🐱

[roberth]

@asymmetric It needs an explicit starting state, and it only works for typical mkDerivation based builds. That should be made very clear. I would also like to see it used in practice somewhere.

[nixos-discourse]

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

https://discourse.nixos.org/t/idea-mkderivation-auto-generate-unwrapped-package-right-after-build/37901/3

[nixos-discourse]

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

https://discourse.nixos.org/t/idea-mkderivation-auto-generate-unwrapped-package-right-after-build/37901/1

[bryango]

I really, really like this idea and would like to use it in practice! However, there are some minor inconsistencies across the docs; e.g. mkCheckpointBuild is mentioned in the doc:

nixpkgs/doc/build-helpers/special/checkpoint-build.section.md

Lines 34 to 36 in d6b045f

	});
	in checkpointBuildTools.mkCheckpointBuild changedHello helloCheckpoint
	```

But the actual command is mkCheckpointedBuild with an ed.

In fact, all other functions in this PR are in the form of *CheckpointBuild* without the ed, so I think we might as well trade the mkCheckpointedBuild name in favor of mkCheckpointBuild for a uniformed look (and link mkCheckpointedBuild to mkCheckpointBuild with a lib.warn "deprecated", although currently there is no usage of this in nixpkgs). Is that okay? If so, I can make a PR!

Update: I made a draft here, along with other cosmetic changes.

[messemar]

In fact, all other functions in this PR are in the form of CheckpointBuild without the ed, so I think we might as well trade the mkCheckpointedBuild name in favor of mkCheckpointBuild for a uniformed look (and link mkCheckpointedBuild to mkCheckpointBuild with a lib.warn "deprecated", although currently there is no usage of this in nixpkgs). Is that okay? If so, I can make a PR!

@bryango From my perspective, the change is reasonable and I like it.

I would love to see a PR for this.