Allow list of chunks to be given to strip*() #2114
Conversation
|
|
|
Wait, does that mean that if I strip modules, I may end up with parts of my build keeping info I expect to be gone once in production? Some of these things you possibly do not want to ever hit a production server. |
|
@ferd I agree--the issue is that some Elixir modules (like cachex) need the 'Attr' chunk. Without it Its standard practice for IoT devices to strip debug information prior to release. This change only exposes an API to allow the list of chunks to be passed in. It's still up to the end user to decide what chunks should be passed on to the final release. Without this, you wind up having to do hacks like https://github.com/se-apc/strip_plugin/blob/master/lib/strip_plugin.ex |
There was a problem hiding this comment.
Thanks for the PR! I think it's a reasonable extension, and it looks good aside from a minor issue with the new interface. Can you update the tests and documentation?
Wait, does that mean that if I strip modules, I may end up with parts of my build keeping info I expect to be gone once in production? Some of these things you possibly do not want to ever hit a production server.
The current behavior remains unchanged, so I think it will be fine as long as the build tools that use this feature are clear about what they're doing.
| strip_release/1, | ||
| strip_release/2, | ||
| significant_chunks/0, |
There was a problem hiding this comment.
The loader requires all "significant chunks" to be present, so it feels a bit heavy to require the user to provide them in all calls to strip*/2. It would be simpler if the second parameter were the chunks to preserve in addition to the significant ones.
There was a problem hiding this comment.
That is a good point. I'll change it to prepend any additional chunks
|
@jhogberg I made the suggested change and updated the docs. Please let me know if you need anything else. |
|
Thank you, can you add a test as well? |
jhogberg
added
stalled
This allows extra chunks to be preserved for languages such as Elixir
michaelkschmidt
force-pushed
the
strip_fix
branch
from
9dd7f78
to
c9546b2
Compare
|
@jhogberg sorry for the delay. I believe I have added a test which verifies the additional chunks are preserved, and I squashed all commits into 1 |
bjorng
added
testing
|
@jhogberg Are there any other changes you would like? |
|
Sorry for the delay, I just got back from a cold. It looks good so I merged it to master, thanks again for the PR! |
This allows other BEAM languages to specify additional chunks that should be preserved. The immediate need is 'Attr' for Elixir.
See whitfin/cachex#205 for full details on the original issue.