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
Remove conflicting STATIC variable from build #28151
Conversation
We currently set `STATIC=0 LDFLAGS='-extldflags -static -s -w'` as part of the build. This does not make much sense, since STATIC sets LDFLAGS to empty, so we don't pass `-static -s -w`. I believe we do want to pass these flags, as they were originally added for a reason. Binary size changes for istioctl, agent, and pilot in mb: ``` 114 -> 81 104 -> 75 108 -> 77 ``` Helps with istio#26232
/retest |
Possibly want to cherrypick to 1.8, or is this just an early change to get more testing for 1.9? |
|
note |
fwiw, this exact set of linkage comes from stack overflow. I think its the wrong choice: https://stackoverflow.com/questions/22267189/what-does-the-w-flag-mean-when-passed-in-via-the-ldflags-option-to-the-go-comman |
@howardjohn I would recommend adding comments around why these flags are important and affects on size if we remove them. @sdake concern here is valid that many Go programs ship with debug symbols to enable analysis via gdb or delve. However, I don’t know if we really need to support that specially if the binary is distributed per-workload and can lead to increased resource consumption. Some of these options can effect usage with profiling tools so we should ensure that we don’t end up losing out a lot of functionality. |
Additionally, I don’t think release notes none is correct here as it’s loss of debugging capabilities for end user. I’m not sure if anyone relies on it but it’s worth mentioning it if we decide strip off symbols. |
We currently set
STATIC=0 LDFLAGS='-extldflags -static -s -w'
as part of thebuild. This does not make much sense, since STATIC sets LDFLAGS to
empty, so we don't pass
-static -s -w
.I believe we do want to pass these flags, as they were originally added
for a reason.
Binary size changes for istioctl, agent, and pilot in mb:
Helps with #26232
[ ] Configuration Infrastructure
[ ] Docs
[ ] Installation
[ ] Networking
[ ] Performance and Scalability
[ ] Policies and Telemetry
[ ] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastructure
Pull Request Attributes
Please check any characteristics that apply to this pull request.
[ ] Does not have any changes that may affect Istio users.