Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow for explicitly turning on debuginfo for builds #59

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maxcountryman
Copy link

@maxcountryman maxcountryman commented Nov 4, 2019

What did you implement:

This patch creates an explicit interface for setting DEBUGINFO on Docker run invocations. To do so, one simply specifies debugInfo under the rust section of their custom section of the Serverless config.

Closes: #49

How did you verify your change:

Manual testing.

What (if anything) would need to be called out in the CHANGELOG for the next release:

  • Added the ability to specify debugInfo in the rust section of the config.

@maxcountryman
Copy link
Author

@softprops any thoughts on this?

@maxcountryman
Copy link
Author

@softprops I can update this if you’d be interested in merging—right now we’re using a fork in order to get reasonable error logging in prod.

@softprops
Copy link
Owner

Have at it. I wanted to get that last release out there to start testing local builds quickly

You'll notice a few changes to the test setup. Namely there are unit tests! If possible try to see the structure for that and see if it is possible to add a unit test for this change.

@softprops
Copy link
Owner

I'll roll this change into the next release which can happen as soon as you like

@maxcountryman
Copy link
Author

@softprops I think this is ready for another look.

@maxcountryman
Copy link
Author

@softprops is this still something you want to merge and release? We'll continue using our branch for now, but would love to switch back at some point. 🙂

@softprops
Copy link
Owner

question. Does it make it any easier to accomplish this with the new dockerless option?

@maxcountryman
Copy link
Author

Local builds don't require this but that may not be an option depending on your build pipeline. For when it's not an option, this is necessary if you want debug info. (We do. 🙂)

@softprops
Copy link
Owner

I totally get that. What I'm trying to gain a sense of is how nesessary docker builds are now that host configured builds ar exposed.

Here's a snippet of the set of of a build pipeline dependencies with GitHub actions

if: matrix.os == 'ubuntu-latest'

@maxcountryman
Copy link
Author

What I'm trying to gain a sense of is how nesessary docker builds are now that host configured builds ar exposed.

This feels orthogonal to this patch; we aren't able to use local builds if that's helpful (various issues with OpenSSL and musl, for instance).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Debug info seems to be lost when deployed to Lambda
2 participants