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

Simple gulp copy garbles font files #2790

Closed
tdukai opened this issue Apr 9, 2024 · 11 comments
Closed

Simple gulp copy garbles font files #2790

tdukai opened this issue Apr 9, 2024 · 11 comments

Comments

@tdukai
Copy link

tdukai commented Apr 9, 2024

Before you open this issue, please complete the following tasks:

What were you expecting to happen?

I am expecting to see the font files copied over to have the same size (and work when consumed - it does not!)

What actually happened?

Simple copy job somehow garbles the files and they become corrupt.
Source files:
209128 Apr 2 14:32 fa-brands-400.ttf
117852 Apr 2 14:32 fa-brands-400.woff2
67860 Apr 2 14:32 fa-regular-400.ttf
25392 Apr 2 14:32 fa-regular-400.woff2
420332 Apr 2 14:32 fa-solid-900.ttf
156400 Apr 2 14:32 fa-solid-900.woff2

Copied files at target directory:
228639 Apr 2 14:32 fa-brands-400.ttf
214303 Apr 2 14:32 fa-brands-400.woff2
76830 Apr 2 14:32 fa-regular-400.ttf
46429 Apr 2 14:32 fa-regular-400.woff2
471443 Apr 2 14:32 fa-solid-900.ttf
284306 Apr 2 14:32 fa-solid-900.woff2

Please give us a sample of your gulpfile

