-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 filenames and paths used in DLL shared library generation #417
Conversation
This change breaks two things:
|
I guess we need some way to find out what platform/compiler/language the SharedLibrary target is targetting. Perhaps by implementing them as child types or a set of variables that indicate the target platform and current compiler/language being used, or some other method. Checking for the current OS is not enough because one might be cross-compiling with cross-mingw on Linux, for instance (we do this in GStreamer). I actually got a distinct feeling in various parts of the code that platform-specific things were difficult to do because such information wasn't available in a consistent/well-layered manner. Am I reading things incorrectly? For instance, in this patch, calling detect_c_compiler() felt really hacky to me since we're not necessarily compiling with a C compiler, and library suffix/prefix is really specific to the target, not the environment. |
The environment info is not easily accessible because cross compilation was not a part of the design from day 1. The reason for this was that it was not obvious how it should be designed (and also to avoid doing a big design up front). There are many places in the code that check for platform stuff in a hacky way but who really should be querying properties of And unfortunately library suffix and the like actually do come from the environment (and others). For example the object file suffix is |
That's sort of what I mean by the suffix/prefix/etc being specific to the target (which should know what compiler/host/target is being used) rather than the environment (which is used for the entire project). For instance, plenty of projects compile C/C++ code and also bindings that use them, such as Java or C#. Do you have thoughts on how to go about improving this? I've been trying to think of a re-structuring that would work, but I don't know nearly enough about the code to come up with something. :( Often when I do something platform-specific in Meson, the hacks required kill me a little inside, but I'm sure we can fix that! |
I guess the obvious first step is to open a bug and try to enumerate how things should work. Something like this:
And so on. Especially the parts that currently go wrong. |
I've made a list of what build outputs should be and where they should go. I couldn't find a better place to put it, so here it is in comment form: Library names and installation directoriesNOTE: C/C++/Asm sourceShared library, name 'foo', version 'X[.Y[.Z]]'
iOS does not support shared libraries OS X:
Linux:
Debug symbol file, name 'foo'
Import library, name 'foo'
Static library, name 'foo'
MSVC convention for static vs import libraries has not been consistent in the past, but starting with MSVC 2015, the new CRT follows this convention: However, GCC does not understand this convention and will not find a file named People who want to use an alternative naming scheme can use the Executable, name 'foo'
C# sourceShared library, name 'foo', version 'X[.Y[.Z]]'
Note: The actual installation directory is usually a subdir of libdir, and the subdir is distro-specific. Probably best to have it default to libdir. Executable, name 'foo'
Note: The actual installation directory on !Windows is usually a subdir of libdir, and the subdir is OS-specific. Probably best to have it default to libdir. Debug symbol file, name 'foo' (no versioning)
Note: Debug symbol files location for Windows is from the gtk-sharp installer provided by upstream (must be same as library being debugged). For others, it is unknown. |
Shared library, name 'foo', version 'X[.Y[.Z]]'
That is, technically, the "correct" way (see below for why it's not immediately useful). However, most projects use the DLLs normally go into It is not stressed in the comments above, but import libraries are not versioned in any way, and i know of no practical way to install multiple versions of an import library (from different versions of a The name of the DLL that a binary links to comes from the import library (if an import library is used; in case of direct DLL linking, it comes from the DLL file itself, probably). Anyway, if the import library This is why having real So yes, CMake-built DLLs and implibs use anything, but can be coerced into using Niche and custom buildsystems use arbitrary naming (usually MinGW does not use Direct linking to DLLs without an import library is supported, but:
It is possible to produce an import library for existing DLL (i.e. without the necessity of Debug symbol file, name 'foo'
MinGW-gcc and MinGW-gdb seem to have full support for separate debug info files Either way, the name of the debuginfo file is baked into a shared library, and is either arbitrary ( |
Thanks for the detailed comment LRN! Most of what you've said matches my research so I'm glad to have independent verification for all this. I'll just add a few comments here: DLL naming
You are correct, and this has also been my experience with almost all projects built with MinGW including gstreamer ( When building with MSVC, we should stick with the foo.dll syntax ( I also think that Meson should always generate both
We do both these in Cerbero (on top of Meson) while building GStreamer, and I have both GCC linking to MSVC libraries and vice-versa working flawlessly. I'm not entirely sure how to handle static libraries yet (since there's a name-collision there with import libraries), but we can figure that out. I have some ideas. Debugging symbol files on WindowsI think for now we should keep things simple and just support split-debug files in the same path as the DLLs themselves. So:
I'm not sure if we should give users the option of splitting the debug information out or not. With MSVC |
DLL naming
Generating
You've stated earlier that for MinGW static libraries are named Debugging symbol files on Windows
Neither do i.
I think build-id is, potentially, a better way, since it allows one to just dump debug info into some global directory without worrying about collisions. Package maintainers will appreciate that. That said, you need a modern version of Now, meson could be taught to automate that process, but should it? If so, how much?
This does the "right thing". If meson supports customizing debuginfo filename (i read MSDN, it seems that it does allow .pdb files to be arbitrarily-named), this facility can be used to give custom names to .pdb (MSVC) and .dbg (MinGW debug-link) files. Otherwise, establish some convention ( If some of the behaviour outlined above wouldn't be implemented, ensure that the part that is implemented is also disableable (i.e. if one wants to make use of build-id debuginfo separation by manually post-processing binaries, and meson support automating that by itself, one should be able to tell meson to just leave binaries with debuginfo and don't do anything to them). |
This is not true. You can use
The convention is new and most people don't follow it yet, and it's not clear whether they will. The old convention was to have static libraries also be called However I think we should go with the new convention anyway.
All of this is very pie-in-the-sky stuff. We don't even properly generate Let's take it step by step. Generate |
Can you repoke this so Travis will run the tests? Thanks. |
I'm in the process of reworking this somewhat. Will re-push when I'm done. |
The latest changes in this branch implement most of what was discussed above. What's missing is:
Lots of the tests fail, but many of them fail because they are wrong. I'll be going through them one by one and fixing them. Will comment again when everything is green. |
Well. After a long journey, all tests pass, and a bunch of additional tests have also been added to test the naming scheme. I think we can finally get this merged now. |
All of these should be one of Meson internal exception types instead. Shared library version checking is probably clearer if done with regex rather than split + int. Something like fullmatch
Not a fan of It seems that the tests do not check the case of installing shared libraries from a subproject. We probably don't need one for all platforms, linuxlike should be sufficient. Fortran shared library test fails with MinGW. MSVC dll versioning test fails with MinGW because no-installed-files is missing. Test 6 fails in run_tests.py line 368 with mixed operands (int and str) with Visual Studio 2010 backend. Ninja works. |
Do we have a way of "asserting" via this? I used AssertionError because I wanted to encourage users to report those errors as bugs. They're supposed to never happen, so I'm treating those as literal asserts. Seeing the word "BUG" makes people report bugs. ;)
True. Fixed!
This was supposed to be part of a feature to build
I did try to implement that first, but it was very invasive, so I thought I'd get this in first and then try to work on that. The
New test added.
I'll look into this. Could you share the error that you get?
Fixing this.
This was fixed in d61656d. I've rebased the branch and you should see the actual test failures now. |
This should be fixed now. I wasn't able to test this because my MinGW Fortran toolchain is broken, so please test it if you can.
This is fixed now. I've added several patches that fix bugs in the vs backends. Some were pre-existing, others were because of the DLL versioning patches in this patchset. I've included them all in here and they're all already being tested by our test suite. There is still one test that fails with the vs backends: I have found more bugs in the vs backends that I will be posting in a new PR with tests for that. |
This commit contains several changes to the naming and versioning of shared and static libraries. The details are documented at: #417 Here's a brief summary: * The results of binary and compiler detection via environment functions are now cached so that they can be called repeatedly without performance penalty. This is necessary because every build.SharedLibrary object has to know whether the compiler is MSVC or not (output filenames depend on that), and so the compiler detection has to be called for each object instantiation. * Linux shared libraries don't always have a library version. Sometimes only soversions are specified (and vice-versa), so support both. * Don't use versioned filenames when generating DLLs, DLLs are never versioned using the suffix in the way that .so libraries are. Hence, they don't use "aliases". Only Linux shared libraries use those. * OS X dylibs do not use filename aliases at all. They only use the soversion in the dylib name (libfoo.X.dylib), and that's it. If there's no soversion specified, the dylib is called libfoo.dylib. Further versioning in dylibs is supposed to be done with the -current_version argument to clang, but this is TBD. https://developer.apple.com/library/mac/documentation/DeveloperTools/Conceptual/DynamicLibraries/100-Articles/DynamicLibraryDesignGuidelines.html#//apple_ref/doc/uid/TP40002013-SW23 * Install DLLs into bindir and import libraries into libdir * Static libraries are now always called libfoo.a, even with MSVC * .lib import libraries are always generated when building with MSVC * .dll.a import libraries are always generated when building with MinGW/GCC or MinGW/clang * TODO: Use dlltool if available to generate .dll.a when .lib is generated and vice-versa. * Library and executable suffix/prefixes are now always correctly overriden by the values of the 'name_prefix' and 'name_suffix' keyword arguments.
Fixes installation of subdirs on Windows
Also add new tests for the platform-specific and compiler-specific versioning scheme. A rough summary is: 1. A bug in how run_tests.py:validate_install checked for files has been fixed. Earlier it wasn't checking the install directory properly. 2. Shared libraries are no longer installed in common tests, and the library name/path testing is now done in platform-specific tests. 3. Executables are now always called something?exe in the installed_files.txt file, and the suffix automatically corrected depending on the platform. 4. If a test installs a file called 'no-installed-files', the installed files for that test are not validated. This is required to implement compiler-specific tests for library names/paths such as MSVC vs MinGW 5. The platform-specific file renaming in run_tests.py has been mostly removed since it is broken for shared libraries and isn't needed for static libraries. 6. run_tests.py now reports all missing and extra files. The logic for finding these has been reworked.
The original test's subproject-install check had to be removed because library names are now properly platform-specific
…mmand list This allows us to catch these errors early and print a useful message
This allows us to output either the relative or absolute path as requested. Fixes usage of configure_file inside CustomTarget commands with the VS backends.
This was not tested at all earlier
Without this the filename set by the user and Meson is completely ignored
Also ensure that they're translated from UNIX to native as required
The link arguments for each dependency are split into these three and added to the vcxproj file. Without this targets cannot find the external dependencies.
They are relative to the path of the vcxproj file, not relative to build root
These need to be set via XML tags and not passed directly as AdditionalOptions. Otherwise the project will end up with inconsistent compiler options and the build will fail. Since Meson internals assume that these will be set via a command-line invocation, we need to detect the presence of various flags in buildtype_args and buildtype_link_args and set the correct options in the vcxproj file. Note that this means different configurations (debug/release/etc) cannot be enumerated in the vcxproj/sln files and chosen by the user at build time because arbitrary build characteristics can depend on that. The only way to support that is by doing a full parse and conversion of Meson build files (for all build options) to vcxproj files.
The path is relative to the vcxproj file, not relative to the build root
This aids debugging
It's not easy to understand what these variables mean and what they're used for without some comments
It should always be passed build_to_src otherwise the path for generated files will always be wrong. Passing the vcxproj path as the build_to_src only works for files in the source tree.
This tests the previous commit: vs: Fix usage of mesonlib.File.rel_to_builddir with generated files
Reduces noise in the vcxproj files
I've just posted those commits here instead since they fix some bugs due to changes in this patchset, and also fix pre-existing bugs related to the directory structure and it's hard to separate the two. |
No installed files still seems to be failing. I added a no-installed-files to test 8 and put this in meson.build:
This passes run_tests even though it should not. Other than that this is looking pretty good. |
The first file might be a header file, in which case this test will fail, so check all the files till a match is found instead. Also remove duplicate and incorrect can_compile check. It just checks the suffix and we already check that above.
It depends on the target machine. Without this building for 64-bit fails when using external dependencies.
When the file 'no-installed-files' is installed, require that the test not install any other files. A test for this is pending.
The test is stricter now and our install script doesn't work without bash
I've fixed this with the last two commits. I have a test for the same pending in my tree, but that depends on fixing |
This PR broke |
[], False, ''] | ||
d.targets.append(i) | ||
outdir = self.environment.get_shared_lib_dir() | ||
elif isinstance(t, build.SharedLibrary): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 515, should be build.StaticLibrary
.
A quick fix 0x1997@a875520 |
That's interesting. I think that fix only masks the problem. Going to look into this and will also add a test for it. |
@nirbheek |
Yes, that's what I mean by the fix only masking the problem. We should probably initialize |
elif 'arm' in target_machine.lower(): | ||
self.platform = 'ARM' | ||
else: | ||
raise MesonException('Unsupported Visual Studio platform: ' + target_machine) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know I'm extremely late to the party here, but shouldn't this have consulted host_machine
instead of target_machine
? The docs indicate that target_machine
is only for compiler-type projects, but I currently need to set it to get Visual Studio to cross-compile anything at all...
bindir
libdir
soversion
orversion
in the filename.so
files should have aliases generated for them (dylibs and DLLs do not use aliases)