brianloveswords / buffer-crc32

A pure javascript CRC32 algorithm that plays nice with binary data
MIT License
97 stars 30 forks source link

Same issue with 0.2.12. Expected version ">=6.0.0" #17

Closed jh2oman closed 8 years ago

jh2oman commented 8 years ago

The engine "node" is incompatible with this module. Expected version ">=6.0.0". node -v v4.5.0

CobaltDelta commented 8 years ago

Edit: Talking out of my ass, please disregard. :) I didn't read carefully enough and jumped to conclusions.

brianloveswords commented 8 years ago

@CobaltTiger for why something that wasn't "broken" got fixed, see https://medium.com/@jasnell/node-js-buffer-api-changes-3c21f1048f97#.wniv7cqhz and https://github.com/nodejs/node/issues/4660. Because there are no external API changes which would warrent a minor release, and there are potential security concerns, which is what patch is made for, if at all possible I want to keep this a patch update since I can't go figure out all the dependents and make them update.

brianloveswords commented 8 years ago

@jh2oman 0.2.13 should fix your issue. @CobaltTiger, gotta dig into yours more.

brianloveswords commented 8 years ago

@CobaltTiger if you get a second can you give me a stack trace using 0.2.12 or 0.2.13 (the only difference is 0.2.13 removes the engine restriction from package.json)

jh2oman commented 8 years ago

@brianloveswords thank you very much. Unfortunately out until after Thanksgiving so I won't be able to verify till then.

brianloveswords commented 8 years ago

@jh2oman sorry I wasn't able to get to it until you were out, let me know if it fixes your issue once you get back and I'll close this. Thanks!

CobaltDelta commented 8 years ago

@brianloveswords Aha, security issues I can completely understand. I had not seen these writeups -- and even at a glance I can see that being an issue that would indeed be best addressed through a patch.

As far as a stack trace goes, I'm unfortunately not able to easily produce one. In my case, the offending first-level dependent is a plugin called grunt-aws-lambda, which leverages archiver ~0.14.4. It proceeds to call npm.load and set its log level to silent. :) I do see that its only interaction with archiver (which, itself, seems to be the culprit as it is the only thing using buffer-crc32) is that it creates an archiver('zip'); We're using different node versions between our personal computers and our build servers (I know...) and the issue does not present itself on Node v4.4.5, but does break in 5.1.1. The error is "Buffer.alloc is not a function," as previously mentioned -- which is very strange because I see your code specifically checks for typeof Buffer.alloc === "function". Pretty sure there's no case where it would pass that check but throw that error afterwards. Maybe it's actually an issue with archiver and not buffer-crc32.

If you really need the stack trace, I can try tomorrow to make that child npm process more talkative.

brianloveswords commented 8 years ago

@CobaltTiger yeah, the stack trace would help – 0.2.12 tries to be smart and fall back to new Buffer if alloc and from are not available. I tried testing it locally using 5.1.1, it passes the test suite and I've confirmed it's falling back, so I'm not sure what's going on.

fengmk2 commented 8 years ago

@brianloveswords I think recently publish versions broken a lot. And I think those verions should be 0.3.x not 0.2.x.

CobaltDelta commented 8 years ago

I feel like yeah, if it does have to go to a new minor version, 0.2.x should immediately have a deprecation warning slapped on it. Won't do a whole lot to force outdated plugins to update, but I'm pretty sure it'll encourage active projects to get up to date.

On Mon, Nov 21, 2016 at 10:15 PM, fengmk2 notifications@github.com wrote:

@brianloveswords https://github.com/brianloveswords I think recently publish versions broken a lot. And I think those verions should be 0.3.x not 0.2.x.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/brianloveswords/buffer-crc32/issues/17#issuecomment-262136501, or mute the thread https://github.com/notifications/unsubscribe-auth/AMv4ADj2Fjm9m-zHw4XCpPHrUU7nYMj7ks5rAl5GgaJpZM4K41CN .

brianloveswords commented 8 years ago

@fengmk2 please file an issue with a stack trace if you're having one. There's definitely been some churn with this update to the new Buffer APIs while still trying to remain backwards compatible, which I take responsibility & apologize for, but I believe it's in the best interest of the community to provide this as a patch update.

CobaltDelta commented 8 years ago

@brianloveswords I'm sorry, I'm unable to produce a stack trace on my end. I did notice that npm was pulling down only 0.2.8 in previous builds -- perhaps that's the crux of the issue.

-- Confirmed that the previously mentioned build problem is NOT present when using 0.2.13 -- my apologies for the confusion.

I failed to notice it at the time, but even as of 7pm EST, npm was still pulling down v0.2.8 for some reason. Which is equally weird, since the only dependencies we've got are requesting buffer-crc32@~0.2.1, which should have picked the latest version... Or it could've been an unfortunate caching/proxy incident, since we proxy the npm repo through Artifactory, and it's proven to not always be the smartest. :T

jh2oman commented 8 years ago

@brianloveswords 0.2.13 Fixes my issue. Thank you very much!

brianloveswords commented 8 years ago

@jh2oman great, thanks for following up and sorry again for breaking it!