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

Support nativeruntime building for Windows #6801

Merged
merged 1 commit into from
Dec 13, 2021
Merged

Support nativeruntime building for Windows #6801

merged 1 commit into from
Dec 13, 2021

Conversation

utzcoz
Copy link
Member

@utzcoz utzcoz commented Oct 27, 2021 •
edited

For #6791.

@utzcoz utzcoz mentioned this pull request Oct 27, 2021
Closed
@utzcoz
Copy link
Member Author

utzcoz commented Oct 27, 2021

Looks like, the generated dll is cygnativeruntime.dll when building with cygwin.

@hoisie
Copy link
Contributor

hoisie commented Nov 14, 2021

At a high-level, should we by using Msys, Cygwin, or just Powershell/VSCode?

@utzcoz
Copy link
Member Author

utzcoz commented Nov 14, 2021

At a high-level, should we by using Msys, Cygwin, or just Powershell/VSCode?

We should use one of them. But after testing, our choices are limited:

  1. vstool can't process included header files of sqlite correctly. There are some header files don't have replacement in Windows.
  2. Cygwin needs to modify JDK header file to process JNI building. It looks lie not using normal Win32 checking. We should use cygwin checking at source code. And we should change renaming logic for nativeruntime binary, because it uses different name for generated binary with its cygwin environment name.
  3. msys should use mingw to compile source code from my testing, although mingw also has some problems. If we select to use mingw, we can use mingw directly.

From current testing result, the mingw is better, and I am trying to fix building problem of mingw.

@utzcoz utzcoz force-pushed the trying-to-support-nativeruntime-building-for-Windows branch from 1b22b4e to 38ebd7c Compare November 15, 2021 16:02
@utzcoz utzcoz mentioned this pull request Nov 15, 2021
Closed
@utzcoz utzcoz force-pushed the trying-to-support-nativeruntime-building-for-Windows branch 2 times, most recently from 736a1e2 to 7a94724 Compare November 15, 2021 16:27
@hoisie
Copy link
Contributor

hoisie commented Nov 15, 2021 •
edited

👍 on MinGW. Android itself uses MinGW for cross-compiling for Windows on Linux:

https://cs.android.com/android/platform/superproject/+/master:prebuilts/gcc/linux-x86/host/x86_64-w64-mingw32-4.8/

This happens when host_supported: true and

windows: {
  enabled: true,
},

is set.

e.g.

https://cs.android.com/android/platform/superproject/+/master:development/host/windows/Android.bp;l=9?q=windows%20enabled%20file:bp

I think it would be wise to use a similar environment so we can reuse some of the existing windows support for some files.

@utzcoz
Copy link
Member Author

utzcoz commented Nov 21, 2021

I think it would be wise to use a similar environment so we can reuse some of the existing windows support for some files.

Agreed. From previous testing, there are some includes used by Android modified sqlite can be processed by MSVC tools. MinGW is better for us. I will focus on MinGW at later trying.

@utzcoz utzcoz force-pushed the trying-to-support-nativeruntime-building-for-Windows branch 4 times, most recently from 17b7593 to 9b4face Compare December 6, 2021 18:12
@utzcoz
Copy link
Member Author

utzcoz commented Dec 6, 2021

Cherry-pick some changes from hoisie@0b707d4 for icu building problems.

@utzcoz
Copy link
Member Author

utzcoz commented Dec 6, 2021

org.robolectric.annotation.processing.RobolectricProcessorTest > shouldGenerateMetaInfServicesFile FAILED
Did not find a generated file corresponding to /C:/Users/username/robolectric/processor/build/resources/test/META-INF/services/org.robolectric.internal.ShadowProvider
at app//org.robolectric.annotation.processing.RobolectricProcessorTest.shouldGenerateMetaInfServicesFile(RobolectricProcessorTest.java:167)

@hoisie, do you encounter this failed test when running tests in msys2?

@utzcoz utzcoz force-pushed the trying-to-support-nativeruntime-building-for-Windows branch from 7795bcc to 9f72c48 Compare December 6, 2021 18:53
@utzcoz utzcoz force-pushed the trying-to-support-nativeruntime-building-for-Windows branch from 9f72c48 to ebb46b9 Compare December 7, 2021 15:57
@utzcoz
Copy link
Member Author

