gruntjs/grunt-legacy-util — pull request #4 by dylancwood (2014-06-23 07:27:15+00:00)
Title: Parent argv

More background can be found at the original issue in the grunt repository: https://github.com/gruntjs/grunt/issues/1172

This patch turned out to be far less simple than I originally thought, but it is working well now, thanks to unit-testing!!

# Motivation:
When spawning a child process using ``grunt.util.spawn`` with the options ``grunt: true``, the child process should inherit any command line flags passed to the parent process. Most importantly, the ``--debug`` and ``--no-write`` flags should always be passed to any children to prevent unwanted behavior.

# Original solution:
It seemed so simple at first:

I modified grunt-legacy-utils/index.js as follows to ensure that the parent's CLI args were passed to the child:
```js
- args = process.execArgv.concat(process.argv[1], opts.args);
+ args = process.execArgv.concat(process.argv[1], opts.args, process.argv.slice(3));
```

## Problem(s) with original solution:
1. Assumes that the *task* argument is at position two of the process.argv array. This is wrong because the task may not even be in the arguments, and if it is, there is no guarantee that it will be the third element in the array.
2. Does not account for ``--gruntfile path/to/gruntfile``: luckily, this was a heavily used paradigm in the unit tests, and I caught this problem when the tests that use ``--gruntfile`` failed. The reason that ``--gruntfile`` causes failures with the original solution is that the ``--gruntfile`` argument was the third argument in the args array, and was therefore being stripped. As a result, the gruntfile path was being interpreted as a task name, which was undefined.
3. Does not account for relative ``--base`` paths. For instance, if a parent process is invoked from ``/var/www`` with ``grunt default --base path/to/somewhere``, then the child process will attempt to calculate it's base as ``/var/www/path/to/somewhere/path/to/somewhere``, which is probably not the user's intention.

# New Solution:
Instead of simply chopping off the first three arguments, we now need to filter them to remove the task name(s). This can be accomplished by removing any arguments that do not start with a ``-``.

Since filepaths do not start with a `-`, the will be stripped from the arguments during filtering. This is a problem when it comes to the file paths specified after the ``--gruntfile`` arguments, which we want to preserve (or at least I assume that we do). To solve this problem, I created a whitelist of arguments whose *next argument* should always be allowed to stay. This allows the argument immediately following a ``--gruntfile`` argument to remain in the args list even if it does not start with a `-`.

To address the issue of relative ``--base`` paths, I implemented a blacklist. Any arguments on the blacklist will be stripped during the filtering process. Since the file path associated with the blacklisted ``--base`` argument probably does not start with a ``-``, it will be removed as well with no further logic required. There is no need to pass the ``--base`` argument to the child process, as it will be spawned from the parent's ``base path`` by default.

**If a process invoked with a ``--gruntfile`` argument spawns a child, and provides different ``--base`` or ``gruntfile`` arguments to the ``spawn`` method, then the new arguments will override the parent's.** This is because the parent's arguments are inserted in the ``args`` array before the ``opts.args`` arguments are, and thus any arguments ins ``opts.args`` will override the parent's arguments.

# Testing
I've added three tests to ensure that this feature is working properly:
* Test to be sure that a ``--no-write`` flag is passed automatically from a parent to a child grunt process.
* Test to be sure that the ``--gruntfile`` flag and its associated value are passed to the child process.
* Test to be sure that the ``--base`` flag and its associated value are passed to the child process.

# Conclusion
This proved to be more complicated than I originally anticipated. I hope that there are not any more *gotchas* or use-cases that this change would cause problems with. An alternative, simpler solution would be to only pass specific arguments from parent to child. For example: only pass ``--debug``, ``--verbose`` and ``--no-write``, as those are the most likely to cause irreversible problems if they are not passed. Failing to pass the other flags may cause a child process to fail or stop, however, the developer can manually add any needed flags to the ``opts.args`` as needed.

I eagerly await your review.