Comment 2 by Richard Levitte, Jan 16, 2011
I experimented a bit with Timmothy's idea, but it became a bit
clumsy... I would rather think that an option argument for the
constructor database::database(app_state&), making it
database::database(app_state&,
database::options=database::options::none). database::options would
be an enum:
enum { none, maybe_unspecified } options;
All that's needed is to have a proper test for it in
database_path_helper::get_database_path.
Comment 3 by Richard Levitte, Jan 17, 2011
Have a look at revision 1680b6fb5779e66e0d1b6cea993e711bd9e01e66, I believe it answers quite a lot of the issue. It still needs a test for commands with --no-workspace. It's obvious most key commands fit in there, but are there other commands that should be checked as well? (stealing this ticket from tommyd ;-))
Owner:
levitte
Comment 4 by Timothy Brownawell, Jan 18, 2011
No other commands come to mind as needing to check. I'd prefer if dboptions was declared inside the database class (or a dboptions namespace), to not put a "none" value in the global namespace.
Comment 5 by Richard Levitte, Jan 18, 2011
Good suggestion, thank you. Latest changes: - dboptions moved into class database - tests/key_management_without_a_database corrected to really work without a database, and had a few more commands added for test. e3116ec35bd77e9419683c331cea14fcb1d2f3cb I think it's ready to get propagated back into net.venge.monotone, but would like a review by someone first.
Status:
Fixed
Summary: database-optional commands require a database now
Summary: database-optional commands require a database now
Comment 6 by Stephen Leake, Jan 23, 2011
looks good to me. the only failing tests (on MinGW) are fixed in main.
Comment 8 by Richard Levitte, Jan 23, 2011
Thanks. I've now propagated this back to net.venge.monotone.
Sign in to reply to this comment.
Reported by Timothy Brownawell, Jan 10, 2011
Steps to reproduce the problem: ------------------------------- 1. mtn privkey $KEY --no-workspace 2. mtn genkey foo@bar.com --no-workspace 3. mtn ls keys --no-workspace Expected result: ---------------- Those should all work. Actual results: --------------- mtn: misuse: no database specified Output of `mtn version --full`: ------------------------------- 3903ea47e7b1a656e0695f10d57bc0e9deca205b (current head; probably broken by the "don't assume :memory:" fix) There is a database::database_specified() that a few places use to check whether there really is a db. Previously constructing a database object without an actual database would work, and it would only error out when you tried to use it. Possibly instead we want something like class maybe_database { // we own it shared_ptr<database> const _db; // we don't own it database * const __db; public: explicit maybe_database(app_state & app) : _db(app.opts.dbname_given ? new database(app) : 0), __db(0) { } /*explicit*/ maybe_database(database & db) : __db(&db) { } bool exists() const { return _db || __db; } database & get() const { if (_db) return *_db; if (__db) return *__db; I(false); } }; and then things that currently call database::database_specified() would instead accept one of these.