Hash modulo meta#14686
Conversation
846a9c9 to
b2c772b
Compare
|
I didn't see this mentioned explicitly, but I assume It's not entirely obvious how derivation meta data would be used in practice, especially since derivers are kind of useless (e.g. cache.nixos.org paths do have a deriver, but you have no way to get to them, so you wouldn't be able to get to the metadata). Perhaps in conjunction with #11749, the metadata could be propagated into the provenance records of store paths (which are already JSON). |
Correct, and this is now also covered by a functional test.
You can instantiate your stuff and inspect the metadata.
So this is a good step towards making #11749 more effective.
|
01d6b9f to
1806b86
Compare
|
Marking as draft. |
|
Still fits the model. could probably improve some labels, but there ya go |
1806b86 to
4845fd6
Compare
|
My view is that we can land it now (needs review but idea approved), but before it is stabilized, it should be rolled into a "v2 input addressing" feature, which will no longer involve |
c9aa510 to
3324540
Compare
| /* Validate derivation-meta usage */ | ||
| if (drv->structuredAttrs.has_value()) { | ||
| auto & structuredAttrs = drv->structuredAttrs->structuredAttrs; | ||
| bool hasMeta = structuredAttrs.contains("__meta"); | ||
| bool hasFeature = usesDerivationMeta(*drv); | ||
|
|
||
| if (hasMeta && !hasFeature) { | ||
| throw Error( | ||
| "derivation '%s' has '__meta' attribute but does not require 'derivation-meta' system feature", | ||
| worker.store.printStorePath(drvPath)); | ||
| } | ||
|
|
||
| if (hasFeature) { | ||
| experimentalFeatureSettings.require(Xp::DerivationMeta); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
I would not put this here but instead make it part of the derivation options parsing logic. Then we will fail harder, which is good.
| // Only extract __meta if derivation-meta feature is used | ||
| if (hasDerivationMetaFeature(structuredAttrs)) { | ||
| return getObject(structuredAttrs.at("__meta")); | ||
| } |
There was a problem hiding this comment.
The error checking for __meta with the feature would go here.
| if (nix::hasDerivationMetaFeature(structuredAttrs)) { | ||
| auto metaIt = structuredAttrs.find("__meta"); | ||
| res["meta"] = metaIt->second; | ||
| structuredAttrs.erase(metaIt); |
There was a problem hiding this comment.
Hmm this is going to be a bit janky
There was a problem hiding this comment.
this crates a "have we parsed the options yet" race condition / state of sorts. We are better off eagerly extracting it at ATerm parse time as we do with structured attrs.
Make the meta field optional in DerivationOptions JSON deserialization to maintain backward compatibility with JSON that predates the derivation-meta feature. Changes: - Use optionalValueAt instead of valueAt for meta field deserialization - Mark meta as optional in derivation-options-v1 schema - Add unit test for backward compatibility
When the derivation-meta experimental feature is enabled, extract the __meta field from structuredAttrs to a top-level meta field in the Derivation JSON format (used by `nix derivation show/add`). This makes the format consistent with DerivationOptions JSON and provides better visibility of metadata without it being buried in structuredAttrs. The implementation: - Creates derivationToJson() helper accepting ExperimentalFeatureSettings - Extracts __meta to top-level meta field when feature is enabled - Requires experimental feature when deserializing JSON with meta field - Reconstructs __meta in structuredAttrs during deserialization - Maintains backward compatibility with JSON lacking meta field Tests verify both serialization and deserialization with mocked experimental feature settings, and functional tests ensure the experimental feature requirement is enforced.
Thank you fricklerhandwerk! Co-authored-by: Valentin Gagarin <valentin@gagarin.work>
It used to defer this to consumers. By performing the transformation immediately upon ingestion we avoid some confusion about the exact meaning of e.g. structuredAttrs at potentially many points in the code. We can now trust that it only has the "true" structured attributes that go into the builder. This may make ill-formed derivations harder to handle, but that is a price worth paying for added clarity going forward. Now, invalid __meta usage (missing experimental feature, unsorted requiredSystemFeatures) is caught immediately when the derivation is parsed or instantiated. To make re-injection into requiredSystemFeatures deterministic, it must be sorted when derivation-meta is used in instantiation. This avoids storing the original array position, and removes unnecessary entropy from derivations. Meta moves from DerivationOptions to BasicDerivation, since it is a derivation-level concept, not a build option.
Old daemons and remote builders don't understand derivation-meta, and would produce hash mismatch errors when receiving and validating such derivations. A new worker protocol feature marks support for ingesting these new derivations. The client checks for this feature in writeDerivation (for daemon) and build-remote (for remote builders) before a hash mismatch would occur, so that it can report a more specific error message.
The 'waiting for another Nix process to finish fetching input' message appears nondeterministically, causing the diff comparison to fail.
|
Motivation
metais lost at the derivation level, but it doesn't need to be that way, thanks to quotient hashing aka hash modulo.This change makes use of the existing "discrepancy" between derivation hashes and output hashes to allow more information to be stored in derivations without causing rebuilds.
The core of the change is fairly simple; see the
hashDerivationModulohunk.Changes
derivation-metaimproving hash quotient logic to support storingmetametaa first class top level attr instead of the ATerm embeddingNotes
Example
Context
meta.timeoutAdd 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.