FluidSynth / fluidsynth

Software synthesizer based on the SoundFont 2 specifications
https://www.fluidsynth.org
GNU Lesser General Public License v2.1
1.82k stars 254 forks source link

Build fails on GCC-only machine, if native symlinks to /usr/bin/{cc,gcc} do not exist. #1301

Closed antecrescent closed 6 months ago

antecrescent commented 8 months ago

FluidSynth version

Reproducable at least since v 2.2.5.

Describe the bug

The build fails on GCC-only machines, if native symlinks to /usr/bin/{cc,gcc} are not available. In the following summary CBUILD refers to the build machine in a cross-compilation scenario.

The issue seems to be the cmake implementation to choose a CBUILD compiler for building the "make_tables" binary. https://github.com/FluidSynth/fluidsynth/blob/eabae3be191279afa3a0221f581d61672732d021/src/CMakeLists.txt#L551-L560

src/gentables/CMakeLists.txt unsets CC to force cmake to look for a C compiler, hopefully a CBUILD one.

Since the CMAKE_C_COMPILER macro is not passed in CONFIGURE_COMMAND, the CMakeDetermineCCompiler module tries and fails to determine a C compiler.

If I'm not mistaken, it first inspects the (unset) CC env var, then tries all of [cc,gcc,cl,bcc,xlc,icx,clang].

Expected behavior

  1. The build system allows user to set a CBUILD compiler
  2. The build succeeds.

Steps to reproduce

  1. Uninstall alternative compilers such as clang.
  2. Remove symlinks to /usr/bin/{cc,gcc}.
  3. Run the build.

Additional context

Workaround: Append -DCMAKE_C_COMPILER=my_compiler to CONFIGURE_COMMAND. A corresponding bug report exists downstream at https://bugs.gentoo.org/833979. This is ultimatively a shortcoming of CMake's support for cross-compilation. There exists a Meson WrapDB port for FluidSynth, which may be worth looking into: https://github.com/mesonbuild/wrapdb/tree/master/subprojects/packagefiles/fluidsynth

derselbst commented 8 months ago

The purpose of this ExternalProject_Add is to let CMake choose the host compiler when cross-compiling, to generate and compile some lookup tables.

Could you please elaborate which distribution comes with an installed host compiler but doesn't provide /usr/bin/cc symlinks?

eli-schwartz commented 8 months ago

The purpose of this ExternalProject_Add is to let CMake choose the host compiler when cross-compiling, to generate and compile some lookup tables.

I wonder how exactly you plan to guarantee that it is a host compiler, rather than a cross compiler named /opt/cross-toolchain/bin/cc which is present in $PATH?

eli-schwartz commented 8 months ago

This is ultimatively a shortcoming of CMake's support for cross-compilation. There exists a Meson WrapDB port for FluidSynth, which may be worth looking into: https://github.com/mesonbuild/wrapdb/tree/master/subprojects/packagefiles/fluidsynth

/cc @kcgen who is the author of this.

derselbst commented 8 months ago

I wonder how exactly you plan to guarantee that it is a host compiler, rather than a cross compiler named /opt/cross-toolchain/bin/cc which is present in $PATH?

If it's not the host compiler, the generated executable will fail to execute, causing the build to fail.

Sorry, but I have no interest in porting to Meson. I'd rather port the lookup tables to C++14 to get rid of this logic altogether.

eli-schwartz commented 8 months ago

If it's not the host compiler, the generated executable will fail to execute, causing the build to fail.

Presumably that is why the bug report author submitted a bug, yes.

derselbst commented 8 months ago

If it's not the host compiler, the generated executable will fail to execute, causing the build to fail.

Presumably that is why the bug report author submitted a bug, yes.

No. Again, a host compiler must be available for compiling fluidsynth. I am questioning the authors use-case of a host compiler being available, but the symlinks /usr/bin/cc not being available, because they were consciously removed. To me, this feels like messing around with the system to see when things start to break rather than being a real problem that Gentoo guys are experiencing.

antecrescent commented 8 months ago

The purpose of this ExternalProject_Add is to let CMake choose the host compiler when cross-compiling [...]

Ideally, the user should be able to choose the host compiler at configure time. This is especially relevant if they installed two or more host compilers on their machine, but prefer to use one over the other.

Could you please elaborate which distribution comes with an installed host compiler but doesn't provide /usr/bin/cc symlinks?

