Replace some bool recursive with a new FileIngestionMethod enum
#3455
Conversation
Ericson2314
force-pushed
the
enum-FileIngestionMethod
branch
from
2a4aea5
to
225e62a
Compare
src/libstore/path.hh
Outdated
| @@ -73,6 +73,11 @@ const size_t storePathHashLen = 32; // i.e. 160 bits | |||
| /* Extension of derivations in the Nix store. */ | |||
| const std::string drvExtension = ".drv"; | |||
|
|
|||
| enum struct FileIngestionMethod : bool { | |||
There was a problem hiding this comment.
Maybe FileIngestionMethod should PathIngestionMethod since it's more than just files. Or maybe PathCopyMode to be a bit less wordy.
There was a problem hiding this comment.
I wanted something shorter too, but naming it's hard:
-
PathIngestionMethodbut we are ingesting the file contents, not just the paths -
PathCopyModesometimes there are no nars involved and we aren't copying anything.
There was a problem hiding this comment.
One thing that will help is I hope to make a FileHash struct for the pair of the ingestion method and the hash; that is a shorter name, and will make a lot of functions take 1 arg less.
There was a problem hiding this comment.
@edolstra Do you have a name in mind? If not, we've now addressed your other points so I think this ready to go?
This is a different recursive than used in makeFixedOutputPath.
This is much less confusing since recursive is no longer a boolean.
There was an enum there that matched in perfectly.
|
Thanks, merged! |
|
@edolstra thank you!!! |
recursiveis a rather overloaded identifier in nix. There is the recursive demon enum, various one-off meanings like with traversing requisites or printing derivations, and the way we hash file paths. The daemon one already has an enum, and the one-offbool recursives are fine, but I think the "file ingestion method" ones are common enough that we ought to use a dedicated type for it. Furthermore---unlike the others which really are something juxtaposed with its negation---we just so happen to have two file ingestion methods today, and could easily have more.