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

Jump to bottom

Make imports from unrealised derivations work well with queries #22

Closed
wants to merge 18 commits into from
Closed

Make imports from unrealised derivations work well with queries #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
dbce480
prim_import: Don't build derivations in read-only mode
shlevy May 16, 2012
7584a74
prim_import: Distinguish between failures due to read-only mode and o…
shlevy May 16, 2012
da038d9
get-drvs: Add infrastructure for queries to work around instatiate fa…
shlevy May 16, 2012
1f17a24
Store read-only errors in a fine-grained structured way rather than u…
shlevy Jun 5, 2012
3a5ecba
ErrorAttrs is a set, not a list
shlevy Jun 6, 2012
20040e1
loadDerivations: Don't filter out derivations whose 'system' attribut…
shlevy Jun 6, 2012
63a342d
When sorting DrvInfos by name, put derivations whose names can't be e…
shlevy Jun 7, 2012
2b89858
isPrebuilt: if outPath can't be evaluated in read-only mode, the path…
shlevy Jun 7, 2012
25be0c5
nix-env -q --status: Use exclamation points when information is unava…
shlevy Jun 7, 2012
c90b952
nix-env -q: Print !name-unknown-in-readonly-mode! when the name canno…
shlevy Jun 7, 2012
92e6e91
compareVersionAgainstSet: Any comparison that includes a derivation w…
shlevy Jun 7, 2012
b7b3e1f
compareVersionAgainstSet: Ensure 'version' is set before any potentia…
shlevy Jun 7, 2012
4738d90
nix-env -q: Print !system-unknown-in-readonly-mode! when the system c…
shlevy Jun 7, 2012
8dbe0cb
nix-env -q: Print !drvPath-unknown-in-readonly-mode! when the drvPath…
shlevy Jun 7, 2012
a9ebafd
nix-env -q: Print !outPath-unknown-in-readonly-mode! when the outPath…
shlevy Jun 7, 2012
6a6c729
nix-env -q: Handle the various ways meta attributes might throw reado…
shlevy Jun 7, 2012
5f1b71b
Remove tpNeedsRealise, metaError takes its place
shlevy Jun 7, 2012
63194a4
Whoops
shlevy Jun 7, 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
58 changes: 51 additions & 7 deletions src/libexpr/get-drvs.cc
View file
Open in desktop
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 {
Expand All @@ -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");
Copy link
Member

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?

Copy link
Member Author

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.

(string &) drvPath = "<derivation path unknown>";
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Member Author

Choose a reason for hiding this comment

The 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;
}
Expand All @@ -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;
}
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;

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

} catch (AssertionError & e) {
return false;
} catch (ImportReadOnlyError & e) {
if (!recoverFromReadOnlyErrors) throw;
return false;
}
}

Expand Down Expand Up @@ -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);
}
Expand Down
5 changes: 5 additions & 0 deletions src/libexpr/get-drvs.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ struct MetaValue

typedef std::map<string, MetaValue> MetaInfo;

typedef std::set<string> ErrorAttrs;

struct DrvInfo
{
Expand All @@ -37,6 +38,8 @@ public:
string name;
string attrPath; /* path towards the derivation */
string system;
ErrorAttrs error;
ErrorAttrs metaError;

/* !!! make this private */
Bindings * attrs;
Expand All @@ -50,11 +53,13 @@ public:

void setDrvPath(const string & s)
{
error.erase("drvPath");
drvPath = s;
}

void setOutPath(const string & s)
{
error.erase("outPath");
outPath = 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 @@ -17,6 +17,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
37 changes: 27 additions & 10 deletions src/libexpr/primops.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,35 @@ 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 {
/* !!! If using a substitute, we only need to fetch
the selected output of this derivation. */
store->buildDerivations(singleton<PathSet>(ctx));
} catch (Error & e) {
throw ImportError(e.msg());
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 (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 {
/* !!! If using a substitute, we only need to fetch
the selected output of this derivation. */
store->buildDerivations(singleton<PathSet>(ctx));
} catch (Error & e) {
throw ImportError(e.msg());
}
}
}
}

state.evalFile(path, v);
Expand Down
1 change: 1 addition & 0 deletions src/libstore/globals.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ Verbosity buildVerbosity = lvlError;
unsigned int maxBuildJobs = 1;
unsigned int buildCores = 1;
bool readOnlyMode = false;
bool recoverFromReadOnlyErrors = false;
string thisSystem = "unset";
time_t maxSilentTime = 0;
time_t buildTimeout = 0;
Expand Down
4 changes: 4 additions & 0 deletions src/libstore/globals.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ extern unsigned int buildCores;
database. */
extern bool readOnlyMode;

/* Whether failures due to read-only mode should cause a crash or allow
the tool to try to work around them. */
extern bool recoverFromReadOnlyErrors;

/* The canonical system name, as returned by config.guess. */
extern string thisSystem;

Expand Down
Loading