Skip to content

Fix linking failures on Windows/MSYS2 - #135

Merged
mwilliamson merged 1 commit into
mwilliamson:masterfrom
ndabas:fix-windows-msys2
Sep 18, 2026
Merged

mwilliamson merged 1 commit into
mwilliamson:masterfrom
ndabas:fix-windows-msys2

Conversation

@ndabas

@ndabas ndabas commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #132. My hypothesis is that the link errors were caused by libjq being built with the MSYS2 toolchain while the extension was linked by the mingw-w64 shipped in the runner image at C:\mingw64. The fix is simply to prepend the MSYS2 environment's bin directory to the PATH unconditionally.

I've also restructured the action a bit in preparation for adding ARM64 builds in #134.

Passing build on my fork.

@mwilliamson mwilliamson left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I might have misunderstood something, but I think this set of changes conflates two things:

  1. Fixing the failures on more recent versions of setup-msys2 by setting PATH for all architectures, not just x86.

  2. Other changes to how the action is used in preparation for #134.

I know virtually nothing about msys2 but will be ultimately responsible for keeping things working, so it would be useful for me to understand changes like using install vs pacboy in the setup-msys2 action, which I assume (perhaps incorrectly!) fall under (2)? But I don't think that needs to be a blocker in getting #132 fixed.

@ndabas

ndabas commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

To clarify the bit about pacboy: it simply adds the correct package prefix for the current environment. So instead to asking to install, say, mingw-w64-${{ inputs.arch == 'x86' && 'i686' || 'ucrt-x86_64' }}-gcc, we can just ask pacboy to install gcc:p.

So it's a bit of a grey area whether those changes belong to classification 1 or 2. The absolute minimum set of changes to fix the link issue would simply be to prepend the MSYS2 path; the other changes are simplifying the package lookup.

Let me know if you'd like the bare minimum change here instead, and I can move the other changes over to #134.

@mwilliamson

Copy link
Copy Markdown
Owner

Gotcha! Thanks for the explanation. Let's keep this change minimal then, in the spirit of only changing one thing at a time, and the restructuring can happen elsewhere.

@ndabas

ndabas commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

Sure, done -- I have reduced this patch to the minimal set of changes required to fix #132.

@mwilliamson
mwilliamson merged commit 2018586 into mwilliamson:master Sep 18, 2026
74 checks passed
@mwilliamson

Copy link
Copy Markdown
Owner

Thanks!

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.

Linking failures with recent msys2 versions

2 participants