const { src, dest, series } = require(‘gulp');

function copyFonts () {
    return src(['node_modules/@realmonitors/mdb/fonts/*.*']).pipe(dest('public/'));
}

exports.fonts = copyFonts;

Terminal output / screenshots

Job runs seemingly without any errors

$ gulp fonts

Please provide the following information:

  • OS & version: Linux Pop!_OS 22.04 LTS
  • node version (run node -v): 20.11.0
  • npm version (run npm -v): 10.2.4
  • gulp version (run gulp -v): CLI version 3.0.0, local version: 5.0.0

Additional information

@tdukai
Copy link
Author

tdukai commented Apr 9, 2024

I just checked, I have a very similar method copyImages, which also a simple copy and it corrupts all image files in the process. So the problem is not just with fonts, but all binary files.

@BotKarma46
Copy link

you have to specify {encoding: false} as an option to src for binary files.

@tdukai
Copy link
Author

tdukai commented Apr 10, 2024

you have to specify {encoding: false} as an option to src for binary files.

Thanks, I hoped something like exists, but the docs not mentioning it.

@jogibear9988
Copy link

same issue for me. this worked with a gulp <5

@jogibear9988
Copy link

https://medium.com/gulpjs/announcing-gulp-v5-c67d077dbdb7

Most usage of gulp won’t need to change anything, but certain plugins may produce non-UTF-8 output and you’ll need to set { encoding: false } on your gulp streams.

@jogibear9988
Copy link

it also corrupts images

@yocontra
Copy link
Member

Closing this since the solution has already been posted - but in v5 you need to specify encoding: false for binary data (was in the blog post, and is in the changelog, and the API docs).

@adamaveray
Copy link

adamaveray commented Apr 11, 2024

@yocontra I don't think this should be closed:

  • The docs page for src() doesn't even contain the encoding option at all – the word "encoding" is not found anywhere in the entire gulpjs.com site.
  • The changelog only says "Default stream encoding to UTF-8" which isn't obvious that that will impact simple copying of binary files.
  • The blog post does at least mention using { encoding: false } by saying "certain plugins may produce non-UTF-8 output and you’ll need to set { encoding: false } on your gulp streams," but that implies only specific plugins may have issues whereas this issue is regarding using zero plugins and merely a direct copy.

I feel this major a breaking change shouldn't be discoverable only deep within an announcement blog post not even in this repository nor the gulpjs.com domain – at the very least the src() docs need to document it but right now this issue is the only search result providing a solution so somewhere more prominent in the docs should also detail the issue (perhaps the "Working With Files" page?).

Even better would be Gulp detecting binary files loaded via src() with an unspecified/default encoding and producing a warning.

Update: This repository's own README file's Incremental Builds example doesn't even specify the { encoding: false } option so the images that example is copying would be corrupted.

Update 2: A quick Github search shows many, many Gulpfiles in use loading binary files as-is for simple copying – including some big projects like Mozilla's pdf.js (46k stars), Cypress (46k stars), Layui (28k stars) – so this change will impact many people when they update.

@yocontra
Copy link
Member

yocontra commented Apr 17, 2024

@adamaveray Closing specific issues because there has been so many duplicates, but totally understand the overall problem with regards to docs and communication of this change and its something we're working on (#2764).

The docs in the repo have been updated but the website has not been updated in a long time (almost 2yrs), there were issues getting a new deploy out and it's being worked on right now to get it updated soon.

As an aside from documentation:
It is likely possible we could guess file encodings ("is this not utf8", mainly for the purposes to detect binary files), then show a warning and proceed as a binary file to make this change backwards compatible, our intention was never to break so many peoples workflows. I'm not a huge fan of having to specify encoding: false, so if we can find a way to identify binary files easily that would be ideal. If this is something anyone is interested in assisting with please let us know, we are stretched pretty thin right now. Here is the original PR for reference: gulpjs/vinyl-fs#287

Also - those projects you linked that use v4 can stick with v4 until v5 has stabilized some more, there is nothing fundamental that would force anyone to upgrade right now if you have a setup that is already working for you. We've been very careful with the ecosystem to make sure v3, v4, v5 are not mandatory upgrades. You can see how many people are still installing an 8yr old v3 per week.

Screenshot 2024-04-17 at 11 27 22 PM

@Jako

This comment has been minimized.

kr8n3r added a commit to alphagov/notifications-admin that referenced this issue Jun 26, 2024
Gulp v5 streams now default to UTF-8 encoding.
Most usage of gulp won’t need to change anything, but certain plugins may produce non-UTF-8 output and you’ll need to set `{ encoding: false }` on your gulp streams.

It appears images and fonts (gulpjs/gulp#2790) get affected hence our test was failing https://concourse.notify.tools/builds/114181855#L666d232d:1367
@nlemoine
Copy link

nlemoine commented Aug 23, 2024

@yocontra I faced the same issue and { encoding: false } solves the problem.

However, how do I deal with mixed file types in src?

src(['assets/*.woff', 'assets/*.css', 'assets/*.png'])

Should I add { encoding: false } as soon as I have some binary files?

kr8n3r added a commit to alphagov/notifications-admin that referenced this issue Sep 5, 2024
Gulp v5 streams now default to UTF-8 encoding.
"Most usage of gulp won’t need to change anything, but certain plugins may produce non-UTF-8 output and you’ll need to set `{ encoding: false }` on your gulp streams."

It appears images and fonts (gulpjs/gulp#2790) get affected hence our test was failing https://concourse.notify.tools/builds/114181855#L666d232d:1367
kr8n3r added a commit to alphagov/notifications-admin that referenced this issue Sep 5, 2024
Gulp v5 streams now default to UTF-8 encoding.
"Most usage of gulp won’t need to change anything, but certain plugins may produce non-UTF-8 output and you’ll need to set `{ encoding: false }` on your gulp streams."

It appears images and fonts (gulpjs/gulp#2790) get affected hence our test was failing https://concourse.notify.tools/builds/114181855#L666d232d:1367
kr8n3r added a commit to alphagov/notifications-admin that referenced this issue Sep 27, 2024
Gulp v5 streams now default to UTF-8 encoding.
"Most usage of gulp won’t need to change anything, but certain plugins may produce non-UTF-8 output and you’ll need to set `{ encoding: false }` on your gulp streams."

It appears images and fonts (gulpjs/gulp#2790) get affected hence our test was failing https://concourse.notify.tools/builds/114181855#L666d232d:1367
artkalugin added a commit to artkalugin/td-technical-task-layout-designer that referenced this issue Oct 28, 2024
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

7 participants