Man page description of SetOption contradicts what is described under AddOption

[bdbaddog]

This issue was originally created at: 2008-06-17 06:41:36. This issue was reported by: neumannc.

neumannc said at 2008-06-17 06:41:37

The description on the man page of SetOption mentions a subset of CL options that can be set with this function. The AddOption description claims that user added options may be set to a default value with SetOption, which clearly does not agree with its description.

Using scons-local-0.98.5 the following SConstruct prints

scons: Reading SConscript files ...
 
scons: *** This option is not settable from a SConscript file: --build
File "/store/home/carsten/devel/opensg2/misc/buildsystem/test/SConstruct", line 49, in <module>

====== BEGIN Sconstruct ======================

AddOption('--build',
          action  = 'store_const',
          dest    = 'ob_mode',
          const   = 'build',
          help    = 'Build the libraries. This is the default mode, but only works after doing a configure first.',
          default = 'build')
 
AddOption('--configure',
          action  = 'store_const',
          dest    = 'ob_mode',
          const   = 'config',
          help    = 'Configure the build. This needs to be run once before doing an actual build.')
 
SetOption('ob_mode', 'build')

stevenknight said at 2008-06-26 17:32:42

Gary to document current behavior for 1.0, then reassign this for fixing in 1.x or 2.0.

stevenknight said at 2008-06-26 17:34:09

Set target milestone.

stevenknight said at 2008-06-27 22:33:28

Actually reassing to Gary, per bug party triage and previous comment...

garyo said at 2008-06-28 06:15:44

Removed doc section about setting user-created Options for now, in r3126. Should put that back when user options can be modified with SetOption. Resetting target milestone to 1.x, to do that work.

gregnoel said at 2008-12-26 13:30:38

Adjust triage of issues.

[mwichmann]

Hmmm, this claims the doc section was removed, but it still has this wording:

The value may also be set, using SetOption or env.SetOption(), if conditions in a SConscript require overriding any default value. Note, however, that a value specified on the command line will always override a value set by any SConscript file.

[bdbaddog]

I guess the bigger question is the issue resolved and thus removing the need to delete this bit of doc?

[mwichmann]

No, I don't think so. The text pasted above still suggests an AddOption'd option might be able to be the subject of SetOption, but this is not true, it only works on a subset of predefined (or "builtin" if you prefer) sons cmdline options. The example in the bug still throws an error:

scons: *** This option is not settable from a SConscript file: ob_mode
File "/home/mats/github/scons/exp/bug2105/SConstruct", line 18, in <module>

[VelocityRa]

Perhaps the API could differentiate between user and builtin options? Something like:

• AddUserOption/GetUserOption/SetUserOption
• GetBuiltinOption/SetBuiltinOption

[mwichmann]

I have a one-liner patch that allows options coming from AddOption to be set using SetOption. At the end of add_local_option, do:

        self.values.settable.append(result.get_opt_string().lstrip('-'))

Is that a wanted change? Else I'll rip the claim out of the docs.

[mwichmann]

Okay, it's not quite that simple: this works for boolean options, but needs extra smarts to set correctly for anything else. Smarts which aren't really available: the set_option method runs in the context of a Values instance, and doesn't have a hook to the definition of the option at that point - we would need to know the option's type to decide how to convert the value to its eventual form. the method "knows" about the scons options that have been specifically enabled, but doesn't do anything in a general way.

[mwichmann]

#3475 would fix the documentation part of this.

[mwichmann]

Should this be closed? The options topic is complicated, and I've written too much about it, but the issue subject as written is fixed.

[bdbaddog]

What happens if the user option is not boolean, say a string and we allow SetOption()? (As in what kind of bad?)

[mwichmann]

I don't recall the exact story for SetOption. If we ever figure it out, I'm happy to remove the recent bit in the manpage that says it doesn't work for user-specified options - and of course we'd have to fix the code to make it work.

[mwichmann]

Okay, swinging back to this again: there is no longer a manpage conflict, so this is no longer a documentation bug. It seems there was a long-ago decision to make added options settable:

Should put that back when user options can be modified with SetOption. Resetting target milestone to 1.x, to do that work.

I don't like keeping the issue open as-is. Here are three ways forward:

1. modify the summary line to something like "user-added options should be modifiable with SetOption" and change flag it as an enhanchement
2. Close the issue and say we're done with it.
3. Close the issue and open a new one for the enhancement request.

[mwichmann]

Since #3983 requests the ability to designate an added option as settable, I'll close this. Feel free to reopen if that's not appropriate.