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

Add 'install-name' target property #11954

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

keith-packard
Copy link

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

Copy link
Member

@eli-schwartz eli-schwartz left a 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?

mesonbuild/interpreter/type_checking.py Outdated Show resolved Hide resolved
mesonbuild/build.py Outdated Show resolved Hide resolved
@keith-packard
Copy link
Author

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.

@keith-packard keith-packard force-pushed the install-name branch 3 times, most recently from 580d6ce to cae54a3 Compare July 7, 2023 22:28
@keith-packard
Copy link
Author

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.

Hrm. Looks like it might be as simple as using the 'install_name' property when calling linker.get_soname_args, and making sure that is used when install_name != None?

@keith-packard
Copy link
Author

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).
picolibc/picolibc@e2b9ef1

@tristan957 tristan957 added this to the 1.3.0 milestone Jul 11, 2023
Copy link
Member

@dcbaker dcbaker left a 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.

mesonbuild/build.py Outdated Show resolved Hide resolved
mesonbuild/build.py Outdated Show resolved Hide resolved
mesonbuild/build.py Outdated Show resolved Hide resolved
mesonbuild/utils/universal.py Outdated Show resolved Hide resolved
mesonbuild/utils/universal.py Outdated Show resolved Hide resolved
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 <keithp@keithp.com>
@keith-packard
Copy link
Author

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.

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'.

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 <keithp@keithp.com>
@keith-packard
Copy link
Author

I'd like to get this finished up if possible; am I blocking this? Or is it just 'in process'?

@tristan957
Copy link
Contributor

Seems to just be pending a re-review

@xclaesse xclaesse modified the milestones: 1.3.0, 1.4.0 Oct 17, 2023
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.

Provide 'rename' capability for installing executables and libraries
7 participants