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 clang build on windows #695

Merged
merged 2 commits into from Aug 21, 2023
Merged

support clang build on windows #695

merged 2 commits into from Aug 21, 2023

Conversation

eagleoflqj
Copy link
Member

@eagleoflqj eagleoflqj commented Aug 20, 2023 •
edited

Pull request

Issue tracker

Fixes will automatically close the related issue

Fixes #

Feature

Add clang build script and ci for Windows.

  • Force MT instead of MD as that's the behavior of current MSVC build.
  • The artifact is not uploaded as rime.dll built by clang and MSVC are incompatible. It's not intended to be used by weasel at least for now. (EDIT: per @Techince 's test 64bit rime.dll of MSVC and clang are compatible, though the clang artifact is a bit bigger)
  • The RIME_API in encoder.h is needed for linking rime_test.exe.
    Screenshot from 2023-08-20 23-57-56

Unit test

  • Done

Manual test

  • Done

Code Review

  1. Unit and manual test pass
  2. GitHub Action CI pass
  3. At least one contributor reviews and votes
  4. Can be merged clean without conflicts
  5. PR will be merged by rebase upstream base

Additional Info

@eagleoflqj eagleoflqj marked this pull request as ready for review August 21, 2023 03:53
@eagleoflqj eagleoflqj requested a review from lotem August 21, 2023 04:02
@lotem
Copy link
Member

lotem commented Aug 21, 2023

LGTM

eagleoflqj
eagleoflqj commented Aug 21, 2023
View reviewed changes
.github/workflows/windows-build.yml
uses: actions/cache@v3
with:
path: |
${{ env.BOOST_ROOT }}.7z
${{ env.BOOST_ROOT }}\stage
key: ${{ runner.os }}-boost-${{ env.boost_version }}
key: ${{ runner.os }}-boost-${{ env.boost_version }}-${{ matrix.compiler }}-src
Copy link
Member Author

Choose a reason for hiding this comment

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

In theory, the source cache should be shared by both compilers. But in reality they conflict.
Screenshot from 2023-08-21 17-33-00

@eagleoflqj eagleoflqj merged commit 1bf1fd4 into rime:master Aug 21, 2023
@eagleoflqj eagleoflqj deleted the clang branch August 21, 2023 21:58
groverlynn pushed a commit to groverlynn/librime that referenced this pull request Sep 27, 2023
@eagleoflqj @groverlynn
support clang build on windows (rime#695)
f69f9b9
graphemecluster pushed a commit to TypeDuck-HK/librime that referenced this pull request Nov 2, 2023
@eagleoflqj @graphemecluster
support clang build on windows (rime#695)
185e09a
graphemecluster pushed a commit to TypeDuck-HK/librime that referenced this pull request Nov 8, 2023
@eagleoflqj @graphemecluster
support clang build on windows (rime#695)
0e76655
graphemecluster pushed a commit to TypeDuck-HK/librime that referenced this pull request Mar 18, 2024
@eagleoflqj @graphemecluster
support clang build on windows (rime#695)
0b3e551
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lotem lotem lotem 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.

None yet
2 participants
@eagleoflqj @lotem