-
-
Notifications
You must be signed in to change notification settings - Fork 171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Knip does not ignore gitignored directory when it has bad permissions #172
Comments
It can probably be reproduced by running this in an existing knip project
|
Can't seem to reproduce it here (I'm on macOS). Not sure if easily fixable by Knip. Perhaps easiest to update config to something like this: {
"entry": ["src/app.js", "scripts/**/*.js"],
"project": ["{src,scripts}/**/*.js"]
} Would that work for you? |
Hi @webpro , I checked a bit more I there was one thing missing in my previous description to reproduce. In an existing knip project do
The fact that it gives this error due to lacking permissions is not unexpected. The weird thing is just that the error still occurs even when the directory is added to the Can you reproduce it this way? |
Globbing the file system is done by globby, which in turn uses fast-glob. It is configured to take the I see fast-glob has a |
That would be a workaround. However it seems to me that, if the globbing correctly ignored the items from |
I think I have narrowed down the last call before the error happens. It should be |
Now that I understand that I think you're sugesstion of catching or supressing the error is probably the way to go here. |
Btw, I modified my {
"entry": ["src/app.js", "scripts/**/*.js"],
"project": ["src/**/*.js", "scripts/**/*.js"]
} and I still get the same error. Even though the offending
|
The exceptions is raised also when only revoking read access ( The issue seems to be that What could Knip do here:
A combination of catch-suppress is most user-friendly I guess, it can print a warning and still return useful results. Implementation and performance wise it's ugly. |
Ah that makes sense. So In the mean time your suggestions sound good. If knip somehow exposed this option then a user (like me) could specify his/her own pattern. In this case I could tell |
Update: Seems like the author of globby considers the current behavior a bug. |
Thanks! There's currently no way to override the hard-coded Still not sure about that workaround in Knip, but will keep this ticket open. |
Got it. My current workaround is to just add read permissions (chmod +r) for all users for the Folder that causes the Bug, which let's me use knip normally. Knowing that the globby author considers this a bug probably means that it will get fixed at some point. Feel free to open or close this issue as you see fit. |
Since the actual issue is upstream, I didn't hear from anyone else, and searching for |
That's fine. The upstream issue is labelled with "help wanted" so it looks like they are looking for a contribution. It could take a while until it's fixed. |
I'm hitting the same error. I have a huge /notes directory in my repo that is gitignored and contains all sorts of files. Deep inside there's something not readable by my current user, and knip fails with The fact that there's no way to work around this in knip makes knip unusable for me which is sad (tried changing the include glob but as written above it doesn't change anything, if something anywhere in the directory structure has no permission, knip won't work. this also seems like a huge perf issue tbh, is it scanning millions of files in my node_modules folder as well?) |
@webpro FYI seems like globby fixed this issue upstream sindresorhus/globby#259 |
I glanced at their fix but if I understand correctly it only solves the issue in narrow cases - when the permissionless files are in one of the locations that's in the hardcoded ignore list (not when it's in a gitignore file) doesn't apply to knip though because we fixed it differently |
Ah ok, I didn't Look at it in depth yet but thought it was the Kind of fix discussed in this issue.
Ah so knip rolled it's own solution? |
Ah fantastic. So knip doesn't even use globby in this case anymore. 👍 |
That PR replaces cq removes globby from the dependency list indeed. |
Hi,
I'm using the latest knip 2.17.0 on a Linux machine.
This is my knip config:
I have a directory called
mongodata
in the project root, which contains MongoDB data from a DB that's running in a docker container. Since it's running in the docker container it has different file permission than the rest of the project files.The mongodata folder is listed in my
.gitignore
.Nevertheless when I run knip I get this:
I even tried adding
ignore: ["mongodata/"]
to the knip.json, but I still got the error.The text was updated successfully, but these errors were encountered: