bitwalker / distillery

Simplify deployments in Elixir with OTP releases!
MIT License
2.97k stars 398 forks source link

Fix Strip #628

Closed michaelkschmidt closed 5 years ago

michaelkschmidt commented 5 years ago

This fixes an issue with using beam_lib to strip the release. Some Elixir Apps (such as Cachex, see https://github.com/whitfin/cachex/issues/205) need the 'Attr' chunk in order to work correctly, and it being removed.

The Stripper module is a direct copy/paste of the beam_lib functions

bitwalker commented 5 years ago

I really don't want to reimplement the strip functionality from beam_lib in Distillery - my take is that if the stripped beams are too stripped, then (for now) custom stripping needs to live in a plugin, and not in Distillery itself.

The ideal approach is to open a PR with the OTP team to extend the beam_lib API to allow us to pass either a custom filtering function, or a list of chunks we want to keep above and beyond the required chunks; Distillery can then make use of this new API when it is available.

I appreciate the PR though, I agree that having a way to tweak the stripping would be a nice feature to have :)

michaelkschmidt commented 5 years ago

I agree that an OTP patch is the best solution. However such a fix will take a year (or more) to make its way through to the average user. In the mean time the strip functionality in Distillery will break Elixir Apps in ways that are really hard to detect / debug. Since the strip option is part of Distillery itself, the fix for it should be too.

bitwalker commented 5 years ago

The strip option is not provided by Distillery though - it is provided by :systools, which we expose via Distillery's config but explicitly do not enable by default, specifically because it can cause issues. Stripping beams is only supposed to be done when the binary size of the extra chunks outweighs the value of those extra chunks, which for most applications is not usually worth the tradeoff.

In any case, yes, OTP PRs can take some time to trickle down to everyone, but those that truly need the functionality will upgrade as soon as its available, and that will be much much sooner than a year. In the meantime, the ability to strip beams manually (as you've done in this PR) remains an option via Distillery's plugin system. This PR is a few changes shy of being ready to be used as a before_package plugin, and could be open sourced as a stopgap measure for those who are in need of it.

michaelkschmidt commented 5 years ago

I can migrate this to a plugin easily enough—it actually started out that way 😊

Any commercial Nerves project will likely need this feature. Stripping debug from embedded binaries is standard practice. Thus it’s suprising that this is broken

michaelkschmidt commented 5 years ago

Here is the upstream fix: https://github.com/erlang/otp/pull/2114

I'll try to push a plugin workaround next

michaelkschmidt commented 5 years ago

Here is the plugin https://hex.pm/packages/strip_plugin