RfidResearchGroup / homebrew-proxmark3

Homebrew tap containing proxmark3 software/firmware
MIT License
46 stars 19 forks source link

Remove puts output from `head` #13

Closed fletchto99 closed 4 years ago

fletchto99 commented 4 years ago

Having output in the head can cause brew bundle to break. For example, I've got an application setup locally which loads formulas into to JSON via brew bundle. Since I had this formula tapped when loaded it would output via puts the TRAVIS_COMMIT env variable into the output making the json invalid causing the application setup via brew bundle to fail.

mistydemeo commented 4 years ago

For context, brew bundle itself parses the JSON output of brew info --installed --json=v1, which is where this would have caused that output not to parse. 😄 https://github.com/Homebrew/homebrew-bundle/blob/master/lib/bundle/locker.rb#L80

iceman1001 commented 4 years ago

Nice find! It would however be nice to still get the output for the env variable instead of just removing it.

fletchto99 commented 4 years ago

Any output here will break brew bundle. Is there a reason its needs to be output? Perhaps it could be output during the install instead?

iceman1001 commented 4 years ago

Not informed in exact reason for printing / usage it there, I would guess it was handy. The output is good when debugging the travis integration. If we can print it during install, it would be fine. lets ask @merlokk if the output was used in any homebrew tests.

mistydemeo commented 4 years ago

Printing during install would be a lot safer than printing it at load-time! You could also guard the puts so it only prints if that variable is set. Printing to stderr would also be a bit safer, to avoid polluting the stream of something else that's consuming machine-readable output from stdout like what happened here.

iceman1001 commented 4 years ago

Looking forward to see the PR become enhanced with @mistydemeo 's suggestion

iceman1001 commented 4 years ago

Merlokk was fine with removing this output.

iceman1001 commented 4 years ago

Thanks for this find and fix!