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

[fuchsia] Enable building with --embedder-for-target. #552

Closed
wants to merge 1 commit into from

Conversation

akbiggs
Copy link
Contributor

@akbiggs akbiggs commented Mar 12, 2022

Without these flags, building for Fuchsia with --embedder_for_target complains about position-independent code being required for the embedder.

I'm not sure if there will be undesired side-effects to this change for regular Fuchsia builds?

@akbiggs
Copy link
Contributor Author

akbiggs commented Mar 17, 2022

@chinmaygarde Do you know if there's a way to tell if we're building with --embedder-for-target from buildroot? I'm concerned this PR could hurt performance for Fuchsia Flutter apps so I'm wondering if there's a way to only -fPIC when building using --embedder-for-target.

I know that --embedder-for-target is part of engine and not part of buildroot, but wondering if there's anything I can do despite that.

@chinmaygarde
Copy link
Member

Do you know if there's a way to tell if we're building with --embedder-for-target from buildroot?

No we don't use this on the recipes.

@zanderso
Copy link
Member

I think the relevant part of the recipe might be here: https://flutter.googlesource.com/recipes/+/refs/heads/main/recipes/engine/engine.py#206

@chinmaygarde
Copy link
Member

@akbiggs Are you unblocked on this patch?

@akbiggs
Copy link
Contributor Author

akbiggs commented Mar 24, 2022

Sort of... I need to run performance tests locally to assuage concerns that this won't regress performance before landing this patch, and I've been procrastinating on doing that. Should land it in the next week or so, sorry for sitting on an open PR again.

@zanderso zanderso marked this pull request as draft March 31, 2022 19:55
@zanderso
Copy link
Member

From PR review triage: Shifting this to a draft. Feel free to make active again if/when it needs more review.

@chinmaygarde
Copy link
Member

Closing this for now. Please re-open when progress can be made. Thanks.

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