This repository was archived by the owner on Feb 25, 2025. It is now read-only.
[Windows] Fix dllimport issue in unit tests#35246
Merged
cbracken merged 1 commit intoflutter:mainfrom Aug 9, 2022
cbracken:fix-bad-import
Merged
[Windows] Fix dllimport issue in unit tests#35246cbracken merged 1 commit intoflutter:mainfrom cbracken:fix-bad-import
cbracken merged 1 commit intoflutter:mainfrom
cbracken:fix-bad-import
Conversation
loic-sharma
approved these changes
Aug 8, 2022
Member
loic-sharma
left a comment
loic-sharma
left a comment
There was a problem hiding this comment.
LGTM, thanks for fixing this!
In #35106, I landed a parameter type fix for the declaration of FlutterDesktopEngineGetTextureRegistrar in our public Windows C API. I also added a unit test that called this function. That function (and all others) in our public Windows API is marked FLUTTER_EXPORT, which resolves to __declspec(dllexport) or __declspec(dllimport) depending on whether FLUTTER_DESKTOP_LIBRARY is defined. It can be defined by adding the following build config: //flutter/shell/platform/common:desktop_library_implementation If the function is marked as an import, we get linker warnings that we're importing a function that's defined in the same executable image. This patch resolves this by marking the functions as export. An alternative fix would be to support a third macro resolution that resolves to nothing in the presence of some definition like FLUTTER_NO_EXPORT; however, since flutter_export.h is a public header, I'd prefer not to complicate it further. See: https://github.com/flutter/engine/blob/main/shell/platform/common/public/flutter_export.h See: https://github.com/flutter/engine/blob/a51c7638e702b086dffaae3ce92f52130a2c23ff/shell/platform/common/BUILD.gn#L8-L10
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Aug 9, 2022
emilyabest
pushed a commit
to emilyabest/engine
that referenced
this pull request
Aug 12, 2022
In flutter#35106, I landed a parameter type fix for the declaration of FlutterDesktopEngineGetTextureRegistrar in our public Windows C API. I also added a unit test that called this function. That function (and all others) in our public Windows API is marked FLUTTER_EXPORT, which resolves to __declspec(dllexport) or __declspec(dllimport) depending on whether FLUTTER_DESKTOP_LIBRARY is defined. It can be defined by adding the following build config: //flutter/shell/platform/common:desktop_library_implementation If the function is marked as an import, we get linker warnings that we're importing a function that's defined in the same executable image. This patch resolves this by marking the functions as export. An alternative fix would be to support a third macro resolution that resolves to nothing in the presence of some definition like FLUTTER_NO_EXPORT; however, since flutter_export.h is a public header, I'd prefer not to complicate it further, and this is a unit test that can't be linked against either way. Issue: flutter/flutter#109184 Original issue: flutter/flutter#86617 See: https://github.com/flutter/engine/blob/main/shell/platform/common/public/flutter_export.h See: https://github.com/flutter/engine/blob/a51c7638e702b086dffaae3ce92f52130a2c23ff/shell/platform/common/BUILD.gn#L8-L10 No new tests since this simply fixes a link warning message in unit tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In #35106, I landed a parameter type fix for the
declaration of FlutterDesktopEngineGetTextureRegistrar in our public
Windows C API. I also added a unit test that called this function.
That function (and all others) in our public Windows API is marked
FLUTTER_EXPORT, which resolves to __declspec(dllexport) or
__declspec(dllimport) depending on whether FLUTTER_DESKTOP_LIBRARY is
defined. It can be defined by adding the following build config:
//flutter/shell/platform/common:desktop_library_implementation
If the function is marked as an import, we get linker warnings that
we're importing a function that's defined in the same executable image.
This patch resolves this by marking the functions as export.
An alternative fix would be to support a third macro resolution that
resolves to nothing in the presence of some definition like
FLUTTER_NO_EXPORT; however, since flutter_export.h is a public header,
I'd prefer not to complicate it further, and this is a unit test that
can't be linked against either way.
Issue: flutter/flutter#109184
Original issue: flutter/flutter#86617
See: https://github.com/flutter/engine/blob/main/shell/platform/common/public/flutter_export.h
See:
engine/shell/platform/common/BUILD.gn
Lines 8 to 10 in a51c763
No new tests since this simply fixes a link warning message in unit tests.
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.