Add support for scanning multiple entries in an action string

[grossag]

This change adds support for scanning multiple entries in an action string in order to better support the following use cases:

1. A file is provided in an action string and should be taken as a dependency. For example, an action string "$PERL somefile.pl".
2. An action string actually has two actions separated by &&. For example, "cd <some_dir> && $ZIP ".

Adding support for #1 actually allows us to fix the test IMPLICIT_COMMAND_DEPENDENCIES.py on Windows, which was previously treating a Python file as executable even on Windows. This was causing tests to repeatedly open the default handler of Python files, which if set to Visual Studio causes DDE hangs. This test is fixed because now we can have the action string specify python as the first command and still take an implicit dependency on the script, which is now the second command.

Contributor Checklist:

• I have created a new test or updated the unit tests to cover the new/changed functionality.
• I have updated master/src/CHANGES.txt directory (and read the README.txt in that directory)
• I have updated the appropriate documentation

[bdbaddog]

I think In general including path into repo is deprecated. Should just look for non-repo file. There's a way to query if File is known without creating the node.

[grossag]

So are you saying the “else” case should be removed because repo support is not needed?

[bdbaddog]

Good work.
Not sure why you're needing to turn off IMPLICIT_COMMAND_DEPENDENCIES for some tests.
Does this imply you've changed the default behavior?

Also right now looks like you have three options: no Implicit, only first argument, or all arguments.

I think it could be useful to be able to specify to process the first N arguments and/or a list of which arguments.
Also needs a manpage entry update for new IMPLICIT_COMMAND_DEPENDENCIES values.

[grossag]

Good work.
Not sure why you're needing to turn off IMPLICIT_COMMAND_DEPENDENCIES for some tests.
Does this imply you've changed the default behavior?

Also right now looks like you have three options: no Implicit, only first argument, or all arguments.

I think it could be useful to be able to specify to process the first N arguments and/or a list of which arguments.
Also needs a manpage entry update for new IMPLICIT_COMMAND_DEPENDENCIES values.

Thanks for the feedback. Turning off IMPLICIT_COMMAND_DEPENDENCIES was a mistake, a remnant of my older change modifying the default behavior. I have reverted that change. I will also look at updating the documentation and CHANGES.txt.

I would prefer to avoid customizing the behavior any further. I understand that we don't want to change the current behavior of IMPLICIT_COMMAND_DEPENDENCIES and the default behavior. Three main reasons:

1. The testability overhead.
2. A lack of an intended consumer and clear use cases.
3. The variance of action strings making it difficult to find meaning in a specific value of N. For example, N=2 may make sense for a Perl script (e.g. N=1 taking a dependency on Perl binary and N=2 taking a dependency on the script path) but not for a linker action.

[bdbaddog]

Since you can set.
env.Command(....., IMPLICIT_COMMAND_DEPENDENCIES=something which only make sense for this command)
Then it can make sense to have it more customizable..

[grossag]

In my opinion, if someone is going to go to the trouble of overriding IMPLICIT_COMMAND_DEPENDENCIES for a specific action, they should just use env.Depends instead. The benefit of implicit command dependency scanning is having a safety net for people not specifying their dependencies correctly. Specifying dependencies manually is always preferred, but this page is useful for large codebases where people aren’t always doing it correctly.

That said, if you won’t accept the PR without this change then I will do it. So I can better understand what you are asking for, here is what I understand you to be requesting:

IMPLICIT_COMMAND_DEPENDENCIES value possibilities:

0/False: no implicit dependencies (as before)
1/True: only 1 implicit dependency even if && is in the command string (as before)
Another integer value N: N implicit dependencies before a && (but what about after? N deps after each && too)
“all”: all implicit dependencies before and after &&

[bdbaddog]

Since values in Command can vary by environment and/or overrides and those values may need to be added to implicit dependencies, allowing more control over the implicit scanning of command line can provide value.

I was thinking:
IMPLICIT_COMMAND_DEPENDENCIES value possibilities:

0/False: no implicit dependencies (as before)
1/True: only 1 implicit dependency even if && is in the command string (as before)
Another integer value N: N implicit dependencies before a && (but what about after? N deps after each && too)
“all”: all implicit dependencies before and after &&
A list with position values: implicit dependencies only for those positions listed.

Seems like you're already 95% of the way to supporting a list.
But if that's not interesting to you, I'm not going to hold up the PR for u.

IMHO.. if you need to use env.Depends() you've done something wrong and/or SCons needs to do more. (for 90% of the cases)

[grossag]

I have updated the PR with support for IMPLICIT_COMMAND_DEPENDENCIES being an integer.