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 / General fallback on find_library() for static/shared mismatch #483

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

markmaker
Copy link

Description

Add a general fallback on compiler.find_library() when the wanted static preference is not found.

The "ERROR: C ... library 'ws2_32' not found" does not only happen for MSVC compilers, but also when cross-compiling with MinGW.

The compiler.get_id() method does not work for MinGW, because it reports as generic 'gcc'. No other compiler property was found to make a clear distinction between Windows/Non-Windows.

Justification

Successfully cross-compile OpenSSL for Windows using MinGW.

Caveats

The fallback now also happens on compilers other than MSVC. If strict static/shared preference enforcement is needed, another way must be found. I would need guidance to make it so.

@markmaker
Copy link
Author

Sorry, I don't understand why checks fail:

AssertionError: '71.1-1' not found in ['70.1-2', '70.1-1', '67.1-4', '67.1-3', '67.1-2', '67.1-1', '55.2-1'] : for icu

@neheb
Copy link
Collaborator

neheb commented Jun 15, 2022

you need to rebase against latest master.

@eli-schwartz
Copy link
Member

/cc @nazar-pc

@eli-schwartz
Copy link
Member

That is a merge commit instead of a git rebase origin/master. It needs to be a rebase in order to be accepted here.

Copy link
Contributor

@nazar-pc nazar-pc left a comment

Choose a reason for hiding this comment

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

I think strict static/shared separation is indeed needed, it is better for things to fail to compile and tell that result isn't a static library/executable, rather than silently using dynamic linking and failing at runtime when libraries are not found.

I think we can just have this behavior on Windows more generally: update check for MSVC compiler to check for Windows OS and I think it'll be what you want? It doesn't seem like fully static executables are generally a thing on Windows anyway.

@eli-schwartz
Copy link
Member

static: get_option('default_library') == 'static',

I think that this new feature from the unreleased changelog of Meson in git master, is what we actually want: https://mesonbuild.com/Release-notes-for-0-63-0.html#new-prefer_static-builtin-option

Keying off of whether openssl itself is being built statically is kind of a hack. ;) Maybe we can just drop the logic altogether?

@nazar-pc
Copy link
Contributor

Hm... I'm not so sure. The idea there was to include dependencies statically if library is built statically. On Windows though dependencies are neither static nor dynamic, so static property needs to be omitted for things to work at all (when referring to those system libraries at least).

@eli-schwartz
Copy link
Member

The idea there was to include dependencies statically if library is built statically.

But this is wrong, because users might not want to do that. They might want to statically link the library, but dynamically link dependencies. Or they might want to build a "fat" openssl shared library. The new Meson option makes that able to be controlled separately.

@nazar-pc
Copy link
Contributor

Then I don't see how "preference" helps, there should be a way to have precise control and let things crash instead of producing unexpected results.

@markmaker markmaker force-pushed the fix/find-library-on-windows-compilers branch from 3186264 to fce00a5 Compare June 16, 2022 12:04
@markmaker
Copy link
Author

That is a merge commit instead of a git rebase origin/master. It needs to be a rebase in order to be accepted here.

Like this?

@xclaesse
Copy link
Member

xclaesse commented Jun 16, 2022

static: get_option('default_library') == 'static' is definitely wrong, I've tried to make people stop doing that, prefer_static is an imperfect way of achieving that. Also it makes it inconsistent with all other meson projects that won't be doing that, so you might easily static link and dynamic link the same library at the same time.

@neheb
Copy link
Collaborator

neheb commented Nov 11, 2022

this is for openssl. Commit title should have that prefix.

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

5 participants