libmirserver parses arguments and fails if it's not something it understands

[Michał Sawicz]

$ unity8 -testability
terminate called after throwing an instance of 'boost::exception_detail::clone_impl<boost::exception_detail::error_info_injector<mir::AbnormalExit> >'
  what(): Failed to parse command line options: unrecognised option '-testability'.
Command-line options.
Environment variables capitalise long form with prefix "MIR_SERVER_" and "_" in place of "-":
  --nested-mode arg Run mir in nested mode. Host socket filename.
  -f [ --file ] arg Socket filename
  --platform-graphics-lib arg Library to use for platform graphics support
                                [default=libmirplatformgraphics.so
  -i [ --enable-input ] arg Enable input. [bool:default=true]
  --display-report arg How to handle the Display report.
                                [{log,off}:default=off]
  --input-report arg How to handle to Input report.
                                [{log,lttng,off}:default=off]
  --legacy-input-report arg How to handle the Legacy Input report.
                                [{log,off}:default=off]
  --session-mediator-report arg How to handle the SessionMediator report.
                                [{log,off}:default=off]
  --msg-processor-report arg How to handle the MessageProcessor report.
                                [{log,lttng,off}:default=off]
  --glog Use google::GLog for logging
  --glog-stderrthreshold arg Copy log messages at or above this level to
                                stderr in addition to logfiles. The numbers of
                                severity levels INFO, WARNING, ERROR, and FATAL
                                are 0, 1, 2, and 3, respectively.
                                [int:default=2]
  --glog-minloglevel arg Log messages at or above this level. The
                                numbers of severity levels INFO, WARNING,
                                ERROR, and FATAL are 0, 1, 2, and 3,
                                respectively. [int:default=0]
  --glog-log-dir arg If specified, logfiles are written into this
                                directory instead of the default logging
                                directory. [string:default=""]
  --ipc-thread-pool arg threads in frontend thread pool.
                                [int:default=10]
  --vt arg VT to run on or 0 to use current.
                                [int:default=0]
  -h [ --help ] this help text

Aborted (core dumped)

ProblemType: Crash
DistroRelease: Ubuntu 13.10
Package: unity8 7.81.3+13.10.20130912-0ubuntu1
Uname: Linux 3.4.0-3-mako armv7l
ApportVersion: 2.12.1-0ubuntu4
Architecture: armhf
Date: Mon Sep 16 19:19:51 2013
ExecutablePath: /usr/bin/unity8
InstallationDate: Installed on 2013-09-16 (0 days ago)
InstallationMedia: Ubuntu Saucy Salamander (development branch) - armhf (20130916.2)
MarkForUpload: True
ProcCmdline: unity8 -testability
Signal: 6
SourcePackage: unity8
UpgradeStatus: No upgrade log present (probably fresh install)
UserGroups: adm autopilot cdrom dialout dip plugdev sudo tty video

[Apport retracing service (apport)]

Stacktrace:
 #0 <unavailable> in ?? ()
 PC not available
StacktraceTop: <unavailable> in ?? ()
ThreadStacktrace: PC not available

[Robert Ancell (robert-ancell)]

Existing Mir code uses mir::report_exception() to print exceptions out nicely.

Discussed with Michał, the request on Mir is that it can ignore options it doesn't understand so Qt can be initialised with the option list after Mir has scanned it.

[Robert Ancell (robert-ancell)]

This should be possible to solve since Mir revision 1069.

You can override DefaultServerConfiguration::parse_options() and parse the arguments yourself without throwing an exception. You would need to copy some code from src/server/default_server_configuration.cpp to do this and keep the environment parsing.

It's not clear to me if the API should be easier than this however.

[Robert Ancell (robert-ancell)]

Relevant boost code to allow unknown options:
http://www.boost.org/doc/libs/1_54_0/doc/html/program_options/howto.html#idp123440592

[Gerry Boland (gerboland)]

Well I'd request by default that Mir, as a library, only listens for the command line arguments it recognises, and ignores all others. Qt are GTK libraries which do this.

While the Mir default behaviour can be overridden by unity-mir, I think it makes more sense for the change to be in Mir itself.

[Alan Griffiths (alan-griffiths)]

In principle I don't think it is a good idea to ignore unrecognised arguments on the command line - that only leads to confusion when users misspell something. Having two separate parts of the program parsing them is the real problem Mir was coded on the misguided assumption that boost.Options was good enough for all client code.

I think a better solution would be to decouple the creation of the options object from the initialisation of DefaultServerConfiguration. That way options can be passed directly to DefaultServerConfiguration without any reference to the command line. Clearly we'd continue to provide a default option parser. (Maybe this could return the arguments it doesn't interpret?)

