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

Build embedder for Windows #4730

Closed
wants to merge 6 commits into from
Closed

Conversation

ds84182
Copy link
Contributor

@ds84182 ds84182 commented Mar 1, 2018

These changes allow the embedder DLL to build on Windows.

Also includes a workaround for DirectoryAssetBundle since FXL doesn't open files with O_BINARY.

@Hixie
Copy link
Contributor

Hixie commented Mar 9, 2018

cc @chinmaygarde for review

@ds84182
Copy link
Contributor Author

ds84182 commented Mar 12, 2018

When https://fuchsia-review.googlesource.com/c/garnet/+/131115 gets merged I'll remove my workaround in DirectoryAssetBundle

@ds84182
Copy link
Contributor Author

ds84182 commented Mar 19, 2018

Updated, PTAL

@ghost
Copy link

ghost commented Mar 22, 2018

LGTM !

@ds84182
Copy link
Contributor Author

ds84182 commented Apr 15, 2018

Rebased

@ds84182 ds84182 reopened this Apr 15, 2018
Several uses of designated initializers, which are non-standard, were
removed.

FLUTTER_EXPORT is defined as dllexport in embedder.cc and dllimport
otherwise.

SAFE_ACCESS uses decltype instead of non-standard __typeof__.

Required libraries (iphlpapi and rpcrt4) are linked into
embedder_unittests
and flutter_engine on Windows.
Two bugs:

To open a directory with CreateFile, FILE_FLAG_BACKUP_SEMANTICS needs to
be specified.

GetFinalPathNameByHandle returns a UNC path, so forward slashes need to
be converted to backslashes.
@ds84182
Copy link
Contributor Author

ds84182 commented Apr 18, 2018

Rebased again

@tvolkert
Copy link
Contributor

/cc @chinmaygarde @jason-simmons for review now that the shell refactor has landed.

@ds84182 it looks like there are some new minor merge conflicts :-/

@chinmaygarde
Copy link
Member

On it. This looks great. I'll fix up the merge conflicts and attempt to land this as soon as the beta 3 stuff is done.

@listepo-alterpost
Copy link

@chinmaygarde any news?

@chinmaygarde
Copy link
Member

All patches required to unblock Windows embedder builds locally and on the bots have landed. The buildbots now test and upload Windows artifacts on each commit. Instructions for accessing these artifacts have been added to the wiki page.

@ds84182: Thank you for your patience while I was retooling the shell and the bots. Please consider adding yourself to the AUTHORS file.

Thanks all.

@ds84182
Copy link
Contributor Author

ds84182 commented May 26, 2018

Thanks! Will do 😊

chinmaygarde pushed a commit that referenced this pull request May 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
6 participants