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

Corrupts binary files (non-unicode files) #71

Closed
simplesmiler opened this issue Nov 25, 2014 · 15 comments
Closed

Corrupts binary files (non-unicode files) #71

simplesmiler opened this issue Nov 25, 2014 · 15 comments
Labels

Comments

@simplesmiler
Copy link

var gulp = require('gulp');
var zip = require('gulp-zip');
var concat = require('gulp-concat');

gulp.task('default', function() {
  return gulp.src('gulpfile.js')
    .pipe(zip('gulpfile.zip'))
    .pipe(concat('gulpfile.zip'))
    .pipe(gulp.dest('.'));
});

Resulting gulpfile.zip is corrupted. With concat line commented out everything works as expected.

gulp-concat v2.4.2, gulp v3.8.10, node v0.10.31

@simplesmiler
Copy link
Author

To be more specific, corrupts non-unicode files.
Introduced in 2.3.0 with adding support for sourcemaps.

@simplesmiler simplesmiler changed the title Corrupts binary files Corrupts binary files (non-unicode files) Nov 25, 2014
@yocontra
Copy link
Member

cc @floridoo

I'm pretty sure I talked about this when the sourcemaps stuff was first proposed - I wanted to make sure that it didn't break binary files

@floridoo
Copy link
Contributor

@contra this is not a problem with concat-with-sourcemaps, the problem is that

file.contents !== new Buffer(file.contents.toString())

for binary files.

@yocontra
Copy link
Member

@floridoo does concat-with-sourcemaps take in a buffer as an argument? The problem is right here where we convert to a string before passing it https://github.com/wearefractal/gulp-concat/blob/master/index.js#L35

@yocontra yocontra added the bug label Nov 28, 2014
@floridoo
Copy link
Contributor

@contra currently not, but I can look int adding that feature.
BTW: what would be a use case for cancatenating binary files?

@yocontra
Copy link
Member

@floridoo I can't think of any in my day to day but it isn't just binary, it could be any file in another encoding that isn't utf8. I 100% think we should support this. I think concat-with-sourcemaps just needs to use Buffer and Buffer.concat instead of strings and +=

@yocontra
Copy link
Member

@floridoo If a file has no sourceMap then it should basically revert to the pre-sourcemaps behavior

@simplesmiler
Copy link
Author

Use cases I'm aware of are node-webkit and love2d. They use concatenation of binary interpreter with zipped application source to ease distribution on certain platforms.

@martinib77
Copy link

Hi i'm using node-webkit and i'm having this issue. Is there any solution? Thanks.

@simplesmiler
Copy link
Author

@martinib77, my current workaround is to stick with 2.2.0.

@yocontra
Copy link
Member

Put "concat":"2.2.0" in your package.json dependencies list for a workaround. This will be fixed soon.

@floridoo Alive?

@martinib77
Copy link

That worked, thanks!

@floridoo
Copy link
Contributor

@contra I'm fine, just very busy ;)

yocontra pushed a commit that referenced this issue Dec 28, 2014
Don't corrupt binary files (fixes #71)
@yocontra
Copy link
Member

Published as 2.4.3

@simplesmiler
Copy link
Author

👍

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

No branches or pull requests

4 participants