Skip to content
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.

Sign up for GitHub

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

Import from derivation: Better queries, better errors #31

Closed
wants to merge 12 commits into from
Closed

Import from derivation: Better queries, better errors #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
0bbd8c7
prim_import: Don't build derivations in read-only mode
shlevy May 16, 2012
9fc4cca
prim_import: Distinguish between failures due to read-only mode and o…
shlevy Aug 29, 2012
9763370
DrvInfo: Put the 'name' member behind a query/set interface
shlevy Jun 23, 2012
b83eab5
DrvInfo: Put the 'system' member behind a query/set interface
shlevy Jun 23, 2012
9ec740e
DrvInfo::queryMetaInfo(state, name): Don't evaluate all of meta just …
shlevy Jun 23, 2012
6f958bc
nix-env -q --print-description: Don't try to evaluate all of meta jus…
shlevy Jun 23, 2012
928f87f
getDerivations: add parameter to skip values that throw ImportReadOnl…
shlevy Jun 23, 2012
6f6f25f
isPrebuilt: Derivations whose outPaths are blocked by a read-only err…
shlevy Aug 29, 2012
36d0d9c
createMetaValue: Handle individual meta values blocked by ImportReadO…
shlevy Jun 23, 2012
efc58a3
nix-env -q: Gracefully bypass ImportReadOnlyErrors in outPath, drvPat…
shlevy Aug 29, 2012
f6db8fe
Make all handling of ImportReadOnlyErrors unconditional
shlevy Jun 23, 2012
a233da3
nix-env opQuery: If we can't get outPath due to an import from deriva…
shlevy Aug 29, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 63 additions & 23 deletions src/libexpr/get-drvs.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,29 @@
namespace nix {


string DrvInfo::queryName(EvalState & state) const
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for queryName() and querySystem()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So getDerivation() doesn't need to evaluate any of the derivations attributes, and to allow graceful handling of the case where name or system depend on an import from derivation in the future

{
if (name == "" && attrs) {
Bindings::iterator i = attrs->find(state.sName);
(string &) name = state.forceStringNoCtx(*i->value);
}
return name;
}


string DrvInfo::querySystem(EvalState & state) const
{
if (system == "" && attrs) {
Bindings::iterator i = attrs->find(state.sSystem);
if (i == attrs->end())
(string &) system = "unknown";
else
(string &) system = state.forceStringNoCtx(*i->value);
}
return system;
}


string DrvInfo::queryDrvPath(EvalState & state) const
{
if (drvPath == "" && attrs) {
Expand All @@ -27,33 +50,44 @@ string DrvInfo::queryOutPath(EvalState & state) const
return outPath;
}

static MetaValue createMetaValue(EvalState & state, Value & v)
{
MetaValue value;
try {
state.forceValue(v);
if (v.type == tString) {
value.type = MetaValue::tpString;
value.stringValue = v.string.s;
} else if (v.type == tInt) {
value.type = MetaValue::tpInt;
value.intValue = v.integer;
} else if (v.type == tList) {
value.type = MetaValue::tpStrings;
for (unsigned int j = 0; j < v.list.length; ++j)
value.stringValues.push_back(state.forceStringNoCtx(*v.list.elems[j]));
} else
value.type = MetaValue::tpNone;
} catch (ImportReadOnlyError & e) {
value.type = MetaValue::tpNone;
}
return value;
}

MetaInfo DrvInfo::queryMetaInfo(EvalState & state) const
{
if (metaInfoRead) return meta;

(bool &) metaInfoRead = true;
((MetaInfo &) meta).clear();

Bindings::iterator a = attrs->find(state.sMeta);
if (a == attrs->end()) return meta; /* fine, empty meta information */

state.forceAttrs(*a->value);

foreach (Bindings::iterator, i, *a->value->attrs) {
MetaValue value;
state.forceValue(*i->value);
if (i->value->type == tString) {
value.type = MetaValue::tpString;
value.stringValue = i->value->string.s;
} else if (i->value->type == tInt) {
value.type = MetaValue::tpInt;
value.intValue = i->value->integer;
} else if (i->value->type == tList) {
value.type = MetaValue::tpStrings;
for (unsigned int j = 0; j < i->value->list.length; ++j)
value.stringValues.push_back(state.forceStringNoCtx(*i->value->list.elems[j]));
} else continue;
((MetaInfo &) meta)[i->name] = value;
MetaValue value = createMetaValue(state, *i->value);
if (value.type != MetaValue::tpNone) ((MetaInfo &) meta)[i->name] = value;
}

return meta;
Expand All @@ -62,8 +96,19 @@ MetaInfo DrvInfo::queryMetaInfo(EvalState & state) const

MetaValue DrvInfo::queryMetaInfo(EvalState & state, const string & name) const
{
/* !!! evaluates all meta attributes => inefficient */
return queryMetaInfo(state)[name];
if (metaInfoRead) return ((MetaInfo &)meta)[name];
if (meta.count(name)) return ((MetaInfo &)meta)[name];

Bindings::iterator m = attrs->find(state.sMeta);
if (m == attrs->end()) return ((MetaInfo &)meta)[name];

state.forceAttrs(*m->value);
Bindings::iterator i = m->value->attrs->find(state.symbols.create(name));
if (i == m->value->attrs->end()) return ((MetaInfo &)meta)[name];
MetaValue value = createMetaValue(state, *i->value);
if (value.type != MetaValue::tpNone) ((MetaInfo &) meta)[name] = value;

return ((MetaInfo &)meta)[name];
}


Expand Down Expand Up @@ -100,13 +145,6 @@ static bool getDerivation(EvalState & state, Value & v,
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);

Bindings::iterator i2 = v.attrs->find(state.sSystem);
if (i2 == v.attrs->end())
drv.system = "unknown";
else
drv.system = state.forceStringNoCtx(*i2->value);

drv.attrs = v.attrs;

Expand All @@ -117,6 +155,8 @@ static bool getDerivation(EvalState & state, Value & v,

} catch (AssertionError & e) {
return false;
} catch (ImportReadOnlyError & e) {
return false;
}
}

Expand Down
16 changes: 14 additions & 2 deletions src/libexpr/get-drvs.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ typedef std::map<string, MetaValue> MetaInfo;
struct DrvInfo
{
private:
string name;
string system;
string drvPath;
string outPath;

Expand All @@ -35,20 +37,30 @@ private:
bool failed; // set if we get an AssertionError

public:
string name;
string attrPath; /* path towards the derivation */
string system;

/* !!! make this private */
Bindings * attrs;

DrvInfo() : metaInfoRead(false), failed(false), attrs(0) { };

string queryName(EvalState & state) const;
string querySystem(EvalState & state) const;
string queryDrvPath(EvalState & state) const;
string queryOutPath(EvalState & state) const;
MetaInfo queryMetaInfo(EvalState & state) const;
MetaValue queryMetaInfo(EvalState & state, const string & name) const;

void setName(const string & s)
{
name = s;
}

void setSystem(const string & s)
{
system = s;
}

void setDrvPath(const string & s)
{
drvPath = s;
Expand Down
1 change: 1 addition & 0 deletions src/libexpr/nixexpr.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ MakeError(ThrownError, AssertionError)
MakeError(Abort, EvalError)
MakeError(TypeError, EvalError)
MakeError(ImportError, EvalError) // error building an imported derivation
MakeError(ImportReadOnlyError, EvalError) // error when trying to import a derivation in read-only mode


/* Position objects. */
Expand Down
47 changes: 32 additions & 15 deletions src/libexpr/primops.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,24 +45,41 @@ static void prim_import(EvalState & state, Value * * args, Value & v)

foreach (PathSet::iterator, i, context) {
Path ctx = decodeContext(*i).first;
string outputName = decodeContext(*i).second;
assert(isStorePath(ctx));
if (!store->isValidPath(ctx))
throw EvalError(format("cannot import `%1%', since path `%2%' is not valid")
% path % ctx);
if (isDerivation(ctx))
try {
/* For performance, prefetch all substitute info. */
PathSet willBuild, willSubstitute, unknown;
unsigned long long downloadSize, narSize;
queryMissing(*store, singleton<PathSet>(ctx),
willBuild, willSubstitute, unknown, downloadSize, narSize);
if (!store->isValidPath(ctx)) {
if (outputName.empty())
throw EvalError(format("cannot import `%1%', since path `%2%' is not valid")
% path % ctx);
else
throw ImportReadOnlyError(format("cannot import `%1%', since path `%2%' cannot be written to the store in read-only mode")
% path % ctx);
}
if (isDerivation(ctx)) {
Derivation drv = derivationFromPath(*store, ctx);

if (outputName.empty() ||
!store->isValidPath(drv.outputs[outputName].path)) {
if (settings.readOnlyMode)
foreach (DerivationOutputs::iterator, j, drv.outputs)
if (!store->isValidPath(j->second.path))
throw ImportReadOnlyError(format("cannot import `%1%', since derivation `%2%' cannot be realised in read-only mode")
% path % ctx);
try {
/* For performance, prefetch all substitute info. */
PathSet willBuild, willSubstitute, unknown;
unsigned long long downloadSize, narSize;
queryMissing(*store, singleton<PathSet>(ctx),
willBuild, willSubstitute, unknown, downloadSize, narSize);

/* !!! If using a substitute, we only need to fetch
the selected output of this derivation. */
store->buildPaths(singleton<PathSet>(ctx));
} catch (Error & e) {
throw ImportError(e.msg());
/* !!! If using a substitute, we only need to fetch
the selected output of this derivation. */
store->buildPaths(singleton<PathSet>(ctx));
} catch (Error & e) {
throw ImportError(e.msg());
}
}
}
}

if (isStorePath(path) && store->isValidPath(path) && isDerivation(path)) {
Expand Down
Loading