-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Add 'install-name' target property #11954
base: master
Are you sure you want to change the base?
Conversation
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.
For executables/static_library, this is probably okay. For shared libraries it seems like this would cause an issue for -Wl,-soname
?
I don't need that particular case, so I'd be happy to disable that case for now. Alternatively, those with a better idea of how to build the shared library so that this works correctly are welcome to chime in with suggestions. |
580d6ce
to
cae54a3
Compare
Hrm. Looks like it might be as simple as using the 'install_name' property when calling |
cae54a3
to
bfd3112
Compare
ok, upon review, my previous patch series was broken in various ways. All new bits available, I'd suggest looking at the patches separately so you can see what the backward-compatibility kludge changes without having that cloud reviewing the new feature patch. I've also re-enabled the warning message when you hit the backward compat kludge; that seems better than silently doing something weird and should encourage people to migrate to the newer mechanism. I've also ported picolibc's build to use this new mechanism (while retaining compatibility with older meson versions, ofc). |
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.
The first patch looks good to me. The second I've got some implementation comments about, and like Eli I'm a little nervous about the flattening.
The 'install_name' property is used in place of 'name' when constructing the install target path. This allows projects to generate files using one name and then install them using another. This can be used when generating multiple targets with the same basename but different install directories within the same source directory. Signed-off-by: Keith Packard <[email protected]>
bfd3112
to
e9b0546
Compare
Yeah, the second patch is a total kludge fest. I'd be fine with just dropping it if you think it's too ugly. Existing builds should still work as nothing else has changed. But, it seemed like using the new mechanism to make the old hack a bit cleaner was 'kinda nice'. |
e9b0546
to
4ad8387
Compare
To work around the restriction that only one target with a given name could be created in any project directory, projects could prefix that name with a unique directory name and things would "just work". Take this example: static_library('foo/libbar', [a.c, b.c], install : true, install_dir : 'foo' ) static_library('bletch/libbar', [d.c, e.c], install : true, install_dir : 'bletch' ) This would generate two libraries. The 'lib' prefix is prepended to the directory names by the StaticLibrary code, while the target names include the 'lib' prefix explicitly on 'libbar': libfoo/libbar.a libbletch/libbar.a. Installation would copy the files. The copy code uses only the basename from the source file when constructing the destination, stripping off libfoo/ and libbletch/: libfoo/libbar.a -> foo/libbar.a libbletch/libbar.a -> bletch/libbar.a We need to preserve this behavior while switching the internal implementation to using the new 'install-name' support. 1. When a target name includes a path separator, extract the basename and assign that to the 'install_name_auto' property. Then replace the path separators with '_' to 'flatten' the name, avoiding incidental creation of directories inside the build tree. 2. When install_name is not set, use this install_name_auto instead, first removing any matching prefix (e.g. 'lib' for libraries) as those will be re-added when the install filename is created. Projects which depend on this behavior can be converted to use 'install-name' directly, once they depend on a version of meson with that feature. Signed-off-by: Keith Packard <[email protected]>
4ad8387
to
dbd24c3
Compare
I'd like to get this finished up if possible; am I blocking this? Or is it just 'in process'? |
Seems to just be pending a re-review |
The first patch in this series adds a new target property, 'install-name', which supplants the name for purposes of computing the install filename. This allows building multiple targets with the same basename but different install directories.
The second patch adds a backward-compatibility kludge for existing projects which do this by prepending a directory name to the target name. When that occurs, the target basename is used as the default 'install-name' value and then the target name is 'flattened' by replacing directory separators with '_'.
Closes: #4019