gulp-cli
gulp-cli copied to clipboard
Add initial cwd to directories for searching config files
I've add initCwd
entry to the last of opts.configFiles
.
This PR is for the issue #146 and changes gulp's behaviors about using config file and selecting gulpfile.
- There is an ancestor directory having
gulpfile.*
. - And current directory has a config file which has
flags.gulpfile
property. - And current directory has no gulpfile or a gulpfile of which name is not 'gulpfile.*'.
In the situation above, gulp after this modification uses the config file in current dir and uses gulpfile specified in the config file, but changes cwd to the ancestor dir. Before this modification, gulp ignores this config file, and finds up a gulpfile in the ancestor dir, changes cwd to the dir and uses the gulpfile.
@sttk I assume this is going to be changed with the new liftoff stuff?
@phated Yes. I think so, too. In addition, I think we can include a change for flags.nodeFlags
too with the new liftoff stuff.
I think we can include a change for flags.nodeFlags too with the new liftoff stuff.
In separate PRs 😛
Sorry. I made a mistake on pushing.
@sttk I'm continuing to dig into this and I think the behavior is even more incorrect when initCwd
isn't the highest priority. If I run cd my-sub-project; gulp
and there's a renamed gulpfile there, my working directory should not be changed to a top-level directory containing a gulpfile.js
- as most of these tests indicate.
I'm experimenting with initCwd
being the highest priority and also setting the env.cwd
if a gulpfile is found in that config.
@sttk I've also found that when using a .gulp.*
file to reference a gulpfile in another location, the .gulp.*
file relative to that gulpfile isn't resolved. I'm about to push some changes and skipped/failing tests that we need to fix before I think this can land. I'm also not sure how we can fix it without deeper integrations in Liftoff with the config files. I'm open to any suggestions.
@sttk I've just force-pushed a rebase on the lastest master plus my changes where I think our behavior is incorrect. I've tried to note in comments how things aren't working correctly or the temporary solutions to some problems exist. Please let me know what you think of all of this.
@phated This issue is very difficult. Please give me some time to think.
@phated Does what your changes suggest that if a config file in initCwd
has flags.gulpfile
or flags.cwd
, gulp-cli
should change cwd to env.cwd
and then loads .gulp.*
file in the changed env.cwd
?
It means that flags by config files in initCwd
behaves like flags by CLI options?
If so, I have two ideas to fix this issue: (The following codes are not actual but an example.)
- Process a config file in
initCwd
beforecli.prepare
.
var cli = new Liftoff({ ... });
var foundInInitCwd = fined({ path: '.', name: '.gulp', extensions: cli.extensions });
var cfg = { flags: {} };
if (foundInInitCwd) {
cfg = require(foundInInitCwd.path);
opts = mergeConfigToCliFlags(opts, cfg);
if (!opts.gulpfile) { opts.gulpfile = cfg.flags.gulpfile; }
if (!opts.cwd) { opts.cwd = cfg.flags.cwd || cfg.flags.gulpfile; }
}
cli.prepare({ cwd: opts.cwd, configPath: opts.gulpfile, ... }, function(env) {
var cfgLoadOrder = ['home'];
if (env.configFiles['.gulp']['cwd'].path !== foundInInitCwd.path) { cfgLoadOrder.push('cwd'); }
var cfg = loadConfigFiles(env.configFiles['.gulp'], cfgLoadOrder);
opts = mergeConfigToCliFlags(opts, cfg);
env = mergeConfigToEnvFlags(env, cfg, opts);
...
cli.execute(env, handleArguments);
});
- Apply the PR #95
var opts = { ... };
var env = {};
var cli = new Liftoff({
...
configFiles: {
'.gulp': {
initCwd: {
order: 1,
path: opts.cwd || process.env.INIT_CWD,
onFound: loadAndMergeConfig,
},
cwd: {
order: 2,
path: env.cwd
onFound: loadAndMergeConfig,
},
home: {
order: 3,
path: '~',
onFound: loadAndMergeConfig,
},
},
},
});
cli.prepare({ cwd: opts.cwd, configPath: opts.gulpfile, ... }, function(env) {
...
cli.execute(env, handleArguments);
});
function loadAndMergeConfigs(name, path) {
var cfg = require(path);
opts = mergeConfigToCliFlags(opts, cfg);
env = mergeConfigToEnvFlags(env, cfg, opts);
}
@sttk I think the issue is actually that too much filesystem traversal is being made in Liftoff.prototype.buildEnvironment
- I'm wondering if we shouldn't move https://github.com/js-cli/js-liftoff/blob/master/index.js#L62-L111 into the execute
phase.
The reason I think this is that the cwd
config file entry in the following code should actually be searching the process.cwd()
(or --cwd
or the resolved dirname of --gulpfile
if either are provided) https://github.com/gulpjs/gulp-cli/blob/1b80d67d6c4c7b9c80a8f49cda009187a91ad88e/index.js#L43-L54
I believe that is what you were trying to achieve by adding initCwd
config file entry. If config files were always started at the cwd
the same way that gulpfiles are looked up, before we attempt to look up the gulpfile, I believe the issue would be solved. Though, there might be an issue with a .gulp.*
file existing at a different place in the tree, but traversing up the filesystem should handle that, I think.
I'm wondering if we shouldn't move https://github.com/js-cli/js-liftoff/blob/master/index.js#L62-L111 into the execute phase.
Rather, that moving may be necessary because there is a case that gulp cannot be run only by changing cwd and gulpfile. It's needed to set env.modulePath
and env.modulePackage
too by executing those part in the link.
$ find . -path './b/node_modules/*' -prune -o -print
.
./a
./a/.gulp.json => `{ "flags.gulpfile": "../b/gulpfile.js" }`
./a/gulpfile.js // This is a dummy gulpfile to make gulp-cli load `a/.gulp.json`
./b
./b/gulpfile.js
./b/node_modules
./b/package.json
$ cd a
$ gulp --gulpfile ../b/gulpfile.js
[00:23:11] Working directory changed to ~/path/to/b
[00:23:11] Using gulpfile ~/path/to/b/gulpfile.js
[00:23:11] Starting 'default'...
[00:23:11] Finished 'default' after 2.71 ms
$ gulp
[00:24:57] Local gulp not found in ~/path/to/a
[00:24:57] Try running: npm install gulp
Furthermore, the current gulp with --cwd
option loads and processes .gulp.*
in the chaged directory.
I think this behavior is correct, but to reproduce the same behavior with a config file it is needed to load and process .gulp.*
in initCwd
before resolving paths of other config files.
$ find . -path './b/node_modules/*' -prune -o -print
.
./a
./a/.gulp.json => `{ "flags.gulpfile": "../b/gulpfile.js" }`
./a/gulpfile.js
./b
./b/.gulp.json => `{ "flags.require": "ansi-colors" }`
./b/gulpfile.js
./b/node_modules
./b/package.json
$ cd a
$ gulp --cwd ../b
[23:38:20] Requiring external module ansi-colors // => Load and process b/.gulp.json
[23:38:20] Working directory changed to ~/path/to/b
[23:38:20] Using gulpfile ~/path/to/b/gulpfile.js
[23:38:20] Starting 'default'...
[23:38:20] Finished 'default' after 2.4 ms
@sttk I believe this needs conflicts resolved since I merged #239