Fix Windows test errors in src/ls.js and test/cp.js. #1165
Conversation
This removes support for configuring `config.globOptions`. Exposing this variable makes it difficult to change (or upgrade) our glob library. It's best to consider our choice of glob library to be an implementation detail. As far as I know, this is not a commonly used option: https://github.com/shelljs/shelljs/issues?q=globOptions currently shows no GitHub issues of users using this option, and there was never really a motivation for why this needed to be exposed (shelljs#400 exposed the option just because we could, not because we should). This is one step toward supporting Issue shelljs#828.
This removes `node-glob` in favor of `fast-glob`. The main motivation for this is because `node-glob` has a security warning and I can't update to `node-glob@9` unless we drop compatibility for node v8. Switching to `fast-glob` seems to be fairly straightforward, although some options need to be changed by default for bash compatibility. Fixes shelljs#828 Fixes shelljs#1149
|
@kmashint can you please rebase this PR on top of the |
nfischer
force-pushed
the
switch-fast-glob
branch
2 times, most recently
from
a242df1 to
5e83898
Compare
|
@nfischer Will rebase on master tonight. |
nfischer
force-pushed
the
switch-fast-glob
branch
from
5e83898 to
53709e2
Compare
|
I merged master into this pull-request branch, and hoping I cut it the right way, removing references to glob in favour of fast-glob. |
src/common.js
Outdated
There was a problem hiding this comment.
I think this was reverted when it should not have been. I think what happened is that you probably have an out-of-date copy of the master branch. Can you please sync this against the main repo's master branch? I think you can do this with something like:
git checkout master
git remote add upstream https://github.com/shelljs/shelljs.git
git fetch upstream
git rebase upstream/master
git checkout switch-fast-glob
git rebase master
git push origin switch-fast-glob --force-with-lease
| "dependencies": { | ||
| "execa": "^1.0.0", | ||
| "glob": "^7.0.0", | ||
| "fast-glob": "^3.3.2", |
There was a problem hiding this comment.
Also, I would like to exclude this part from this PR. My change will take care of switching over to fast-glob. I'd like to merge your change before mine though to make sure it is backward-compatible with "plain" glob. Then I'll rebase my change on top of yours to test it with fast-glob`.
It's also fine if you want to test this with fast-glob locally in your checkout. We should just remove this part before merging this on the master branch.
|
Closing this one in favor of #1166. |
2 participants
These are fixes for Windows in the switch-fast-blob branch as mentioned in this pull-request.
#1153