There is a precedent for manipulating the "arguments" to get around this - the following comes from the Mir test framework:

int main(int argc, char** argv)
{
    ::argc = std::remove_if(
        argv,
        argv+argc,
        [](char const* arg) { return !strncmp(arg, "--gtest_", 8); }) - argv;
    ::argv = const_cast<char const**>(argv);

  ::testing::InitGoogleTest(&argc, argv);

  return RUN_ALL_TESTS();
}

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Michael Terry (mterry)]

Looks like unity-mir would really need to re-implement mir::options::ProgramOption and pass our own version back in an overridden version of the_options().

ProgramOption actually makes the call to parse_command_line() internally, its variable_maps instance that holds all the parsed options is private, and it doesn't expose any other way to set the options. So that's where the action is.

Unfortunately, we'd have to just copy the chunks of code for parse_environment() and parse_file().

[Michael Terry (mterry)]

Oh, but I guess we can't do that either, because parse_arguments() is only in mir::options::ProgramOption rather than mir::options::Option. So it's not something we could pass back with an overridden parse_arguments().

So we'd have to override both the_options() and parse_options(), duplicating a fair bit of code in the process.

[Michael Terry (mterry)]

Nope... argv and argc are private. I'm beginning to think this needs to be solved at the Mir level.

[Michael Terry (mterry)]

(Sorry, misspoke. Obviously we could have access to argv or argc if we need them, they pass through our constructor. What I meant was that program_options is private, which holds the results of calls to add_option(), which is not virtual. Thus we can't reasonably parse argv ourselves. We need to either strip argv before passing to Mir or modify Mir itself.)

[Alan Griffiths (alan-griffiths)]

To be clear program_options may be private, but it is passed to parse_options() which could be overridden so that you can parse argv yourself.

[kevin gunn (kgunn72)]

needs a quick fix at least to unblock AP tests.
currently available is this MP https://code.launchpad.net/~mterry/mir/unregistered-options/+merge/188125
(which needs to be retargeted to dev branch)

[Michael Terry (mterry)]

> To be clear program_options may be private, but it is passed to parse_options() which could be overridden so that you can parse argv yourself.

Yes, you have access to the stated options. And you can access argv/argc yourself (if you save them, since they are private in DefaultServerConfiguration). But you still have the problem that you can't change the contents of the ProgramOption class.

The only fix I can think of is to override the_options() to create our own class to replace ProgramOption (say, UnityMirOption), *but* still call the DefaultServerConfiguration version so that we can access program_options in parse_options(), which we would override to do our non-aborting parsing.

We'd still have to reproduce a lot of parsing code that happens in that call manually, because it happens in non-class functions. Additionally, we could either reproduce the environment/file parsing that ProgramOption has or let it parse the environment/file but then iterate over its contents and grab each set option and stuff them into our UnityMirOption version.

So I think it's possible to fix as-is. But super awkward. If we're looking at the long term, surely we can do better.

[PS Jenkins bot (ps-jenkins)]

Fix committed into lp:~mir-team/mir/development-branch at revision None, scheduled for release in mir, milestone Unknown