grunt.file.delete incorrectly reports path as outside current directory

[rockerest]

grunt-cli v0.1.13
grunt v0.4.5
node v5.7.0
npm 3.6.0

Given the following task:

grunt.registerTask( "clean", "Wipe ./build", function(){
    if( grunt.file.isDir( "./build" ) ){
        grunt.file.delete( "./build" );
    }
} );

And the directory structure:

/
|---Gruntfile.js
|---build/
    |--- [files]

Running grunt clean fails and reports a warning:

Warning: Cannot delete files outside the current working directory. Use --force to continue.

Is this expected behavior?

On my *nix terminal any combination of ./, /./, etc. always results in the current path.
From my perspective, requesting that ./build be deleted is explicitly asking that the folder named build in the cwd be deleted, making this warning (and task failure) objectively incorrect.

I could be missing a nuance about how this is resolved, but having an error about not being able to delete files outside the cwd while explicitly requesting to start at the cwd feels off to me.

I'm also too unfamiliar with node to know if this is an issue with Grunt (maybe here or here?) or with Node's fs or path modules themselves.

[vladikoff]

@rockerest what version of node?

[vladikoff]

@rockerest see gruntjs/grunt-contrib-clean#85 and nodejs/node#5383

[rockerest]

node version v5.7.0 and npm version 3.6.0.

I'll also update the main issue to keep it all in one place.

Re: the issue you linked - I strongly believe the issue resides out there in node.
I don't know whether to leave this issue open, but I suppose it's up to you as a member of the Grunt team if you want to close here or not.

It might be a good point of reference for the node team regarding . in addition to the rest of the .relative() implementation.

[sibsibsib]

I just ran into this too. The issue is that .relative() is returning '../../whatever' instead of just '../' which causes the regex test in grunt's code (https://github.com/gruntjs/grunt/blob/master/lib/grunt/file.js#L444) to return false.

[vladikoff]

nodejs/node#5383

[vladikoff]

nodejs/node#5400

[aranaea]

I ran into this and dug into it enough to find the problem. Grunt's unit tests were failing when run with node 5.7 so that made it easier to chase down. I also verified that the unit tests still pass with this change and node 5.6.

[shama]

Closing this issue as this is a regression with Node.js and has been fixed for the node@5.7.1 release. We don't need to add a workaround to Grunt for this.

[aranaea]

@shama, the arguments being passed to path.relative are in the wrong order. Even if the issue is resolved in Node 5.7.1 the grunt code is wrong and uses a regex to catch itself.

[shama]

@aranaea Sorry, I'm not understanding the problem. If the regression is fixed in Node.js, what issue is that patch fixing?

[aranaea]

Hi @shama

The way the code is currently written it asks for the relative path from the target directory to the current and expects only ../ characters in the result. It seems more correct, and intuitive, to get the relative path from the current directory to the target and check to see if it begins with ... This has the added benefit of working in node 5.7 and 5.6.

Thanks for thinking about this.