utzcoz commented Dec 7, 2021

org.robolectric.annotation.processing.RobolectricProcessorTest > shouldGenerateMetaInfServicesFile FAILED
Did not find a generated file corresponding to /C:/Users/username/robolectric/processor/build/resources/test/META-INF/services/org.robolectric.internal.ShadowProvider
at app//org.robolectric.annotation.processing.RobolectricProcessorTest.shouldGenerateMetaInfServicesFile(RobolectricProcessorTest.java:167)

@hoisie, do you encounter this failed test when running tests in msys2?

Looks like its the problem of com.google.common.io.Resources#getResource. It generates resource path: /C:/Users/username/robolectric/processor/build/resources/test/META-INF/services/org.robolectric.internal.ShadowProvider, but msys2 uses path /c/Users/username/robolectric/processor/build/resources/test/META-INF/services/org.robolectric.internal.ShadowProvider.

hoisie
hoisie approved these changes Dec 9, 2021
View reviewed changes
Copy link
Contributor

@hoisie hoisie left a comment

Choose a reason for hiding this comment

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

This is great! Thanks for doing this.

nativeruntime/cpp/androidfw/CursorWindow.cpp Outdated Show resolved Hide resolved
@utzcoz utzcoz force-pushed the trying-to-support-nativeruntime-building-for-Windows branch from 8cb82cb to bfe134e Compare December 9, 2021 15:53
@utzcoz utzcoz changed the title Trying to support nativeruntime building for Windows Support nativeruntime building for Windows Dec 9, 2021
@utzcoz utzcoz marked this pull request as ready for review December 9, 2021 16:45
@utzcoz
Copy link
Member Author

utzcoz commented Dec 9, 2021

I re-checked it at my Windows machine with msys2, and all tests passed with command ./gradlew :robolectric:test, except some known failed tests(pointed out at #6751). @hoisie could you help to check it on your Windows machine, and internal test suites? Thanks.

@utzcoz
Copy link
Member Author

utzcoz commented Dec 10, 2021 •
edited

Looks like I should support windows when ICU_ROOT_DIR defined, and I have found problem at #6923.

@utzcoz utzcoz mentioned this pull request Dec 10, 2021
Merged
@utzcoz
Support nativeruntime building for Windows
9af48c1 
1. Java's System.mapLibraryName will not add `lib` as prefix for library
file on Windows. We should process it separately to avoid library not
found problem on Windows.
2. Windows' CursorWindow uses in-memory data to replace sys/mman.h.
3. Use MinGW to build icu on Windows.

Signed-off-by: utzcoz <utzcoz@outlook.com>
@utzcoz utzcoz force-pushed the trying-to-support-nativeruntime-building-for-Windows branch from bfe134e to 9af48c1 Compare December 10, 2021 17:41
@utzcoz
Copy link
Member Author

utzcoz commented Dec 11, 2021 •
edited

Looks like I should support windows when ICU_ROOT_DIR defined, and I have found problem at #6923.

@hoisie I have tested new change to support ICU_ROOT_DIR at #6923, and all platform can build nativeruntime with ICU_ROOT_DIR. I will continue work on #6923 for CI building(current problems not related to build config and real source code).

@hoisie
Copy link
Contributor

hoisie commented Dec 12, 2021

@utzcoz looks good, merge whenever you think it's ready

@utzcoz utzcoz merged commit ce29238 into robolectric:master Dec 13, 2021
@utzcoz utzcoz deleted the trying-to-support-nativeruntime-building-for-Windows branch December 13, 2021 00:42
@utzcoz
Copy link
Member Author

utzcoz commented Dec 13, 2021

@utzcoz looks good, merge whenever you think it's ready

Thanks. Merged.

@utzcoz utzcoz linked an issue Dec 27, 2021 that may be closed by this pull request
Supporting nativeruntime building for Windows #6791
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hoisie hoisie hoisie approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Supporting nativeruntime building for Windows
2 participants
@utzcoz @hoisie