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

Fix linking when cross-compiling and a windows resource is first object #4724

Merged
merged 2 commits into from
Mar 19, 2019

Conversation

jon-turney
Copy link
Member

Closes #4463. Supersedes #4040.

It appears that LIB/LINK default to the host architecture if they can't
guess it from the first object.  With the MSVC toolchain, resource files
are (usually) compiled to an arch-neutral .res format.  Always
explicitly provide a '/MACHINE:' argument to avoid it guessing
incorrectly when cross-compiling.
@jon-turney
Copy link
Member Author

jon-turney commented Jan 5, 2019

While this contains a test, I couldn't actually get it to fail in CI, currently. Notwithstanding #4402 (comment) I think this only fails when cross-compiling.

This is also probably missing the correct handling of this scenario when the visual studio librarian is being used along with other language compilers (e.g. D), (if that even makes sense)

For those reasons, this should probably be considered WIP.

@jpakkane
Copy link
Member

Since this is marked as superseding #4040, has this been tested to work for that use case?

@jon-turney
Copy link
Member Author

I could not reproduce the problem (or even get a warning) using the instructions in #4040 (comment).

Perhaps @oleavr can help?

@jpakkane
Copy link
Member

Is the fix itself no longer considered to be WIP?

@jpakkane
Copy link
Member

Merge window is approaching, what should we do with this?

@jon-turney
Copy link
Member Author

Sorry for the slow response.

I'm going to propose this for merging as-is.

It's not the best possible PR, but I think it does fix an actual problem, and I'm unlikely to find the time to write a better PR currently.

Cross-compiling on Windows needs more work, but hopefully at some stage meson will have some CI which covers that scenario, which will exercise the test added here and show that this change addresses #4463.

(I did some manual tests with cross-compiling when writing this change to demonstrate the problem and fix it).

@jpakkane jpakkane merged commit 44dd553 into mesonbuild:master Mar 19, 2019
@jon-turney jon-turney deleted the lib-machine-always branch March 24, 2019 14:14
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