Skip to content

Fix missing rebuild of all MinGW libraries#205

Open
Blackhex wants to merge 1 commit intomainfrom
fix-mingw-build
Open

Fix missing rebuild of all MinGW libraries#205
Blackhex wants to merge 1 commit intomainfrom
fix-mingw-build

Conversation

@Blackhex
Copy link
Copy Markdown
Member

@Blackhex Blackhex commented Oct 8, 2024

The current .github/scripts/toolchain/build-mingw.sh actually do not re-build MinGW libraries, e.g. libwinpthreads. Also, it seems that build of MinGW is not needed for Cygwin toolchain builds.

@Blackhex
Copy link
Copy Markdown
Member Author

Finishing this PR depends on resolving #208.

Comment thread .github/scripts/build.sh
$ROOT_PATH/.github/scripts/toolchain/build-gcc.sh

if [[ "$PLATFORM" =~ (mingw|cygwin) ]]; then
if [[ "$PLATFORM" =~ mingw ]]; then
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

As discussed with MinGW community there is no use case for building entire MinGW with Cygwin toolchain, especially winpthread library is not compatible with Cygwin system headers. Only MinGW CRT is needed to build Cygwin GCC which is done in build-mingw-crt.sh.

@Blackhex Blackhex force-pushed the fix-mingw-build branch 2 times, most recently from 1cea8c4 to 96b075d Compare March 21, 2025 10:55

case "$ARCH-$PLATFORM" in
x86_64-w64-mingw32)
LDFLAGS="$LDFLAGS -L$TOOLCHAIN_PATH/lib/gcc/$TARGET/lib"
Copy link
Copy Markdown
Member Author

@Blackhex Blackhex Mar 21, 2025

Choose a reason for hiding this comment

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

I don't know why but without this, there is an issue that linker cannot find libgcc_s on x86_64-w64-mingw32. This is not the case for aarch64-w64-mingw32 when linking winpthreads.

Comment thread .github/workflows/advanced.yml Outdated
description: 'MinGW branch to build'
required: false
default: 'woarm64'
default: 'fix-stage2-fakelib'
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

To be reverted before merge.

mingw_branch:
description: 'Mingw branch to build'
required: false
default: 'woarm64'
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

To be reverted before merge.

@Blackhex Blackhex changed the title WIP: Fix missing rebuild of all MinGW libraries Fix missing rebuild of all MinGW libraries Mar 21, 2025
@Blackhex Blackhex marked this pull request as ready for review March 22, 2025 20:07
@Blackhex Blackhex requested review from Copilot, eukarpov, thiru-mcw and vejbomar and removed request for eukarpov March 22, 2025 20:07
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses the issue of missing rebuilds for all MinGW libraries by updating the default branch for MinGW builds and adjusting the build conditions to target the correct platform.

  • Updated default branch values for the MinGW build in both advanced and main workflows
  • Modified the conditional logic for triggering the MinGW build in the advanced workflow

Reviewed Changes

Copilot reviewed 2 out of 5 changed files in this pull request and generated no comments.

File Description
.github/workflows/advanced.yml Updated default branch and build condition to trigger only on w64-mingw32
.github/workflows/main.yml Updated default branch for MinGW builds
Files not reviewed (3)
  • .github/scripts/build.sh: Language not supported
  • .github/scripts/toolchain/build-mingw.sh: Language not supported
  • .github/scripts/toolchain/install-cygwin-headers.sh: Language not supported
Comments suppressed due to low confidence (5)

.github/workflows/advanced.yml:21

  • Verify that the new default branch 'fix-stage2-fakelib' is intentional and aligns with the repository’s branch naming scheme.
default: 'fix-stage2-fakelib'

.github/workflows/advanced.yml:78

  • Ensure that updating the MINGW_BRANCH default to 'fix-stage2-fakelib' does not conflict with other configurations or tooling expectations.
MINGW_BRANCH: ${{ inputs.mingw_branch || 'fix-stage2-fakelib' }}

.github/workflows/advanced.yml:318

  • Review the updated condition to ensure that limiting the MinGW build to matrix.platform 'w64-mingw32' is correct, as it replaces the previous exclusion of 'pc-linux-gnu'.
if: ${{ steps.cache-toolchain.outputs.cache-hit != 'true' && matrix.platform == 'w64-mingw32' }}

.github/workflows/main.yml:21

  • Confirm that the new default branch 'fix-stage2-fakelib' for MinGW in this workflow is consistent with the intended build process.
default: 'fix-stage2-fakelib'

.github/workflows/main.yml:25

  • Ensure that the change to update the MINGW_BRANCH default value to 'fix-stage2-fakelib' is aligned with the overall branch strategy used in the project.
MINGW_BRANCH: ${{ inputs.mingw_branch || 'fix-stage2-fakelib' }}

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.

2 participants