heartsucker / node-deb

Debian packaging for Node.js projects
https://www.npmjs.com/package/node-deb
MIT License
206 stars 54 forks source link

Fix sed md5 error on macOS #100

Closed gutenye closed 4 years ago

heartsucker commented 5 years ago

The change you made here makes the two branches of the if/else statement identical. This would effectively revert the change from commit 233f219 from PR #84. What is the error you're experiencing that requires this fix?

gutenye commented 5 years ago

I use sed installed by brew, extra '' gives a syntax error. I checked #84, looks like only the builtin sed needs the extra ''. And I totally don't understand what the extra '' for.

Anyway, I suggest list sed as a requirement in README and remove the special case for macOS, that keeps sed syntax consistent.

heartsucker commented 5 years ago

This is getting into some serious grittiness because the version of sed could be the GNU version or the BSD version, and it could come from one of many places on many systems. I think that honestly this is just a crappy bug that will exist until I can find the time to port all this bash to JS (#66). I also don't have a Mac to test with, so a lot of the bugs that exist as Mac only are extremely hard for me to debug reliably. The README says that I don't officially support Mac because of this, but once #66 is done, I think I could remove that line since the only external shell out would be at the very end for calling dpkg-deb.