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.

Sign up for GitHub

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

Jump to bottom

build: strip swiftshader binaries #18588

Merged
merged 1 commit into from Jun 3, 2019
Merged

build: strip swiftshader binaries #18588

merged 1 commit into from Jun 3, 2019

Conversation

miniak
Copy link
Contributor

@miniak miniak commented Jun 3, 2019 •
edited

Description of Change

Add missing swiftshader libraries (libEGL.so, libGLESv2.so) to LINUX_BINARIES_TO_STRIP.

Checklist

Release Notes

Notes: Fixed electron.zip size increase regression on Linux (compared to Electron 3) by stripping swiftshader libraries (libEGL.so, libGLESv2.so).

@miniak miniak self-assigned this Jun 3, 2019
@miniak
Copy link
Contributor Author

miniak commented Jun 3, 2019 •
edited

Size comparison
Screen Shot 2019-06-03 at 2 43 06 PM

@bpasero
Copy link
Contributor

bpasero commented Jun 3, 2019

Yes please 👍

@codebytere
Copy link
Member

@miniak i'm pretty sure there is an issue linked to this, could you please find it and indicate that this PR closes it?

@miniak
Copy link
Contributor Author

miniak commented Jun 3, 2019

@codebytere this one? #17623

@miniak
Copy link
Contributor Author

miniak commented Jun 3, 2019 •
edited

@codebytere that PR seems to be an issue with a local build, where the main electron binary is not stripped. the size increase due to un-stripped swiftshader binaries was not as massive as the one reported in the PR.

nornagon
nornagon approved these changes Jun 3, 2019
View reviewed changes
Copy link
Member

@nornagon nornagon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a complete list? Should we find every .so? Do we need to strip e.g. the sandbox executable?

jkleinsc
jkleinsc approved these changes Jun 3, 2019
View reviewed changes
Copy link
Contributor

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, assuming the release test builds I just kicked off pass.

@jkleinsc
Copy link
Contributor

jkleinsc commented Jun 3, 2019

x64 linux: https://circleci.com/gh/electron/electron/214698
ia32 linux: https://circleci.com/gh/electron/electron/214709
arm linux: https://circleci.com/gh/electron/electron/214710
arm64 linux: https://circleci.com/gh/electron/electron/214711

@jkleinsc
Copy link
Contributor

jkleinsc commented Jun 3, 2019

Merging as release builds pass.

@jkleinsc jkleinsc merged commit 8b5473c into master Jun 3, 2019
@release-clerk
Copy link

release-clerk bot commented Jun 3, 2019

Release Notes Persisted

Fixed electron.zip size increase regression on Linux (compared to Electron 3) by stripping swiftshader libraries (libEGL.so, libGLESv2.so).

This was referenced Jun 3, 2019
Merged
Merged
@trop
Copy link
Contributor

trop bot commented Jun 3, 2019

I have automatically backported this PR to "6-0-x", please check out #18600

@trop trop bot mentioned this pull request Jun 3, 2019
Merged
@trop
Copy link
Contributor

trop bot commented Jun 3, 2019

I have automatically backported this PR to "5-0-x", please check out #18601

@trop
Copy link
Contributor

trop bot commented Jun 3, 2019

I have automatically backported this PR to "4-2-x", please check out #18602

@jkleinsc jkleinsc deleted the miniak/strip-swiftshader branch June 3, 2019 19:53
@miniak
Copy link
Contributor Author

miniak commented Jun 3, 2019

@nornagon let’s consider that as a follow-up

@sofianguy sofianguy added this to Fixed in 5.0.3 in 5.0.x Jun 4, 2019
@sofianguy sofianguy added this to Fixed in 6.0.0-beta.6 in 6.1.x Jun 5, 2019
@bpasero
Copy link
Contributor

bpasero commented Jul 5, 2019

i am still seeing this folder (Linux, version 1.36):

image

@jkleinsc
Copy link
Contributor

jkleinsc commented Jul 5, 2019

@bpasero this PR didn't remove those files, it just stripped them to make them smaller.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nornagon nornagon nornagon approved these changes

@alexeykuzmin alexeykuzmin alexeykuzmin approved these changes

@jkleinsc jkleinsc jkleinsc approved these changes

@codebytere codebytere codebytere approved these changes

Assignees

@miniak miniak

Labels
None yet
Projects
No open projects
5.0.x
Fixed in 5.0.3
6.1.x
Fixed in 6.0.0-beta.6
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@miniak @bpasero @codebytere @jkleinsc @nornagon @alexeykuzmin