Gentoo Linux officially supports this setup by unsetting the native-symlinks USE flag on the sys-devel/gcc-config package. I can try to provide you with a Dockerfile to replicate this issue, though this may take some time since I'm not familiar with Docker.

Presumably that is why the bug report author submitted a bug [...]

I submitted this bug to highlight the insufficient logic for determining the host compiler.

eli-schwartz commented 8 months ago

No. Again, a host compiler must be available for compiling fluidsynth. I am questioning the authors use-case of a host compiler being available, but the symlinks /usr/bin/cc not being available, because they were consciously removed. To me, this feels like messing around with the system to see when things start to break rather than being a real problem that Gentoo guys are experiencing.

This is kind of circular logic...

I am questioning your belief that the symlink /usr/bin/cc will be "correctly" detected by cmake, even when it does exist.

As I stated originally, cmake does a $PATH lookup for cc, it does not look in /usr/bin for it. This means that it can detect the wrong cc by accident, even when /usr/bin/cc is a native compiler then simply adding the cross toolchain to $PATH can override cc as well.

Your reply was that the result of this is a fluidsynth build that contains a bug: namely, it fails to build with an error. I then pointed out that yes, builds that contain a bug due to failing with an error are, naturally, bugs. Bugs like the one that has been reported in this bug ticket.

I am not at all sure why my clarification that a bug report that is reporting a bug is being used as proof that there is no bug. This has nothing to do with /usr/bin/cc, even if the lack of a /usr/bin/cc is one thing that can expose the bug.

eli-schwartz commented 8 months ago

I'm commenting here because of two reasons:

I know a little bit about build systems, I think. :)

autotools and meson both have out of the box support for cross compilation (you don't have to have two different projects in order to cross compile while also building a native tool such as gentables). CMake is, somewhat famously, "not great" at cross compilation.

You can of course manually work around CMake's deficiencies here. The problem is that you've incompletely done so, because your workaround doesn't include support for the industry standard _FOR_BUILD suffixed environment variables that distinguish between native ("build machine") tools and cross ("host machine") tools.

(CMake also gets the terminology wrong, and calls the native machine the "host", which just causes endless confusion when discussing topics with people who use the well established existing terminology. This is a side effect of the fact that cmake doesn't support mixing host and build machines in a single build, so they don't actually need terminology to describe the difference.)

...

The solution here is to allow a dedicated -D<SOMETHING> option that defines the native compiler which shall be passed to e.g. the gentables project. This is extraneous for build systems such as autotools or meson which provide this for free as part of their streamlined, builtin cross compilation support, but there is no reason you cannot add it to the existing manual cmake hacks for cross compilation.

derselbst commented 8 months ago

This is especially relevant if they installed two or more host compilers on their machine, but prefer to use one over the other.

I understand this use-case from a user perspective. But from a technical perspective, it doesn't make a difference for fluidsynth in this particular case, because the generated lookup tables should always contain the same values, no matter what host compiler was used for gentables. (Although there was an interesting finding by Bernhard in #512 some time ago.)

Gentoo Linux officially supports this setup by unsetting the native-symlinks USE flag on the sys-devel/gcc-config package. I can try to provide you with a Dockerfile to replicate this issue, though this may take some time since I'm not familiar with Docker.

Ok, thanks for the clarification. No need for docker.

I am questioning your belief that the symlink /usr/bin/cc will be "correctly" detected by cmake, even when it does exist.

We are using this logic for 5 years now, and I'm not aware of a bug pointing out this is utterly broken. Hence I do believe, the ExternalProject works pretty well in practice... unless you mess around with the symlinks as we've just learned.

even if the lack of a /usr/bin/cc is one thing that can expose the bug.

I was questioning the motivation behind this being a valid use-case. E.g. if you were to remove /bin/sh this would surely expose many problems, but I personally would not consider reporting that as a bug to anyone. Yet, the comment above has clarified things for me now.

The solution here is to allow a dedicated -D option that defines the native compiler which shall be passed to e.g. the gentables project.

Yes, I agree. I must admit I haven't heard of the _FOR_BUILD suffix industry standard you were referring to... presumably because I'm not an autotools guy. I would prefer the terms "host" and "target" compiler. So in this case I'd suggest introducing a FLUID_HOST_COMPILER flag but I'm open for different names. I'm not sure when I'll have time for implementing this... I welcome a PR.