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

[MSVC/Win32] Wrap for {fmt} doesn't work with default_library=shared #6270

Open
marti4d opened this issue Nov 30, 2019 · 9 comments
Open

[MSVC/Win32] Wrap for {fmt} doesn't work with default_library=shared #6270

marti4d opened this issue Nov 30, 2019 · 9 comments

Comments

@marti4d
Copy link

marti4d commented Nov 30, 2019

The current wrap for the {fmt} library fails when used with default_library=shared. The issue is that {fmt} doesn't use a .def file, and it only uses dllexport when FMT_EXPORT is defined and dllimport when FMT_SHARED is defined.

Since the wrap doesn't include either of these macros, there are no exported symbols in fmt.dll and therefore fmt.lib import library is not produced.

Then, when I try to use it as a dependency of my project, I get an error because the compiler can't find fmt.lib.

I don't know how to modify a wrap, but my suggestion to fix it is either:

  1. Always use static_library() for fmt (Probably not the worst idea for a C++ library anyhow)
  2. Use a code snippet like I used in my Reddit post:
if get_option('default_library') in ['both', 'static']
    lib = static_library('fmt', src, include_directories : inc)
    fmt_static_dep = declare_dependency(link_with : lib, 
                                        include_directories : inc)
    fmt_dep = fmt_static_dep
endif


if get_option('default_library') in ['both', 'shared']
    dll = shared_library('fmt', src, include_directories : inc,
                         cpp_args : '-DFMT_EXPORT')
    fmt_shared_dep = declare_dependency(link_with : dll,
                                        include_directories : inc,
                                        compile_args : '-DFMT_SHARED')
    fmt_dep = fmt_shared_dep
endif
@marti4d marti4d changed the title Wrap for {fmt} dpesn Wrap for {fmt} doesn't work with default_library=shared Nov 30, 2019
@marti4d marti4d changed the title Wrap for {fmt} doesn't work with default_library=shared [MSVC/Win32] Wrap for {fmt} doesn't work with default_library=shared Nov 30, 2019
@xclaesse
Copy link
Member

xclaesse commented Dec 4, 2019

If I understand correctly, what you want is something like:

lib = library('fmt', src, include_directories : inc,
                  cpp_shared_args : '-DFMT_EXPORT')

Where it would build both static and/or shared libraries (depending on default_library) but with an extra CFLAG when compiling the shared library?

I made an attempt at this there: #4159

@marti4d
Copy link
Author

marti4d commented Dec 6, 2019

@xclaesse It would be nice to have something like that, but I thought the point of library and both_libraries was to allow the compiler to build both the DLL and the static lib with the exact same object files.

That wouldn't be the case anymore if different defines were used for each. So I don't mind that I had to be a bit more "explicit" about the fact that there are 2 libraries with separate object files being built (principle of "make expensive things look expensive"). It would be nice if there were a slightly suger-y way of doing that though, since it's so common.

It also might have been better if I could have "synthesized" the object that's returned by both_libraries() that normally allows dependants to access either the shared or static library from the dependency object.

@marti4d
Copy link
Author

marti4d commented Dec 6, 2019

To be clear though - For this issue I just would like somebody to update the {fmt} wrap to do this thing correctly. (Or at least explain what I have to do to fix it - I don't see its source in tree anywhere)

@mensinda
Copy link
Member

mensinda commented Dec 6, 2019

Alternatively, you could try to use fmt as a CMake subproject.

This is how I am currently using fmt in my projects (though you need meson 0.52.x for it to work):

[wrap-file]
directory = fmt-6.0.0

source_url      = https://github.com/fmtlib/fmt/archive/6.0.0.tar.gz
source_filename = fmt-6.0.0.tar.gz
source_hash     = f1907a58d5e86e6c382e51441d92ad9e23aea63827ba47fd647eacc0d3a16c78
cm = import('cmake')
fmt_sp = cm.subproject(
  'fmt',
  cmake_options: [
    '-DFMT_DOC=OFF',
    '-DFMT_INSTALL=OFF',
    '-DFMT_TEST=OFF',
  ]
)

fmt    = fmt_sp.dependency('fmt')

@xclaesse
Copy link
Member

xclaesse commented Dec 6, 2019

@xclaesse It would be nice to have something like that, but I thought the point of library and both_libraries was to allow the compiler to build both the DLL and the static lib with the exact same object files.

It will already transparently build all objects twice in the case you're building the static library without -fPIC. The PR I linked above would also build objects twice if you give different cflags/etc (which is pretty much always the case on Windows). IMHO, the point is really to make it transparent to the user.

That wouldn't be the case anymore if different defines were used for each. So I don't mind that I had to be a bit more "explicit" about the fact that there are 2 libraries with separate object files being built (principle of "make expensive things look expensive"). It would be nice if there were a slightly suger-y way of doing that though, since it's so common.

I think your case is actually very common, it's not niche at all. Anyone wanting to build both static and shared on Windows will have to set a different CFLAGS for exports. So I think it should be supported by Meson. Really sad that PR has been blocked.

It also might have been better if I could have "synthesized" the object that's returned by both_libraries() that normally allows dependants to access either the shared or static library from the dependency object.

Not sure to understand what you mean here, but in the case you use lib=both_libraries() you can get the shared and static libs by doing lib.get_shared_lib() and lib.get_static_lib(). By default lib represents the shared lib.

@marti4d
Copy link
Author

marti4d commented Dec 7, 2019

Ah - I didn't realize that the object files are already generated twice by Meson. Then yes, I guess I'm with you @xclaesse that it seems like there ought to be a way to build both libraries with different args. It seems like the holdup was the syntax though, which I understand, but it's a shame there hasn't been any movement. This feature seems very important for Windows users.

Not sure to understand what you mean here, but in the case you use lib=both_libraries() you can get the shared and static libs by doing lib.get_shared_lib() and lib.get_static_lib(). By default lib represents the shared lib.

Yeah - I think we're talking about the same thing. Since I had to create the libraries separately, it would've been nice if I could've done this:

lib = static_library(...)
dll = shared_library(...)
dep = both_libraries_dependency(lib, dll, ...)
# Now dep.get_shared_lib() and dep.get_static_lib() both work

Instead of

lib_dep = static_library(...)
dll_dep = shared_library(...)
dep = dll_dep
# Now consumer of my library has to know that I have multiple dependencies and they
# need logic to choose the one they want

Of course, as you said I wouldn't have to do any of this if both_libraries() would allow me to pass in different args 🤷‍♂

@xclaesse
Copy link
Member

xclaesse commented Dec 7, 2019

If you've got better API idea, feel free to comment ;-)

@marti4d
Copy link
Author

marti4d commented Dec 9, 2019

@xclaesse I wonder why the last suggestion in the previous issue wasn't taken more seriously? From an outsider perspective, it seems like an elegant API.

And then there's no need to make things all complicated by talking about "merging dictionaries together". You're just concatenating lists in the order they're given. For example:

cargs = target_variants(static_library: ['-DSTATIC'], shared_library: ['-DSHARED'])
lib = library(... cargs: ['-DFOO', cargs]) # Args will be "-DFOO -DSTATIC" 
                                           # or "-DFOO -DSHARED"

@xclaesse
Copy link
Member

xclaesse commented Dec 9, 2019

We clashed a bit because that functionality went through many different syntaxes proposal with no general consensus. Now time passed, I guess we (I) have cold heads now and could be constructive again, let's see.

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

No branches or pull requests

3 participants