mywave82 / opencubicplayer

Open Cubic Player (unix fork). Music visualizer for various tracked music formats (amiga modules, S3M, IT), chiptunes and other formats related to demoscene
https://stian.cubic.org/project-ocp.php
GNU General Public License v2.0
283 stars 19 forks source link

configure.ac: remove `-flat_namespace` usage #50

Closed nandahkrishna closed 2 years ago

nandahkrishna commented 2 years ago

Apple discourages the use of -flat_namespace, and the only reason it continues to exist is for compatibility with very old versions of OS X. Unless the program being built specifically requires the benefits of a flat namespace, it is best to avoid the flag to prevent linker errors, e.g. due to name collisions, as discussed here. (Note: An Apple staff member mentions that the flag is effectively deprecated.)

This PR aims to fix the flat namespace audit failure at Homebrew/homebrew-core#95769.

mywave82 commented 2 years ago

Does it actually work without it?

I know old MacOS required it in order to work - it would compile and install, but fail to run as symbols were unable to resolve.

nandahkrishna commented 2 years ago

I'd forgotten to pass dynamic_lookup to -undefined, which means symbols are resolved at runtime. I then tried building and testing 0.2.93 with Homebrew, and I was able to play audio without problems. Are there any additional/comprehensive tests that I should run to be sure?

alexmyczko commented 2 years ago

0.2.94?

nandahkrishna commented 2 years ago

Hm, 0.2.94 fails to build but the error seems unrelated to this change:

/Applications/Xcode.app/Contents/Developer/usr/bin/make -C cdfs TOPDIR=../.././
clang -g -O2 -fno-common -fPIC -Wall -I../.././ audio.c -o audio.o -c
audio.c:135:2: error: expected statement
        }
        ^
1 error generated.
make[2]: *** [audio.o] Error 1
make[1]: *** [all] Error 2
make: *** [dirs] Error 2
mywave82 commented 2 years ago

Add a before ; before the }

Some C compilers are more strict than others. A Label seems to must have a statement after it, even if it is empty.

I will merge this PR after I fix that semicolon (keeping this PR open as a reminder to myself until next time I'm at a computer)

mywave82 commented 2 years ago

I'd forgotten to pass dynamic_lookup to -undefined, What did you mean by this (I'm not a Mac user/developer)?

nandahkrishna commented 2 years ago

What did you mean by this (I'm not a Mac user/developer)?

In a previous version of the changes I'd proposed here, I'd forgotten to update the argument to -undefined from suppress to dynamic_lookup. We need to change this because omitting the -flat_namespace flag will build shared libraries with a two-level namespace, and this does not support -undefined suppress. By specifying dynamic_lookup, we indicate that the dynamic linker will resolve undefined symbols at runtime.

mywave82 commented 2 years ago

Fixed the issue you encountered in audio.c, and regenerated ./configure too :-) Thank you for the PR

nandahkrishna commented 2 years ago

Thanks! 😄