From 64519cfd657d024ae6e2bb74cb21ad21b886fd2a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 3 Dec 2008 15:06:30 +0000 Subject: * Unify the treatment of sources copied to the store, and recursive SHA-256 outputs of fixed-output derivations. I.e. they now produce the same store path: $ nix-store --add x /nix/store/j2fq9qxvvxgqymvpszhs773ncci45xsj-x $ nix-store --add-fixed --recursive sha256 x /nix/store/j2fq9qxvvxgqymvpszhs773ncci45xsj-x the latter being the same as the path that a derivation derivation { name = "x"; outputHashAlgo = "sha256"; outputHashMode = "recursive"; outputHash = "..."; ... }; produces. This does change the output path for such fixed-output derivations. Fortunately they are quite rare. The most common use is fetchsvn calls with SHA-256 hashes. (There are a handful of those is Nixpkgs, mostly unstable development packages.) * Documented the computation of store paths (in store-api.cc). --- src/libexpr/primops.cc | 48 ++++++++++--------- src/libstore/local-store.cc | 11 +++-- src/libstore/local-store.hh | 6 +-- src/libstore/remote-store.cc | 9 ++-- src/libstore/remote-store.hh | 6 +-- src/libstore/store-api.cc | 111 ++++++++++++++++++++++++++++++++----------- src/libstore/store-api.hh | 12 ++--- src/nix-store/nix-store.cc | 5 +- src/nix-worker/nix-worker.cc | 4 +- tests/add.sh | 27 ++++++++--- tests/fixed.nix.in | 4 +- tests/fixed.sh | 27 +++++++++++ 12 files changed, 191 insertions(+), 79 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 4b7702effc..27ce28b8f7 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -215,6 +215,14 @@ static Expr prim_trace(EvalState & state, const ATermVector & args) *************************************************************/ +static bool isFixedOutput(const Derivation & drv) +{ + return drv.outputs.size() == 1 && + drv.outputs.begin()->first == "out" && + drv.outputs.begin()->second.hash != ""; +} + + /* Returns the hash of a derivation modulo fixed-output subderivations. A fixed-output derivation is a derivation with one output (`out') for which an expected hash and hash algorithm are @@ -227,27 +235,23 @@ static Expr prim_trace(EvalState & state, const ATermVector & args) function, we do not want to rebuild everything depending on it (after all, (the hash of) the file being downloaded is unchanged). So the *output paths* should not change. On the other hand, the - *derivation store expression paths* should change to reflect the - new dependency graph. + *derivation paths* should change to reflect the new dependency + graph. That's what this function does: it returns a hash which is just the - of the derivation ATerm, except that any input store expression + hash of the derivation ATerm, except that any input derivation paths have been replaced by the result of a recursive call to this - function, and that for fixed-output derivations we return - (basically) its outputHash. */ + function, and that for fixed-output derivations we return a hash of + its output path. */ static Hash hashDerivationModulo(EvalState & state, Derivation drv) { /* Return a fixed hash for fixed-output derivations. */ - if (drv.outputs.size() == 1) { + if (isFixedOutput(drv)) { DerivationOutputs::const_iterator i = drv.outputs.begin(); - if (i->first == "out" && - i->second.hash != "") - { - return hashString(htSHA256, "fixed:out:" - + i->second.hashAlgo + ":" - + i->second.hash + ":" - + i->second.path); - } + return hashString(htSHA256, "fixed:out:" + + i->second.hashAlgo + ":" + + i->second.hash + ":" + + i->second.path); } /* For other derivations, replace the inputs paths with recursive @@ -304,8 +308,7 @@ static Expr prim_derivationStrict(EvalState & state, const ATermVector & args) PathSet context; - string outputHash; - string outputHashAlgo; + string outputHash, outputHashAlgo; bool outputHashRecursive = false; for (ATermMap::const_iterator i = attrs.begin(); i != attrs.end(); ++i) { @@ -380,6 +383,7 @@ static Expr prim_derivationStrict(EvalState & state, const ATermVector & args) throw EvalError("required attribute `system' missing"); /* If an output hash was given, check it. */ + Path outPath; if (outputHash == "") outputHashAlgo = ""; else { @@ -398,6 +402,7 @@ static Expr prim_derivationStrict(EvalState & state, const ATermVector & args) % outputHash % outputHashAlgo); string s = outputHash; outputHash = printHash(h); + outPath = makeFixedOutputPath(outputHashRecursive, outputHashAlgo, h, drvName); if (outputHashRecursive) outputHashAlgo = "r:" + outputHashAlgo; } @@ -413,13 +418,12 @@ static Expr prim_derivationStrict(EvalState & state, const ATermVector & args) have an empty value. This ensures that changes in the set of output names do get reflected in the hash. */ drv.env["out"] = ""; - drv.outputs["out"] = - DerivationOutput("", outputHashAlgo, outputHash); + drv.outputs["out"] = DerivationOutput("", outputHashAlgo, outputHash); /* Use the masked derivation expression to compute the output path. */ - Path outPath = makeStorePath("output:out", - hashDerivationModulo(state, drv), drvName); + if (outPath == "") + outPath = makeStorePath("output:out", hashDerivationModulo(state, drv), drvName); /* Construct the final derivation store expression. */ drv.env["out"] = outPath; @@ -632,8 +636,8 @@ static Expr prim_filterSource(EvalState & state, const ATermVector & args) FilterFromExpr filter(state, args[0]); Path dstPath = readOnlyMode - ? computeStorePathForPath(path, false, false, "", filter).first - : store->addToStore(path, false, false, "", filter); + ? computeStorePathForPath(path, true, "sha256", filter).first + : store->addToStore(path, true, "sha256", filter); return makeStr(dstPath, singleton(dstPath)); } diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 9dcd134711..e015894b9f 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -670,14 +670,14 @@ void LocalStore::invalidatePath(const Path & path) } -Path LocalStore::addToStore(const Path & _srcPath, bool fixed, +Path LocalStore::addToStore(const Path & _srcPath, bool recursive, string hashAlgo, PathFilter & filter) { Path srcPath(absPath(_srcPath)); debug(format("adding `%1%' to the store") % srcPath); std::pair pr = - computeStorePathForPath(srcPath, fixed, recursive, hashAlgo, filter); + computeStorePathForPath(srcPath, recursive, hashAlgo, filter); Path & dstPath(pr.first); Hash & h(pr.second); @@ -696,10 +696,13 @@ Path LocalStore::addToStore(const Path & _srcPath, bool fixed, copyPath(srcPath, dstPath, filter); + /* !!! */ +#if 0 Hash h2 = hashPath(htSHA256, dstPath, filter); if (h != h2) throw Error(format("contents of `%1%' changed while copying it to `%2%' (%3% -> %4%)") % srcPath % dstPath % printHash(h) % printHash(h2)); +#endif canonicalisePathMetaData(dstPath); @@ -713,10 +716,10 @@ Path LocalStore::addToStore(const Path & _srcPath, bool fixed, } -Path LocalStore::addTextToStore(const string & suffix, const string & s, +Path LocalStore::addTextToStore(const string & name, const string & s, const PathSet & references) { - Path dstPath = computeStorePathForText(suffix, s, references); + Path dstPath = computeStorePathForText(name, s, references); addTempRoot(dstPath); diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 37ed543fc3..3d47446f6e 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -89,11 +89,11 @@ public: bool querySubstitutablePathInfo(const Path & substituter, const Path & path, SubstitutablePathInfo & info); - Path addToStore(const Path & srcPath, bool fixed = false, - bool recursive = false, string hashAlgo = "", + Path addToStore(const Path & srcPath, + bool recursive = true, string hashAlgo = "sha256", PathFilter & filter = defaultPathFilter); - Path addTextToStore(const string & suffix, const string & s, + Path addTextToStore(const string & name, const string & s, const PathSet & references); void exportPath(const Path & path, bool sign, diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 7e5fa2d865..f79b22310b 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -278,14 +278,15 @@ Path RemoteStore::queryDeriver(const Path & path) } -Path RemoteStore::addToStore(const Path & _srcPath, bool fixed, +Path RemoteStore::addToStore(const Path & _srcPath, bool recursive, string hashAlgo, PathFilter & filter) { Path srcPath(absPath(_srcPath)); writeInt(wopAddToStore, to); writeString(baseNameOf(srcPath), to); - writeInt(fixed ? 1 : 0, to); + /* backwards compatibility hack */ + writeInt((hashAlgo == "sha256" && recursive) ? 0 : 1, to); writeInt(recursive ? 1 : 0, to); writeString(hashAlgo, to); dumpPath(srcPath, to, filter); @@ -294,11 +295,11 @@ Path RemoteStore::addToStore(const Path & _srcPath, bool fixed, } -Path RemoteStore::addTextToStore(const string & suffix, const string & s, +Path RemoteStore::addTextToStore(const string & name, const string & s, const PathSet & references) { writeInt(wopAddTextToStore, to); - writeString(suffix, to); + writeString(name, to); writeString(s, to); writeStringSet(references, to); diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 969c8f9b62..cb9124a4cd 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -42,11 +42,11 @@ public: bool querySubstitutablePathInfo(const Path & path, SubstitutablePathInfo & info); - Path addToStore(const Path & srcPath, bool fixed = false, - bool recursive = false, string hashAlgo = "", + Path addToStore(const Path & srcPath, + bool recursive = true, string hashAlgo = "sha256", PathFilter & filter = defaultPathFilter); - Path addTextToStore(const string & suffix, const string & s, + Path addTextToStore(const string & name, const string & s, const PathSet & references); void exportPath(const Path & path, bool sign, diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index ced5fd0072..fe4ecfad54 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -99,55 +99,112 @@ void checkStoreName(const string & name) } +/* Store paths have the following form: + + /- + + where + + = the location of the Nix store, usually /nix/store + + = a human readable name for the path, typically obtained + from the name attribute of the derivation, or the name of the + source file from which the store path is created + + = base-32 representation of the first 160 bits of a SHA-256 + hash of ; the hash part of the store name + + = the string ":sha256:

::"; + note that it includes the location of the store as well as the + name to make sure that changes to either of those are reflected + in the hash (e.g. you won't get /nix/store/-name1 and + /nix/store/-name2 with equal hash parts). + + = one of: + "text:::..." + for plain text files written to the store using + addTextToStore(); ... are the references of the + path. + "source" + for paths copied to the store using addToStore() when recursive + = true and hashAlgo = "sha256" + "output:out" + for either the outputs created by derivations, OR paths copied + to the store using addToStore() with recursive != true or + hashAlgo != "sha256" (in that case "source" is used; it's + silly, but it's done that way for compatibility). + +

= base-16 representation of a SHA-256 hash of: + if = "text:...": + the string written to the resulting store path + if = "source": + the serialisation of the path from which this store path is + copied, as returned by hashPath() + if = "output:out": + for non-fixed derivation outputs: + the derivation (see hashDerivationModulo() in + primops.cc) + for paths copied by addToStore() or produced by fixed-output + derivations: + the string "fixed:out:::", where + = "r:" for recursive (path) hashes, or "" or flat + (file) hashes + = "md5", "sha1" or "sha256" + = base-16 representation of the path or flat hash of + the contents of the path (or expected contents of the + path for fixed-output derivations) + + It would have been nicer to handle fixed-output derivations under + "source", e.g. have something like "source:", 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 starting with + "output:out:", while the latter would have a <2> starting with + "source:". +*/ + + Path makeStorePath(const string & type, - const Hash & hash, const string & suffix) + const Hash & hash, const string & name) { /* e.g., "source:sha256:1abc...:/nix/store:foo.tar.gz" */ string s = type + ":sha256:" + printHash(hash) + ":" - + nixStore + ":" + suffix; + + nixStore + ":" + name; - checkStoreName(suffix); + checkStoreName(name); return nixStore + "/" + printHash32(compressHash(hashString(htSHA256, s), 20)) - + "-" + suffix; + + "-" + name; } Path makeFixedOutputPath(bool recursive, string hashAlgo, Hash hash, string name) { - /* !!! copy/paste from primops.cc */ - Hash h = hashString(htSHA256, "fixed:out:" - + (recursive ? (string) "r:" : "") + hashAlgo + ":" - + printHash(hash) + ":" - + ""); - return makeStorePath("output:out", h, name); + return hashAlgo == "sha256" && recursive + ? makeStorePath("source", hash, name) + : makeStorePath("output:out", hashString(htSHA256, + "fixed:out:" + (recursive ? (string) "r:" : "") + hashAlgo + ":" + printHash(hash) + ":"), + name); } std::pair computeStorePathForPath(const Path & srcPath, - bool fixed, bool recursive, string hashAlgo, PathFilter & filter) + bool recursive, string hashAlgo, PathFilter & filter) { - Hash h = hashPath(htSHA256, srcPath, filter); - - string baseName = baseNameOf(srcPath); - - Path dstPath; - - if (fixed) { - HashType ht(parseHashType(hashAlgo)); - Hash h2 = recursive ? hashPath(ht, srcPath, filter) : hashFile(ht, srcPath); - dstPath = makeFixedOutputPath(recursive, hashAlgo, h2, baseName); - } - - else dstPath = makeStorePath("source", h, baseName); - + HashType ht(parseHashType(hashAlgo)); + Hash h = recursive ? hashPath(ht, srcPath, filter) : hashFile(ht, srcPath); + string name = baseNameOf(srcPath); + Path dstPath = makeFixedOutputPath(recursive, hashAlgo, h, name); return std::pair(dstPath, h); } -Path computeStorePathForText(const string & suffix, const string & s, +Path computeStorePathForText(const string & name, const string & s, const PathSet & references) { Hash hash = hashString(htSHA256, s); @@ -159,7 +216,7 @@ Path computeStorePathForText(const string & suffix, const string & s, type += ":"; type += *i; } - return makeStorePath(type, hash, suffix); + return makeStorePath(type, hash, name); } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index ed7e601460..adfd40a919 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -173,13 +173,13 @@ public: derivation is pre-loaded into the Nix store. The function object `filter' can be used to exclude files (see libutil/archive.hh). */ - virtual Path addToStore(const Path & srcPath, bool fixed = false, - bool recursive = false, string hashAlgo = "", + virtual Path addToStore(const Path & srcPath, + bool recursive = true, string hashAlgo = "sha256", PathFilter & filter = defaultPathFilter) = 0; /* Like addToStore, but the contents written to the output path is a regular file containing the given string. */ - virtual Path addTextToStore(const string & suffix, const string & s, + virtual Path addTextToStore(const string & name, const string & s, const PathSet & references) = 0; /* Export a store path, that is, create a NAR dump of the store @@ -274,7 +274,7 @@ Path followLinksToStorePath(const Path & path); /* Constructs a unique store path name. */ Path makeStorePath(const string & type, - const Hash & hash, const string & suffix); + const Hash & hash, const string & name); Path makeFixedOutputPath(bool recursive, string hashAlgo, Hash hash, string name); @@ -285,7 +285,7 @@ Path makeFixedOutputPath(bool recursive, Returns the store path and the cryptographic hash of the contents of srcPath. */ std::pair computeStorePathForPath(const Path & srcPath, - bool fixed = false, bool recursive = false, string hashAlgo = "", + bool recursive = true, string hashAlgo = "sha256", PathFilter & filter = defaultPathFilter); /* Preparatory part of addTextToStore(). @@ -302,7 +302,7 @@ std::pair computeStorePathForPath(const Path & srcPath, simply yield a different store path, so other users wouldn't be affected), but it has some backwards compatibility issues (the hashing scheme changes), so I'm not doing that for now. */ -Path computeStorePathForText(const string & suffix, const string & s, +Path computeStorePathForText(const string & name, const string & s, const PathSet & references); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 63635b52d3..e03e822212 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -129,7 +129,7 @@ static void opAddFixed(Strings opFlags, Strings opArgs) opArgs.pop_front(); for (Strings::iterator i = opArgs.begin(); i != opArgs.end(); ++i) - cout << format("%1%\n") % store->addToStore(*i, true, recursive, hashAlgo); + cout << format("%1%\n") % store->addToStore(*i, recursive, hashAlgo); } @@ -151,6 +151,9 @@ static void opPrintFixedPath(Strings opFlags, Strings opArgs) if (*i == "--recursive") recursive = true; else throw UsageError(format("unknown flag `%1%'") % *i); + if (opArgs.size() != 3) + throw UsageError(format("`--print-fixed-path' requires three arguments")); + Strings::iterator i = opArgs.begin(); string hashAlgo = *i++; string hash = *i++; diff --git a/src/nix-worker/nix-worker.cc b/src/nix-worker/nix-worker.cc index 1ba74f46fe..e4fe94926b 100644 --- a/src/nix-worker/nix-worker.cc +++ b/src/nix-worker/nix-worker.cc @@ -290,7 +290,7 @@ static void performOp(unsigned int clientVersion, case wopAddToStore: { /* !!! uberquick hack */ string baseName = readString(from); - bool fixed = readInt(from) == 1; + readInt(from); /* obsolete; was `fixed' flag */ bool recursive = readInt(from) == 1; string hashAlgo = readString(from); @@ -300,7 +300,7 @@ static void performOp(unsigned int clientVersion, restorePath(tmp2, from); startWork(); - Path path = store->addToStore(tmp2, fixed, recursive, hashAlgo); + Path path = store->addToStore(tmp2, recursive, hashAlgo); stopWork(); writeString(path, to); diff --git a/tests/add.sh b/tests/add.sh index 83092b26c1..7ea4cb6d6a 100644 --- a/tests/add.sh +++ b/tests/add.sh @@ -1,13 +1,28 @@ source common.sh -file=./add.sh +path1=$($nixstore --add ./dummy) +echo $path1 -path=$($nixstore --add $file) +path2=$($nixstore --add-fixed sha256 --recursive ./dummy) +echo $path2 -echo $path +if test "$path1" != "$path2"; then + echo "nix-store --add and --add-fixed mismatch" + exit 1 +fi -hash=$($nixstore -q --hash $path) +path3=$($nixstore --add-fixed sha256 ./dummy) +echo $path3 +test "$path1" != "$path3" || exit 1 -echo $hash +path4=$($nixstore --add-fixed sha1 --recursive ./dummy) +echo $path4 +test "$path1" != "$path4" || exit 1 -test "$hash" = "sha256:$($nixhash --type sha256 --base32 $file)" +hash1=$($nixstore -q --hash $path1) +echo $hash1 + +hash2=$($nixhash --type sha256 --base32 ./dummy) +echo $hash2 + +test "$hash1" = "sha256:$hash2" diff --git a/tests/fixed.nix.in b/tests/fixed.nix.in index 5d364eb39e..ad5306cf6a 100644 --- a/tests/fixed.nix.in +++ b/tests/fixed.nix.in @@ -20,7 +20,6 @@ rec { (f ./fixed.builder1.sh "flat" "sha1" "a0b65939670bc2c010f4d5d6a0b3e4e4590fb92b") (f ./fixed.builder2.sh "recursive" "md5" "3670af73070fa14077ad74e0f5ea4e42") (f ./fixed.builder2.sh "recursive" "sha1" "vw46m23bizj4n8afrc0fj19wrp7mj3c0") - (f ./fixed.builder2.sh "recursive" "sha256" "1ixr6yd3297ciyp9im522dfxpqbkhcw0pylkb2aab915278fqaik") ]; good2 = [ @@ -30,6 +29,9 @@ rec { (f ./fixed.builder2.sh "flat" "md5" "8ddd8be4b179a529afa5f2ffae4b9858") ]; + sameAsAdd = + f ./fixed.builder2.sh "recursive" "sha256" "1ixr6yd3297ciyp9im522dfxpqbkhcw0pylkb2aab915278fqaik"; + bad = [ (f ./fixed.builder1.sh "flat" "md5" "0ddd8be4b179a529afa5f2ffae4b9858") ]; diff --git a/tests/fixed.sh b/tests/fixed.sh index 72038adfce..9d5de2929b 100644 --- a/tests/fixed.sh +++ b/tests/fixed.sh @@ -34,3 +34,30 @@ clearStore drvs=$($nixinstantiate fixed.nix -A parallelSame) echo $drvs $nixstore -r $drvs -j2 + +# Fixed-output derivations with a recursive SHA-256 hash should +# produce the same path as "nix-store --add". +echo 'testing sameAsAdd...' +drv=$($nixinstantiate fixed.nix -A sameAsAdd) +echo $drv +out=$($nixstore -r $drv) +echo $out + +# This is what fixed.builder2 produces... +rm -rf $TEST_ROOT/fixed +mkdir $TEST_ROOT/fixed +mkdir $TEST_ROOT/fixed/bla +echo "Hello World!" > $TEST_ROOT/fixed/foo +ln -s foo $TEST_ROOT/fixed/bar + +out2=$($nixstore --add $TEST_ROOT/fixed) +echo $out2 +test "$out" = "$out2" || exit 1 + +out3=$($nixstore --add-fixed --recursive sha256 $TEST_ROOT/fixed) +echo $out3 +test "$out" = "$out3" || exit 1 + +out4=$($nixstore --print-fixed-path --recursive sha256 "1ixr6yd3297ciyp9im522dfxpqbkhcw0pylkb2aab915278fqaik" fixed) +echo $out4 +test "$out" = "$out4" || exit 1 -- cgit 1.4.1