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

Add CI job to build nativeruntime for Windows #6923

Conversation

utzcoz
Copy link
Member

@utzcoz utzcoz commented Dec 10, 2021

For #6791.

@utzcoz
Copy link
Member Author

utzcoz commented Dec 10, 2021

It must wait #6801 and #6924.

@utzcoz utzcoz force-pushed the add-CI-job-to-build-nativeruntime-for-Windows branch 5 times, most recently from e6797a0 to 00b6b84 Compare December 10, 2021 16:20
@utzcoz utzcoz force-pushed the add-CI-job-to-build-nativeruntime-for-Windows branch 4 times, most recently from 5010da4 to 031071b Compare December 10, 2021 17:48
@utzcoz
Copy link
Member Author

utzcoz commented Dec 10, 2021

I found a very strange problem: the cache restored, but restored icu-bin doesn't have generated .a files.

@utzcoz
Copy link
Member Author

utzcoz commented Dec 10, 2021

Looks like I should find a method to pass value to $GITHUB_ENV at msys2 environment. It's closer to the target. I will take a look at tomorrow.

@utzcoz utzcoz force-pushed the add-CI-job-to-build-nativeruntime-for-Windows branch 4 times, most recently from e7488e3 to fd72b1a Compare December 11, 2021 10:02
@utzcoz
Copy link
Member Author

utzcoz commented Dec 11, 2021

Looks like I should find a method to pass value to $GITHUB_ENV at msys2 environment. It's closer to the target. I will take a look at tomorrow.

I decided to use hard-coded file name to replace GITHUB_ENV for uploading step for Windows.

@utzcoz utzcoz force-pushed the add-CI-job-to-build-nativeruntime-for-Windows branch 2 times, most recently from ea99fdb to 0cf016d Compare December 11, 2021 10:58
@utzcoz
Copy link
Member Author

utzcoz commented Dec 11, 2021

I found a very strange problem: the cache restored, but restored icu-bin doesn't have generated .a files.

I updated cache key with adding cache-v2 to fix this problem for macOS and Linux. But cache doesn't work for Windows with msys2 now. So I let job to build ICU for Windows forcibly temporarily.

@utzcoz
Copy link
Member Author

utzcoz commented Dec 11, 2021

There is another weird problem: Windows generated dll file is small, about 5~6MB, less than other platforms' file size: about 30MB.

@utzcoz utzcoz requested a review from hoisie December 11, 2021 11:30
@utzcoz
Copy link
Member Author

utzcoz commented Dec 11, 2021

@hoisie I think this PR is ready for initial review to looking forward your suggestion.

@hoisie
Copy link
Contributor

hoisie commented Dec 12, 2021

There is another weird problem: Windows generated dll file is small, about 5~6MB, less than other platforms' file size: about 30MB.

This is very likely a libICU data issue. The libICU data file alone is like 25 MB, which could explain the difference.

I don't think it's urgent to fix this right away. The libICU sqlite integration is only used for order by ... collate LOCALIZED and order by ... collate UNICODE, which is not super common.

Anyway, I am ok with getting the build working first, and then we can figure out the runtime issues.

@utzcoz
Copy link
Member Author

utzcoz commented Dec 13, 2021

There is another weird problem: Windows generated dll file is small, about 5~6MB, less than other platforms' file size: about 30MB.

This is very likely a libICU data issue. The libICU data file alone is like 25 MB, which could explain the difference.

I don't think it's urgent to fix this right away. The libICU sqlite integration is only used for order by ... collate LOCALIZED and order by ... collate UNICODE, which is not super common.

Anyway, I am ok with getting the build working first, and then we can figure out the runtime issues.

Thanks. I will give more tries for CI's cache for msys2 to reduce CI times, and omit this dll size problem now.

@utzcoz utzcoz force-pushed the add-CI-job-to-build-nativeruntime-for-Windows branch 2 times, most recently from 9f204a3 to 5a531b2 Compare December 17, 2021 16:29
@utzcoz
Copy link
Member Author

utzcoz commented Dec 17, 2021

Looks like we can set msys2 installation location to define cache directory for msys2 more easily. I will give a try.

@utzcoz utzcoz force-pushed the add-CI-job-to-build-nativeruntime-for-Windows branch 3 times, most recently from 5ccd3b1 to 6a78a4c Compare December 17, 2021 18:49
@utzcoz
Copy link
Member Author

utzcoz commented Dec 17, 2021

Looks like we can set msys2 installation location to define cache directory for msys2 more easily. I will give a try.

Oh, it works, and Windows building time reduced to 3min from 20min. There are some clean-up needs to be done. I will clean up those scripts this weekend for final reviewing. :)

@utzcoz utzcoz force-pushed the add-CI-job-to-build-nativeruntime-for-Windows branch 3 times, most recently from 6ac1248 to 4cf42ad Compare December 20, 2021 15:52
@utzcoz
Copy link
Member Author

utzcoz commented Dec 20, 2021

@hoisie I think it can be reviewed again. I have tested it with cache, and Windows cache works fine. The things to clean up cmake and CI jobs will be left for later PRs, if possible.

@utzcoz
Copy link
Member Author

utzcoz commented Jan 4, 2022

@hoisie I think it can be reviewed again. I have tested it with cache, and Windows cache works fine. The things to clean up cmake and CI jobs will be left for later PRs, if possible.

Hi @hoisie , could you help to review this PR again? I plan to merge it if there is no any important problem should be resolved. Thanks.

Signed-off-by: utzcoz <utzcoz@outlook.com>
@utzcoz utzcoz force-pushed the add-CI-job-to-build-nativeruntime-for-Windows branch from 4cf42ad to b6f1fc3 Compare January 4, 2022 15:02
@utzcoz
Copy link
Member Author

utzcoz commented Jan 4, 2022

@hoisie Looks like self-runner encounters problem again.

@utzcoz utzcoz merged commit 78fabaf into robolectric:master Jan 4, 2022
@utzcoz utzcoz deleted the add-CI-job-to-build-nativeruntime-for-Windows branch January 4, 2022 15:40
@utzcoz
Copy link
Member Author

utzcoz commented Jan 4, 2022

https://github.com/robolectric/robolectric/runs/4704128816?check_suite_focus=true instrumentation-tests failed because of network problem to downloading gradle.

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

Successfully merging this pull request may close these issues.

None yet

2 participants