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

Hidden 'dotfiles' within an extraResources folder aren't copied #733

Closed
joerick opened this issue Sep 8, 2016 · 8 comments · May be fixed by qcif/data-curator#563
Closed

Hidden 'dotfiles' within an extraResources folder aren't copied #733

joerick opened this issue Sep 8, 2016 · 8 comments · May be fixed by qcif/data-curator#563
Labels

Comments

@joerick
Copy link

joerick commented Sep 8, 2016

  • 5.18.0:
  • Mac:

Using electron-builder to build a Mac .app file. My resources include some Python libraries, and they store some files in a folder called .dylibs. So the tree looks like (simplified):

Repo root
|- vendor
|  |- tide-packages
|  |  |- PIL
|  |  |  |- image.py
|  |  |  |- .dylibs
|  |  |  |  |- libjpeg.dylib

I've specified the vendor folder in extraResources in my package.json, and everything inside it is copied, except the .dylibs folder. I think its due to the use of minimatch in the copy filter.

Currently the workaround is to add another rule in package.json:

  "build": {
    "extraResources": [
      "vendor",
      "vendor/**/.*",
      "vendor/**/.*/**",
    ],
  }

I didn't expect this to be needed, though, since I don't expect glob to be used when there are no wildcards in the path.

Ref: tingbot/tide-electron#42

@develar
Copy link
Member

develar commented Sep 8, 2016

https://github.com/electron-userland/electron-builder/wiki/Options#file-patterns hidden files are ignored by default

I don't want to change defaults — is there magic or not in the pattern, doesn't matter.

But yes – .dylibs should be added to exclusions add copied by default, because it is common known folder name.

@develar develar added the feature label Sep 8, 2016
@joerick
Copy link
Author

joerick commented Sep 8, 2016

Okay, that's your call. I don't agree, because I'd expect it to work like cp, i.e.

    # this would copy everything inside vendor
    cp -R vendor $BUILD_ROOT/vendor
    # this would skip the dotfiles in vendor
    mkdir -p $BUILD_ROOT/vendor
    cp -R vendor/* $BUILD_ROOT/vendor

Adding extra filters after the user has given an explicit path seems to go against the wishes of the user.

@develar
Copy link
Member

develar commented Sep 8, 2016

seems to go against the wishes of the user.

Files like .DS_Store or thumbs.db should be ignored. Excluding dot files by default helps to reduce size of packaged app.

@develar develar added the discuss label Sep 8, 2016
@joerick
Copy link
Author

joerick commented Sep 8, 2016

Files like .DS_Store or thumbs.db should be ignored.

Couldn't disagree with that.

Another factor was that this decreases dev/prod parity - for me it caused a bug that only occurred in production builds. And I can only test a fix by doing a full rebuild.

@develar
Copy link
Member

develar commented Sep 8, 2016

Ok. If we will exclude only dot files, but not dot folders, is it ok for you?

@joerick
Copy link
Author

joerick commented Sep 8, 2016

My absolute preferred solution would be: when a folder is explicitly listed as something to copy, only exclude files within that we know are useless in a production build - .DS_Store, thumbs.db, .git, etc.

Just checked Xcode's copy command... it does the following-

builtin-copy -exclude .DS_Store -exclude CVS -exclude .svn -exclude .git -exclude .hg -resolve-src-symlinks $SRC $DEST

@develar
Copy link
Member

develar commented Sep 10, 2016

Thanks a lot that you decided to argue your position instead of just workaround only for your project.

@develar develar removed the discuss label Sep 10, 2016
@joerick
Copy link
Author

joerick commented Sep 13, 2016

👍 no worries!

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

Successfully merging a pull request may close this issue.

2 participants