Re: [PATCH v3 3/3] meson: Drop the .fa library prefix

[Paolo Bonzini <pbonzini@redhat.com>]

On Wed, May 22, 2024 at 3:45 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On Wed, May 22, 2024 at 12:49 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> > The non-standard .fa library prefix breaks the link source
> > de-duplication done by Meson so drop it.
>
> Can you show the difference in the command lines?
>
> One possibility to force de-duplication of objects is to change
> "link_whole: foo" to "objects: foo.extract_all_objects(recursive:
> false)" in all the declare_dependency() invocations that involve a
> 'fa' archive.
>
> This completely gets rid of the archives, which now become just a
> dummy target. I have gotten reports of "ld" exhausting the limit of
> open files when using thin archives (thin archives contain "symbolic
> links" to the files with the actual object code, thus reducing disk
> usage), and this would also be fixed.

Ah, I forgot that this would also fix an issue with link_whole
dependencies (https://github.com/mesonbuild/meson/pull/8151). It would
be possible to revert 3eacf70bb5a83e4775ad8003cbca63a40f70c8c2 and
instead just add gnutls to the crypto variable.

Paolo

> The disadvantage is requiring a bump to Meson 1.1.x as the minimum
> required version (the recommended version is 1.2.x because earlier
> versions are incompatible with recent macOS). It could be done before
> this patch (because then this patch is a total no-op), or after too to
> fix the immediate issue with sanitizer builds.
>
> Paolo
>
> > The lack of link source de-duplication causes AddressSanitizer to
> > complain ODR violations, and makes GNU ld abort when combined with
> > clang's LTO.
> >
> > Previously, the non-standard prefix was necessary for fork-fuzzing.
> > Meson wraps all standard-prefixed libraries with --start-group and
> > --end-group. This made a fork-fuzz.ld linker script wrapped as well and
> > broke builds. Commit d2e6f9272d33 ("fuzz: remove fork-fuzzing
> > scaffolding") dropped fork-fuzzing so we can now restore the standard
> > prefix.
> >
> > The occurences of the prefix were detected and removed by performing
> > a tree-wide search with 'fa' and .fa (note the quotes and dot).
> >
> > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> > ---
> >  docs/devel/build-system.rst         |  5 -----
> >  meson.build                         | 17 ++---------------
> >  stubs/blk-exp-close-all.c           |  2 +-
> >  .gitlab-ci.d/buildtest-template.yml |  2 --
> >  .gitlab-ci.d/buildtest.yml          |  2 --
> >  gdbstub/meson.build                 |  2 --
> >  tcg/meson.build                     |  2 --
> >  tests/Makefile.include              |  2 +-
> >  tests/qtest/libqos/meson.build      |  1 -
> >  9 files changed, 4 insertions(+), 31 deletions(-)
> >
> > diff --git a/docs/devel/build-system.rst b/docs/devel/build-system.rst
> > index 09caf2f8e199..5baf027b7614 100644
> > --- a/docs/devel/build-system.rst
> > +++ b/docs/devel/build-system.rst
> > @@ -236,15 +236,10 @@ Subsystem sourcesets:
> >    are then turned into static libraries as follows::
> >
> >      libchardev = static_library('chardev', chardev_ss.sources(),
> > -                                name_suffix: 'fa',
> >                                  build_by_default: false)
> >
> >      chardev = declare_dependency(link_whole: libchardev)
> >
> > -  As of Meson 0.55.1, the special ``.fa`` suffix should be used for everything
> > -  that is used with ``link_whole``, to ensure that the link flags are placed
> > -  correctly in the command line.
> > -
> >  Target-independent emulator sourcesets:
> >    Various general purpose helper code is compiled only once and
> >    the .o files are linked into all output binaries that need it.
> > diff --git a/meson.build b/meson.build
> > index 3c3ad0d5f5eb..9eaf218609eb 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -3462,14 +3462,12 @@ endif
> >  qom_ss = qom_ss.apply({})
> >  libqom = static_library('qom', qom_ss.sources() + genh,
> >                          dependencies: [qom_ss.dependencies()],
> > -                        name_suffix: 'fa',
> >                          build_by_default: false)
> >  qom = declare_dependency(link_whole: libqom)
> >
> >  event_loop_base = files('event-loop-base.c')
> >  event_loop_base = static_library('event-loop-base',
> >                                   sources: event_loop_base + genh,
> > -                                 name_suffix: 'fa',
> >                                   build_by_default: false)
> >  event_loop_base = declare_dependency(link_whole: event_loop_base,
> >                                       dependencies: [qom])
> > @@ -3703,7 +3701,6 @@ qemu_syms = custom_target('qemu.syms', output: 'qemu.syms',
> >  authz_ss = authz_ss.apply({})
> >  libauthz = static_library('authz', authz_ss.sources() + genh,
> >                            dependencies: [authz_ss.dependencies()],
> > -                          name_suffix: 'fa',
> >                            build_by_default: false)
> >
> >  authz = declare_dependency(link_whole: libauthz,
> > @@ -3712,7 +3709,6 @@ authz = declare_dependency(link_whole: libauthz,
> >  crypto_ss = crypto_ss.apply({})
> >  libcrypto = static_library('crypto', crypto_ss.sources() + genh,
> >                             dependencies: [crypto_ss.dependencies()],
> > -                           name_suffix: 'fa',
> >                             build_by_default: false)
> >
> >  crypto = declare_dependency(link_whole: libcrypto,
> > @@ -3722,13 +3718,11 @@ io_ss = io_ss.apply({})
> >  libio = static_library('io', io_ss.sources() + genh,
> >                         dependencies: [io_ss.dependencies()],
> >                         link_with: libqemuutil,
> > -                       name_suffix: 'fa',
> >                         build_by_default: false)
> >
> >  io = declare_dependency(link_whole: libio, dependencies: [crypto, qom])
> >
> >  libmigration = static_library('migration', sources: migration_files + genh,
> > -                              name_suffix: 'fa',
> >                                build_by_default: false)
> >  migration = declare_dependency(link_with: libmigration,
> >                                 dependencies: [zlib, qom, io])
> > @@ -3738,7 +3732,6 @@ block_ss = block_ss.apply({})
> >  libblock = static_library('block', block_ss.sources() + genh,
> >                            dependencies: block_ss.dependencies(),
> >                            link_depends: block_syms,
> > -                          name_suffix: 'fa',
> >                            build_by_default: false)
> >
> >  block = declare_dependency(link_whole: [libblock],
> > @@ -3748,7 +3741,6 @@ block = declare_dependency(link_whole: [libblock],
> >  blockdev_ss = blockdev_ss.apply({})
> >  libblockdev = static_library('blockdev', blockdev_ss.sources() + genh,
> >                               dependencies: blockdev_ss.dependencies(),
> > -                             name_suffix: 'fa',
> >                               build_by_default: false)
> >
> >  blockdev = declare_dependency(link_whole: [libblockdev],
> > @@ -3757,13 +3749,11 @@ blockdev = declare_dependency(link_whole: [libblockdev],
> >  qmp_ss = qmp_ss.apply({})
> >  libqmp = static_library('qmp', qmp_ss.sources() + genh,
> >                          dependencies: qmp_ss.dependencies(),
> > -                        name_suffix: 'fa',
> >                          build_by_default: false)
> >
> >  qmp = declare_dependency(link_whole: [libqmp])
> >
> >  libchardev = static_library('chardev', chardev_ss.sources() + genh,
> > -                            name_suffix: 'fa',
> >                              dependencies: chardev_ss.dependencies(),
> >                              build_by_default: false)
> >
> > @@ -3771,7 +3761,6 @@ chardev = declare_dependency(link_whole: libchardev)
> >
> >  hwcore_ss = hwcore_ss.apply({})
> >  libhwcore = static_library('hwcore', sources: hwcore_ss.sources() + genh,
> > -                           name_suffix: 'fa',
> >                             build_by_default: false)
> >  hwcore = declare_dependency(link_whole: libhwcore)
> >  common_ss.add(hwcore)
> > @@ -3807,8 +3796,7 @@ common_all = static_library('common',
> >                              sources: common_ss.all_sources() + genh,
> >                              include_directories: common_user_inc,
> >                              implicit_include_directories: false,
> > -                            dependencies: common_ss.all_dependencies(),
> > -                            name_suffix: 'fa')
> > +                            dependencies: common_ss.all_dependencies())
> >
> >  feature_to_c = find_program('scripts/feature_to_c.py')
> >
> > @@ -3909,8 +3897,7 @@ foreach target : target_dirs
> >                   objects: objects,
> >                   include_directories: target_inc,
> >                   c_args: c_args,
> > -                 build_by_default: false,
> > -                 name_suffix: 'fa')
> > +                 build_by_default: false)
> >
> >    if target.endswith('-softmmu')
> >      execs = [{
> > diff --git a/stubs/blk-exp-close-all.c b/stubs/blk-exp-close-all.c
> > index 1c7131676392..2f68e06d7d05 100644
> > --- a/stubs/blk-exp-close-all.c
> > +++ b/stubs/blk-exp-close-all.c
> > @@ -1,7 +1,7 @@
> >  #include "qemu/osdep.h"
> >  #include "block/export.h"
> >
> > -/* Only used in programs that support block exports (libblockdev.fa) */
> > +/* Only used in programs that support block exports (libblockdev.a) */
> >  void blk_exp_close_all(void)
> >  {
> >  }
> > diff --git a/.gitlab-ci.d/buildtest-template.yml b/.gitlab-ci.d/buildtest-template.yml
> > index 22045add8064..69e468a576ba 100644
> > --- a/.gitlab-ci.d/buildtest-template.yml
> > +++ b/.gitlab-ci.d/buildtest-template.yml
> > @@ -45,10 +45,8 @@
> >      exclude:
> >        - build/**/*.p
> >        - build/**/*.a.p
> > -      - build/**/*.fa.p
> >        - build/**/*.c.o
> >        - build/**/*.c.o.d
> > -      - build/**/*.fa
> >
> >  .common_test_job_template:
> >    extends: .base_job_template
> > diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> > index cfdff175c389..c156e6f1d90e 100644
> > --- a/.gitlab-ci.d/buildtest.yml
> > +++ b/.gitlab-ci.d/buildtest.yml
> > @@ -178,10 +178,8 @@ build-previous-qemu:
> >      exclude:
> >        - build-previous/**/*.p
> >        - build-previous/**/*.a.p
> > -      - build-previous/**/*.fa.p
> >        - build-previous/**/*.c.o
> >        - build-previous/**/*.c.o.d
> > -      - build-previous/**/*.fa
> >    needs:
> >      job: amd64-opensuse-leap-container
> >    variables:
> > diff --git a/gdbstub/meson.build b/gdbstub/meson.build
> > index da5721d8452b..c91e398ae726 100644
> > --- a/gdbstub/meson.build
> > +++ b/gdbstub/meson.build
> > @@ -19,13 +19,11 @@ gdb_system_ss = gdb_system_ss.apply({})
> >
> >  libgdb_user = static_library('gdb_user',
> >                               gdb_user_ss.sources() + genh,
> > -                             name_suffix: 'fa',
> >                               c_args: '-DCONFIG_USER_ONLY',
> >                               build_by_default: false)
> >
> >  libgdb_system = static_library('gdb_system',
> >                                  gdb_system_ss.sources() + genh,
> > -                                name_suffix: 'fa',
> >                                  build_by_default: false)
> >
> >  gdb_user = declare_dependency(link_whole: libgdb_user)
> > diff --git a/tcg/meson.build b/tcg/meson.build
> > index 8251589fd4e9..f941413d5801 100644
> > --- a/tcg/meson.build
> > +++ b/tcg/meson.build
> > @@ -31,7 +31,6 @@ tcg_ss = tcg_ss.apply({})
> >
> >  libtcg_user = static_library('tcg_user',
> >                               tcg_ss.sources() + genh,
> > -                             name_suffix: 'fa',
> >                               c_args: '-DCONFIG_USER_ONLY',
> >                               build_by_default: false)
> >
> > @@ -41,7 +40,6 @@ user_ss.add(tcg_user)
> >
> >  libtcg_system = static_library('tcg_system',
> >                                  tcg_ss.sources() + genh,
> > -                                name_suffix: 'fa',
> >                                  c_args: '-DCONFIG_SOFTMMU',
> >                                  build_by_default: false)
> >
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index c9d1674bd070..d39d5dd6a43e 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -87,7 +87,7 @@ distclean-tcg: $(DISTCLEAN_TCG_TARGET_RULES)
> >  .PHONY: check-venv check-avocado check-acceptance check-acceptance-deprecated-warning
> >
> >  # Build up our target list from the filtered list of ninja targets
> > -TARGETS=$(patsubst libqemu-%.fa, %, $(filter libqemu-%.fa, $(ninja-targets)))
> > +TARGETS=$(patsubst libqemu-%.a, %, $(filter libqemu-%.a, $(ninja-targets)))
> >
> >  TESTS_VENV_TOKEN=$(BUILD_DIR)/pyvenv/tests.group
> >  TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results
> > diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build
> > index 3aed6efcb8d1..45b81c83ade3 100644
> > --- a/tests/qtest/libqos/meson.build
> > +++ b/tests/qtest/libqos/meson.build
> > @@ -68,7 +68,6 @@ if have_virtfs
> >  endif
> >
> >  libqos = static_library('qos', libqos_srcs + genh,
> > -                        name_suffix: 'fa',
> >                          build_by_default: false)
> >
> >  qos = declare_dependency(link_whole: libqos)
> >
> > --
> > 2.45.1
> >