globby
globby copied to clipboard
globby is b0rked on Windows: .sync nor .async deliver /any/ result.
Related to #152, but .sync
is broken as well: doesn't matter what you try, globby won't produce any results.
Sample code:
var testset = globby.sync(__dirname + '/specs/*.jisonlex');
console.error({testset, dir: path.normalize(__dirname + '/specs/')});
produces this:
{
testset: [],
dir: 'W:\\Projects\\sites\\library.visyond.gov\\80\\lib\\tooling\\jison\\packages\\lex-parser\\tests\\specs\\'
}
while that directory is filled with tens of *.jisonlex files.
Ditto for the original code:
var testset = globby.sync([
__dirname + '/specs/*.jison',
__dirname + '/specs/*.lex',
__dirname + '/specs/*.jisonlex',
__dirname + '/specs/*.json5',
'!'+ __dirname + '/specs/*-ref.json5',
__dirname + '/specs/*.js',
__dirname + '/lex/*.jisonlex',
]);
console.error({testset, dir: __dirname});
Hi! What NodeJS version? Did you try other versions?
$ node --version
v12.18.4
No, haven't tried other versions, but have seen this issue before. Then I switched tactics as I was in a hurry to get things done, so I don't expect this to be a node version specific thing.
Side Note: globby use like this (i.e. without directories in the search argument) works as expected:
globby([ 'ebnf-parser*.js' ])
New info
Worky 😥 (phew!)
This works on Windows:
process.chdir(__dirname);
var testset = globby.sync(['./specs/*.jisonlex', './lex/*.jisonlex']);
As long as you
- DO NOT pass Windows path separators ('\') as delivered by
path.normalize()
et al 💢 - DO NOT pass Windows drive / network share identifiers (e.g. 'W:' in 'W:\' which would be a absolute path in Windows) into globby anywhere, you'll be fine.
Not Worky 1
Ergo: doing something like
let basepath = path.normalize(path.join(__dirname, ......)).replace(/\\\\/g, '/'); // UNIXify path
// ^^^ INSUFFICIENT, as the Windows drive letter plus colon, e.g. 'W:/' will make it through
// and then globby will barf a hairball, delivering NIL results!
globby(basepath + '/specs/*.jisonlex', ......);
will FAIL.
Not Worky 2
Ditto for any inattentive coding like this, where you employ path.join
just to shut up eslint
et al -- as would happen with the case above:
(note the trouble-causing path.join
in the globby
statement; compare to previous example)
let basepath = path.normalize(path.join(__dirname, ......)).replace(/\\/g, '/'); // UNIXify path
// ^^^ AS PREVIOUS EXAMPLE ABOVE.
globby(path.join(basepath, 'specs/*.jisonlex', ......));
// ^^^ BAD JUJU: path.join screws you over as it will inject a '\' and you will be TOAST, once again.
Conclusion
Only work with paths coding and globby when you're totally hopped up on caffeine and bright and shiny yourself. 👼
"Using relative UNIXy paths only" won't save your bacon because:
- your globbing MAY span multiple drives, e.g.
['D:/projectBla/', 'Z:/bit-of-network/projectBlaOverlord/']
-- no way out but to split those searches into one glob per drive and join up the results by hand. 😠 Buggerit Millenium Hand & Shrimp. - you MAY have used a Node
path
API a little late in the game (e.g. second example above: the secondpath.join
in there) and your paths won't be relative and utterly UNIX any more. A non-exhaustive list of devils:- path.join
- path.normalize
- path.resolve
- path.format
Be vewwy vewwy careful and on the look-out for hidden backslash-in-a-path delivery boys, such as:
-
__dirname
-
process.cwd()
- anything else that heralds producing an absolute path
@GerHobbelt yes, adding this test to test.js
fails on Windows but passes on Mac/Unix.
test('glob - async - absolute', async t => {
const temporaryAbsolute = path.resolve(temporary);
const result = (await globby(path.join(temporaryAbsolute, '*.tmp'))).sort();
t.true(result.length === 5);
for (const [i, element] of fixture.entries()) {
t.true(result[i].endsWith(element));
}
});
However, as a workaround you can do .replace(/\\/g, '//')
. This test will pass.
test('glob - async - absolute', async t => {
const temporaryAbsolute = path.resolve(temporary);
const result = (await globby(path.join(temporaryAbsolute, '*.tmp').replace(/\\/g, '//'))).sort();
t.true(result.length === 5);
for (const [i, element] of fixture.entries()) {
t.true(result[i].endsWith(element));
}
});
Definitely some kind of bug in handle of full system paths combined with path.sep. cc. @sindresorhus
@phawxby: yup, did another test and the multiple-drive search scenario seems to pass as well.
🤔 What did go wrong before over here then as when I did the replace
thingy it was not enough? 🤔
A-ha! I did it too early: before the path.normalize()
calls like in
globby([
path.normalize(p1.replace(/\\/g, '/')),
path.normalize(p2.replace(/\\/g, '/')),
...
], ...
which, 🤦 of course 🤦 , would happily convert those UNIX slashes back to Windows' \
backslashes again and thus throw me a curveball. 😢 (I did not re-check the replace
-only work-around after I found out path.normalize
was having me on when I wrote the previous message and results therein. 😓 )
So the chdir+useRelativePathsOnly
approach is not required, while my list of culprits to watch out for stands:
Them Nasty Buggers
-
path.join
-
path.normalize
-
path.resolve
-
path.format
-
__dirname
-
process.cwd()
- 😇 ... quite probably missed a couple backslash-stabbing others here ... 😇
Back on the topic of globby
: what really fazes me now is the question how I got away with that __dirname
-based stuff passing into globby
. 🤔 Apparently I had some magic mix that I cannot bring back, as one thing's for sure: this was in a unit test rig which has run many times before (at least in 2018/2019).
What changed since that stuff got run last time?
Over time...
- nvm-for-windows 👍 got installed and the node installs got ditched.
- dev/test boxes got upgraded using
nvm
all the way from node v6 + v8 + v10 to node v12. -
ncu
(https://www.npmjs.com/package/npm-check-updates) was run on the projects everywhere. - Plus some bash scripting around
npm install && npm prune && npm audit fix
too, for good measure.
And after all these updates, said test rig went t*ts up. (Not a big surprise. What was a surprise was that globby
was doing all the b0rking and once I got fed up and hardcoded the search result set using UNIX find
and some shell scripting + fs.readfileSync().split('\n')
instead of globby
all tests behaved as before, no sweat.
Anyway, that's where I chose to file an issue this time around. And here we are. 😅
Anyway, I hope some poor bugger will find some useful help in all this.
Which changes this to maybe a choice:
a. make globby
cope with backslash paths (or at least warn about them entering the premises, maybe?) despite \
being a (ahem) legal filename character on UNIX. (Note the K&P quote in https://stackoverflow.com/questions/1976007/what-characters-are-forbidden-in-windows-and-linux-directory-names for extra fun.)
b. add a bit to the documentation about Windows folks being extra special careful with them path
babies.
(Incidentally, I bet the other globber libs out there suffer the same issue. I seem to recall some similar struggle with node-glob
or what-was-it-I-used-back-in-the-day?)
@GerHobbelt @phawxby I believe this was deliberate and was flagged as a breaking change in the v10.0.0 release, as fast-glob changed this behaviour in their v3.0.0 release.
Fast-globs proposed workaround is .replace(/\\/g, '//')
https://github.com/micromatch/micromatch#backslashes. So I don't see this being fixed in globby as it will break special character escaping.
I am only using globby to get all the deep filenames from a given cwd (so the results won't contain the parents dirs). The glob pattern is just ''
. Do I need to care about Windows/Posix fs? Because it says the cwd defaults to process.cwd(), and it on Windows, will contain backslashes.