-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make imports from unrealised derivations work well with queries #22
Changes from all commits
dbce480
7584a74
da038d9
1f17a24
3a5ecba
20040e1
63a342d
2b89858
25be0c5
c90b952
92e6e91
b7b3e1f
4738d90
8dbe0cb
a9ebafd
6a6c729
5f1b71b
63194a4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
#include "get-drvs.hh" | ||
#include "util.hh" | ||
#include "eval-inline.hh" | ||
#include "globals.hh" | ||
|
||
|
||
namespace nix { | ||
|
@@ -11,7 +12,13 @@ string DrvInfo::queryDrvPath(EvalState & state) const | |
if (drvPath == "" && attrs) { | ||
Bindings::iterator i = attrs->find(state.sDrvPath); | ||
PathSet context; | ||
(string &) drvPath = i != attrs->end() ? state.coerceToPath(*i->value, context) : ""; | ||
try { | ||
(string &) drvPath = i != attrs->end() ? state.coerceToPath(*i->value, context) : ""; | ||
} catch (ImportReadOnlyError & e) { | ||
if (!recoverFromReadOnlyErrors) throw; | ||
((ErrorAttrs &) error).insert("drvPath"); | ||
(string &) drvPath = "<derivation path unknown>"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An exceptional value like "<derivation path unknown>" is scary to me because a caller might actually try to use that value. Why not just handle the ImportReadOnlyError exception in printTable() in nix-env.cc? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok. I was just trying to make it so drvPath wasn't evaluated again if queryDrvPath is called again, but since drvPath/outPath are already wrapped in getters as described above I don't need to catch the exception at all here. |
||
} | ||
} | ||
return drvPath; | ||
} | ||
|
@@ -22,7 +29,13 @@ string DrvInfo::queryOutPath(EvalState & state) const | |
if (outPath == "" && attrs) { | ||
Bindings::iterator i = attrs->find(state.sOutPath); | ||
PathSet context; | ||
(string &) outPath = i != attrs->end() ? state.coerceToPath(*i->value, context) : ""; | ||
try { | ||
(string &) outPath = i != attrs->end() ? state.coerceToPath(*i->value, context) : ""; | ||
} catch (ImportReadOnlyError & e) { | ||
if (!recoverFromReadOnlyErrors) throw; | ||
((ErrorAttrs &) error).insert("drvPath"); | ||
(string &) outPath = "<output path unknown>"; | ||
} | ||
} | ||
return outPath; | ||
} | ||
|
@@ -37,11 +50,23 @@ MetaInfo DrvInfo::queryMetaInfo(EvalState & state) const | |
Bindings::iterator a = attrs->find(state.sMeta); | ||
if (a == attrs->end()) return meta; /* fine, empty meta information */ | ||
|
||
state.forceAttrs(*a->value); | ||
try { | ||
state.forceAttrs(*a->value); | ||
} catch (ImportReadOnlyError & e) { | ||
if (!recoverFromReadOnlyErrors) throw; | ||
((ErrorAttrs &) error).insert("meta"); | ||
return meta; | ||
} | ||
|
||
foreach (Bindings::iterator, i, *a->value->attrs) { | ||
MetaValue value; | ||
state.forceValue(*i->value); | ||
try { | ||
state.forceAttrs(*a->value); | ||
} catch (ImportReadOnlyError & e) { | ||
if (!recoverFromReadOnlyErrors) throw; | ||
((ErrorAttrs &) metaError).insert(i->name); | ||
continue; | ||
} | ||
if (i->value->type == tString) { | ||
value.type = MetaValue::tpString; | ||
value.stringValue = i->value->string.s; | ||
|
@@ -69,6 +94,8 @@ MetaValue DrvInfo::queryMetaInfo(EvalState & state, const string & name) const | |
|
||
void DrvInfo::setMetaInfo(const MetaInfo & meta) | ||
{ | ||
error.erase("meta"); | ||
metaError.clear(); | ||
metaInfoRead = true; | ||
this->meta = meta; | ||
} | ||
|
@@ -96,17 +123,27 @@ static bool getDerivation(EvalState & state, Value & v, | |
done.insert(v.attrs); | ||
|
||
DrvInfo drv; | ||
|
||
Bindings::iterator i = v.attrs->find(state.sName); | ||
/* !!! We really would like to have a decent back trace here. */ | ||
if (i == v.attrs->end()) throw TypeError("derivation name missing"); | ||
drv.name = state.forceStringNoCtx(*i->value); | ||
try { | ||
drv.name = state.forceStringNoCtx(*i->value); | ||
} catch (ImportReadOnlyError & e) { | ||
if (!recoverFromReadOnlyErrors) throw; | ||
drv.error.insert("name"); | ||
} | ||
|
||
Bindings::iterator i2 = v.attrs->find(state.sSystem); | ||
if (i2 == v.attrs->end()) | ||
drv.system = "unknown"; | ||
else | ||
drv.system = state.forceStringNoCtx(*i2->value); | ||
try { | ||
drv.system = state.forceStringNoCtx(*i2->value); | ||
} catch (ImportReadOnlyError & e) { | ||
if (!recoverFromReadOnlyErrors) throw; | ||
drv.error.insert("system"); | ||
} | ||
|
||
drv.attrs = v.attrs; | ||
|
||
|
@@ -117,6 +154,9 @@ static bool getDerivation(EvalState & state, Value & v, | |
|
||
} catch (AssertionError & e) { | ||
return false; | ||
} catch (ImportReadOnlyError & e) { | ||
if (!recoverFromReadOnlyErrors) throw; | ||
return false; | ||
} | ||
} | ||
|
||
|
@@ -179,6 +219,10 @@ static void getDerivations(EvalState & state, Value & vIn, | |
attribute. */ | ||
if (v2.type == tAttrs) { | ||
Bindings::iterator j = v2.attrs->find(state.symbols.create("recurseForDerivations")); | ||
/* !!! in the weird case where v2.recurseForDerivations | ||
requires evaluating an import of an unrealised derivation | ||
to be evaluated, this will throw in read-only | ||
mode even if read-only recovery was specified */ | ||
if (j != v2.attrs->end() && state.forceBool(*j->value)) | ||
getDerivations(state, v2, pathPrefix2, autoArgs, drvs, done); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the ErrorAttrs for? Is it actually necessary to remember that an attribute failed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's only necessary because there are times when we evaluate multiple attributes at once. In particular I'm thinking of getDerivation and queryMetaInfo: If, say, name can be determined in read-only mode but system can't, there's no way for opQuery to get the name without breaking the abstraction of the DrvInfo. It's not really necessary for queryOutPath and queryDrvPath, but since the data structure was already there I thought we might as well use it.
An alternative that will make it possible to keep get-drvs.cc ignorant of ImportReadOnlyErrors is to create a getter interface for name, system, and the meta attributes along the lines of queryOutPath and queryDrvPath. There is supposedly one for meta attributes but since the entire set of meta attributes is evaluated at one time it's not quite enough if we want to extract all the information possible. I'll switch to that unless I hear otherwise.