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: cross-build test for windows using LLVM-MinGW in Ubuntu. #288

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

okibcn
Copy link
Contributor

@okibcn okibcn commented Mar 25, 2023

Adds Xbuild matrixed Job with 11 additional build tests

@GitMensch
Copy link
Collaborator

GitMensch commented Mar 25, 2023 via email

@okibcn
Copy link
Contributor Author

okibcn commented Mar 25, 2023

Why using LLVM-mingw instead of plain mingw?

Short Answer: it is the only way so far to support cross-compilation for Windows on Arm.
Long answer: Raw MinGW OS packages don't support cross-compilation for Windows UCRT lib. So the only way to support Windows on Arm, and compile against UCRT library and UWP framework is by using LLVM/clang/LD.LLD/MinGW toolchain. As you can see, it has everything in a small deliverable. On top of that the code generated is way smaller and faster than the old MinGW packages using the legacy MSVCRT. llvm-MinGW can also use the old MSVCRT and generate code for armv7 (32 bits) on Windows. However, Microsoft has not adopted it as a standard.

Copy link
Collaborator

@GitMensch GitMensch left a comment

Choose a reason for hiding this comment

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

When we have a way to store the download to not re-do it every time this can be pulled in.

shell: bash
run: |
## download llvm-MinGW
assetsUrl=$(wget -qO- https://github.com/mstorsjo/llvm-mingw/releases/latest | grep "expanded_assets" | grep -Po 'https[^"]+')
Copy link
Collaborator

Choose a reason for hiding this comment

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

downloading each time seems too much, as far as I remember there's an option to store and re-use later

Copy link
Contributor Author

@okibcn okibcn Mar 25, 2023

Choose a reason for hiding this comment

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

It really doesn't matter, the source is in github, so it is as fast as using caches. Using caches is the method you mention, but it needs to download it no matter what as github store caches internally in .tar.xz format. So it won't be any faster.

I am using that method for nano for windows but for another reason. I use an optimized version of LLVM-MinGW there using musl under Alpine. So, I had to build my own tweaked version. I refresh the cache every week as it takes about 4 hours to build all the tools for all the environments.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It really doesn't matter, the source is in github, so it is as fast as using caches.

Just out of interest: Do you have any docs that the "storage on github" is identical between the release artifacts and the caches? The first is reasonable to be kept with a CDN, while it makes more sense to keep the cache "as near as possible" to its single using worker(s).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Checking back again - using caches (which also allows to check its state and drop the cache if necessary) really seems to be preferable. Please modify and we can pull this in.

.github/workflows/github_actions_build.yml Outdated Show resolved Hide resolved
@okibcn
Copy link
Contributor Author

okibcn commented Mar 25, 2023

BTW, why using appveyor??? Github actions can do the same in the same environment way faster than appveyor. You can even generate new versions and automate CI/CD, generate artifacts, pre-releases, and releases.

@okibcn
Copy link
Contributor Author

okibcn commented Mar 26, 2023

Artifacts are now available for download. The first run is available here: https://github.com/okibcn/PDCursesMod.fork/actions/runs/4522282410#artifacts

@GitMensch
Copy link
Collaborator

BTW, why using appveyor???

Because "back then" there was no GitHub actions available :-)

Github actions can do the same in the same environment way faster than appveyor.

Note sure on that, the OpenWatcom builds take much less than 1 minute, same for VS arm64.
But they all run single or dual-core, while the Actions (currently) have 20 parallel jobs.

@okibcn
Would you be willing to "move" the Windows builds (at least the VS ones) into the GitHub actions?
Should we pull this PR in now (and possibly do the Windows parts in a later PR) or wait?

export PATH="$LLVMBASE/bin:$PATH"

cd ${{ matrix.ENV }}
[[ "${{ matrix.UTF8 }}" == "Y" ]] && make -j$(nproc) demos ${{ matrix.ARCH }}=Y WIDE=Y UTF8=Y || make -j$(nproc) demos ${{ matrix.ARCH }}=Y
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you for adding UTF8/non-wide.

But actually there are three options:

  • UTF8=Y (implies WIDE=Y)
  • WIDE=Y (with the default UTF8=N)
  • WIDE=N (the default)

Can you please add the missing option (second one) to the build matrix?

@Bill-Gray
Copy link
Owner

BTW, why using appveyor???

Aside from the lack of GitHub Actions at the time... I don't think there is a way to do (for example) the OpenWATCOM 1.9 and 2.0 compilations on GitHub Actions. I'm open to correction on that, though.

While AppVeyor is really slow, it's been sufficient for our needs most of the time. But it'll be nice to be able to make good use of GHA and AppVeyor. I don't think it'll be an either/or choice.

Very minor nitpick : in Simon's listing of the three character set options (wide, UTF-8 forced, eight-bit), that last involves "no options set". You can explicitly set WIDE=N, but don't have to.

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

3 participants