Re: [Monotone-devel] commit ignores _MTN/options keydir

[Derek Scherger]

Stephen Leake wrote:
> Derek Scherger <address@hidden> writes:
> I think deleting that invariant check is the best fix. There is
> nothing the in contract for get_ws_options that says 'opts' must be
> empty before it is called; it just overrides the values in opts that
> come from the _MTN/options file.

Yeah, I think that the invariant should probably be removed. Any objections?

> Although that's not true for the branchname. Perhaps there needs to be
> an 'opts.branch_given' flag?

I think every option has a _given flag, based on all the preprocessor 
shenanigans in the various options*.{cc,hh} files.

I vaguely recall several places that are checking for empty option 
values that should probably be using the associated _given flags too. 
They were probably written before the _given flags existed. Looking this 
over might be a good little project for anyone who is interested.

> Deleting the similar check in dbname also makes sense.

Seems reasonable to me.

For anyone who wants to quickly look, the code in question is in work.cc 
lines 446 through 456. The assertions in question require that the 
associated db and key_dir options have empty values before they allow 
the values from _MTN/options to be used. It currently looks like this:

  read_options_file(o_path,
                    database_option, branch_option, key_option,
                    keydir_option);

  // Workspace options are not to override the command line.
  if (!opts.dbname_given)
    {
      I(opts.dbname.empty());
      opts.dbname = database_option;
    }

  if (!opts.key_dir_given && !opts.conf_dir_given)
    {
      I(opts.key_dir.empty());
      opts.key_dir = keydir_option;
    }

  if (opts.branchname().empty() && !branch_option().empty())
    {
      opts.branchname = branch_option;
      branch_is_sticky = true;
    }


Cheers,
Derek