AddOption() adds user-defined options using spaces to COMMAND_LINE_TARGETS

[bdbaddog]

This issue was originally created at: 2011-11-17 14:20:24. This issue was reported by: dirkbaechle. dirkbaechle said at 2011-11-17 14:20:24

As reported by Steve Reinhardt in

http://scons.tigris.org/ds/viewMessage.do?dsForumId=1272&dsMessageId=2714584

There seems to be an inconsistency in handling options that take values when they're specified without an '='. This seems to be caused by the loop in _main that assigns anything that doesn't start with '-' or have an '=' in it to COMMAND_LINE_TARGETS, rather than letting the option parser figure it out properly.

Here's an example SConscript:

import sys

AddOption('--option')

print 'option:', GetOption('option')
print 'targets:', COMMAND_LINE_TARGETS

sys.exit(0)

And here's the problem:

% scons --option=FOO BAR
scons: Reading SConscript files ...
option: FOO
targets: ['BAR']
% scons --option FOO BAR
scons: Reading SConscript files ...
option: FOO
targets: ['FOO', 'BAR']

[kprussing]

It looks like this conversation is related to this issue. I started digging around while attempting to update the parser to argparse and came across this problem while running the tests. From what I can tell, the loop on line 982 in Main.py places all unparsed command line arguments into the targets list. I managed to work around the problem by moving the target assignment to after reading the SConscripts. This has the added benefit of allowing the SConscript calls to AddOptions to remove options from the command line arguments. I backed out the relevant change and put it on this branch. Some of the tests break because it changes some implicit assumptions about the parsing so I need to check on those before doing a pull request.

The one issue I see in the afore mentioned loop assumes any argument with = that is not a long option is an environment variable override. This prevents specifying targets with = in the path. I'm not sure how critical this problem is or how to work around it. I had a thought about using a regular expression ('\w+=.*'), but that still fails for targets in the current directory that have a = in the name.

[mwichmann]

Another related bug. Please see comments in both #2977 and #2748. I believe these are all three the same issue. If I've analysed this correctly, the algorithm of processed/saved cmdline args needs to be considerably reworked. If I am right, it explains why "If I remove the nargs option and use the `=' as described in thread [1]" makes it work.

[mwichmann]

This: "This has the added benefit of allowing the SConscript calls to AddOptions to remove options from the command line arguments. " - yes, that's what needs to happen. Did you find that things were still properly grouped at that point?

[kprussing]

From what I could tell, everything was working as expected. I didn't get a chance to address the failing tests because my schedule got too busy this past spring/summer. You're correct that the option parsing has to be reworked. But some (two or three) tests assume all arguments not absorbed by the parser on the first pass are targets which is not necessarily correct because the user defined options haven't been parsed where the tests occur. I can't remember which particular tests those were and how they failed.

The down side is all of that work is at home on my personal computer. I'll try to take some time over the next couple of days to push what changes I have lingering and drum up a better summary of where I stand.

[mwichmann]

I'm wondering if scons ought to just disallow: (a) user-added long options with args that use a space rather than = as separator, and (b) user-added long options with nargs>1 (supply them as a single comma-separated arg instead, like gcc does - and indeed like scons itself does :

Multiple options may be specified, separated by commas:

# Prints only derived files, with status information:
scons --tree=derived,status

# Prints all dependencies of target, with status information
# and pruning dependencies of already-visited Nodes:
scons --tree=all,prune,status target