Skip to content
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

.DS_Store in installed lib #13

Open
OrangeDog opened this issue Jun 5, 2019 · 11 comments
Open

.DS_Store in installed lib #13

OrangeDog opened this issue Jun 5, 2019 · 11 comments

Comments

@OrangeDog
Copy link

npm install extglob@2.0.4 creates the file node_modules/extglob/lib/.DS_Store

I'm not using macOS so I'm pretty sure it's not at my end.

@djzara
Copy link

djzara commented Nov 24, 2019

@vue/cli has this as a dependency in its dependency tree. Every time stuff changes there it happens. Not the worst error in the world, but one of those "really?" moments. This is still happening

@jonschlinkert
Copy link
Member

Agreed, it's irritating that NPM picked up this file when I published the lib, despite always being ignored in .gitignore.

@djzara
Copy link

djzara commented Nov 24, 2019

I noticed that when I went to investigate. I'm like...ok nobody actually leaves that in. And yeah, it's in there. Maybe got committed by mistake at some point? Who knows. Bet money if you send something you'll get the same error message you always get when something happens with the npm util when something goes wrong
"This is probably not a problem with NPM itself" lol

@jonschlinkert
Copy link
Member

Maybe got committed by mistake at some point?

Remember that NPM doesn't care what is committed to git. It only zips up what you explicitly define in package.json.

@djzara
Copy link

djzara commented Nov 24, 2019

Remember that NPM doesn't care what is committed to git. It only zips up what you explicitly define in package.json.

I'm mainly thinking out loud, but wouldn't you still need it in there to keep NPM from picking it up when it pulls from the repository listed in package.json?

full disclosure: I'm learning here, never actually published to npm before

@jonschlinkert
Copy link
Member

jonschlinkert commented Nov 24, 2019

full disclosure: I'm learning here, never actually published to npm before

No worries! I don't mind the questions. Happy to help!

but wouldn't you still need it in there to keep NPM from picking it up when it pulls from the repository listed in package.json?

When you run npm publish, NPM doesn't check to see if a .git/ directory even exists (if the CLI does check, it's only to fill in some kind of details that might be useful for metadata, but it's not necessary). Also, if you specify files in package.json, then NPM will only include files specified on that property, while also ignoring any files ignored by .gitignore or .npmignore.

In any case, since the file is ignored, doesn't that address all of the scenarios you're asking about? What's more likely is that NPM's matcher (minimatch) didn't match the file. Perhaps this is a nuance caused by having a * before the . in *.DS_Store (fwiw that pattern was generated, and it should work just fine with a glob matcher).

My hunch is that NPM does not explicitly set minimatch's dot option to true, and instead they rely on testing the first character of a pattern to see if it's attempting to match a dot--which would be incorrect matching behavior for gitignore patterns.

@djzara
Copy link

djzara commented Nov 24, 2019

That all makes perfect sense! I'm not in the npm package dev world so it's still got some intricacies I don't understand quite yet. That right there is gonna save some headaches. I never thought about the way the path is defined as being an issue anywhere, honestly, but it makes sense given that pattern in gitignore being kind of critical.

Much appreciated!!

@jonschlinkert
Copy link
Member

fwiw I will republish to get rid of this file as soon as I have a chance

@hantuzun
Copy link

Everytime I install something with brew this bug pops up like this:

Error: Permission denied @ apply2files - /usr/local/lib/node_modules/expo-cli/node_modules/extglob/lib/.DS_Store
Requirements installation failed with status: 1.

Then I need to manually delete this file and run again.

@jonschlinkert
Copy link
Member

Then I need to manually delete this file and run again.

Damn that's irritating. Sorry you have to do that. I've been swamped, I'll try to push up a patch to fix this.

@reviewher
Copy link

Here because of the brew apply2files issue in brew. It suffices to delete all of the .DS_Store files in /usr/local/lib/node_modules/:

find /usr/local/lib/node_modules/ -name .DS_Store | while read x; do echo "$x"; sudo rm "$x"; done

@jonschlinkert the simplest way to fix the module is to change the files entry to be a glob:

https://github.com/micromatch/extglob/blob/master/package.json#L23

  "files": [
    "index.js",
    "lib/*.js"
  ],

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants