Include store path exact spec in the docs #9295
Conversation
Ericson2314
requested review from
thufschmitt and
fricklerhandwerk
as code owners
github-actions
bot
added
the
store
|
Hm, wouldn't it make more sense to make the doxygen output more prominent for the purpose of reimplementing things? |
|
Perhaps? Right many separate functions create different types of store paths, whereas this comment wasn't associated with any one of function but described all of them / the big picture. |
Ericson2314
force-pushed
the
store-path-complete-construction
branch
2 times, most recently
from
3985e7c to
95b83f0
Compare
|
I do think this should go in the main manual (though appendix/protocols section rather than this is OK) because at the end of the data it is describing Nix's behavior --- what do valid store paths look like? --- not the private details of a specific implementation. |
Ericson2314
force-pushed
the
store-path-complete-construction
branch
from
95b83f0 to
f017598
Compare
Ericson2314
force-pushed
the
store-path-complete-construction
branch
2 times, most recently
from
461b4a2 to
1b65758
Compare
| Note that since an output derivation has always type output, while | ||
| something added by addToStore can have type output or source depending | ||
| on the hash, this means that the same input can be hashed differently | ||
| if added to the store via addToStore or via a derivation, in the sha256 | ||
| recursive case. | ||
|
|
||
| It would have been nicer to handle fixed-output derivations under | ||
| "source", e.g. have something like "source:<rec><algo>", but we're | ||
| stuck with this for now... | ||
|
|
||
| The main reason for this way of computing names is to prevent name | ||
| collisions (for security). For instance, it shouldn't be feasible | ||
| to come up with a derivation whose output path collides with the | ||
| path for a copied source. The former would have a <s> starting with | ||
| "output:out:", while the latter would have a <s> starting with | ||
| "source:". |
There was a problem hiding this comment.
Note that the first paragraph at the end is wrong, and the second two are misleading. That is why the new version (the historical note) looks quite different.
Ericson2314
force-pushed
the
store-path-complete-construction
branch
2 times, most recently
from
e85fff5 to
9dd6fa8
Compare
There was a problem hiding this comment.
I'm not fully convinced yet that this is an improvement, since it breaks locality of implementation comments. Assuming the main intention of this change is to increase discoverability, an alternative would be exposing the C++ API docs in a subdirectory of the manual and linking to that piece from the regular store path page of the manual.
But indeed it's high-level enough to have a place in the protocols chapter. I'd want to make sure though that the high level overview doesn't duplicate too much of it.
src/libstore/store-api.cc
Outdated
| "output:out:", while the latter would have a <s> starting with | ||
| "source:". | ||
| /* | ||
| The algorithm here is documented in the Store section of the Nix |
There was a problem hiding this comment.
If we really move that to the manual, make that a concrete reference. A mechanism like tagref would come in handy here... in fact, maybe we can just place an ad hoc tag reference here, even if there's no automatic processing. It can still be searched for in the source.
There was a problem hiding this comment.
I wrote protocols/store-path.md in a new version of the comment. (The new version also is clearer about outward-visible spec vs mere implementation details; the protocol documentation should not be updated for the latter as they are out of scope.)
Ericson2314
force-pushed
the
store-path-complete-construction
branch
2 times, most recently
from
c9dc07a to
072e750
Compare
Done, it's moved there.
I don't anything does?
Well I don't think this is very tied to the implement. The implementation works "forwards", i.e. "given some information a store object, compute its store path. This spec reads more "backwards", like a grammar for store paths. If one could magically invert the hashing function, it looks more like a parser for store paths than a pretty-printer. |
This is niche, but deserves to be in the manual because it is describing behavior visible to the outside world, not mere implementation details.
Ericson2314
force-pushed
the
store-path-complete-construction
branch
from
072e750 to
a34ec0b
Compare
|
Does this look good to you now, @fricklerhandwerk? |
| Since `64519cfd657d024ae6e2bb74cb21ad21b886fd2a` (2008), however, it was decided that separting derivation-produced vs manually-hashed content-addressed data like this was not useful. | ||
| Now, data this is to be SHA-256 + NAR-serialization content-addressed always uses the `source:...` construction, regardless of how it was produced (manually or by derivation). | ||
| This allows freely switching between using [fixed-output derivations](@docroot@/glossary.md#gloss-fixed-output-derivation) for fetching, and fetching out-of-band and then manually adding. | ||
| It also removes the ambiguity from the grammar. |
There was a problem hiding this comment.
Not sure if we should reference future possibilities, but
Co-authored-by: Robert Hensing <[email protected]>
As suggested by @roberth in #9295 (comment). Thanks!
Co-authored-by: Robert Hensing <[email protected]>
Co-authored-by: Robert Hensing <[email protected]>
Motivation
This is niche, but deserves to be in the manual because it is describing behavior visible to the outside world, not mere implementation details.
Context
I think this isn't quite ready until some other things are documented first, like the NAR format, and how file system objects can be content addressed more generally.Nah let's just get this over with. It's fine.
I also want this to have some sort of "advanced" disclaimer.Done
Priorities
Add 👍 to pull requests you find important.