On Thu, Sep 06 2018, timo.my...@bittivirhe.fi (Timo Myyrä) wrote:
> Jeremie Courreges-Anglas <j...@wxcvbn.org> writes:
>
>> On Fri, Aug 03 2018, timo.my...@bittivirhe.fi (Timo Myyrä) wrote:
>>
>>> timo.my...@bittivirhe.fi (Timo Myyrä) writes:
>>>
>>>> Solene Rapenne <sol...@perso.pw> writes:
>>>>
>>>>> timo.my...@bittivirhe.fi (Timo Myyrä) wrote:
>>>>>
>>>>>> Hi,
>>>>>> 
>>>>>> Here's a updated port for latest gzdoom version.
>>>>>> Merged the stuff from Solene's port into my old gzdoom port and bumped 
>>>>>> it to
>>>>>> latest version. Tested on amd64 and quick gameplay test seems to work and
>>>>>> installing soundfont and tuning the ini file, the fluidsynth playback 
>>>>>> works.
>>>>>> 
>>>>>> - added patch to fix the fluidsynth library name
>>>>>> 
>>>>>> - dropped old linker args from Makefile, these don't seem to be needed 
>>>>>> at all
>>>>>> 
>>>>>> - Added flag to disable GTK dialogs from building so no need for Gtk 
>>>>>> dependency
>>>>>> 
>>>>>> - fluidsynth is detected at build time so add it as build_depends. At 
>>>>>> run time
>>>>>>   it needs to be installed but gzdoom can use other midi players as well 
>>>>>> so I
>>>>>>   didn't add it to run_depends. 
>>>>>> 
>>>>>> - Dropped previous gxmessage dependy, the game tries kdialog, gxmessage 
>>>>>> and
>>>>>>   finally xmessage to show crash log.
>>>>>> 
>>>>>> - OpenAL needs to be installed to have audio.
>>>>>> 
>>>>>> 
>>>>>> Timo
>>>>>
>>>>> Your port is way better than the one I submitted last month, good work! 
>>>>> Still
>>>>> when using mods, I only have music and no other sound, do you have the 
>>>>> same
>>>>> issue? Doom1 and Doom2 runs fine, so it may be related to the mods...
>>>>
>>>> I tested Doom One mod and only got sound in menu and no gameplay sounds or 
>>>> music
>>>> at all. Got bunch of errors in console so I guess the mods are to blame.
>>>>
>>>> Timo
>>>
>>> Actually its the problem in library loading. There was wrong library names 
>>> for
>>> libmpg123, libsndfile in the code. I've patched those and the sounds seem to
>>> work now after quick test.
>>>
>>> Updated port attached.
>>
>> Here's some late feedback.
>>
>> Regarding the patches, I don't think that #ifdef __OpenBSD__ is a good
>> approach.  (For more on this subject, please look at "#ifdef considered
>> harmful" from Henry Spencer.)  Please don't push such patches upstream.
>> For our needs in ports land, we can just patch and hardcode the use of
>> the un-versioned .so libs.  Ideas for solutions pushable upstream:
>> a cmake option that controls whether dlopened library names should
>> contain a version; or just
>> #ifndef LIBFOO_NAME
>> #define LIBFOO_NAME "libfoo.so.1"
>> ...
>> which would allow downstreams to override the name.
>>
>> About the dlopened libraries: I guess it would be good to list them in
>> LIB_DEPENDS and add the relevant entries to WANTLIB so that gzdoom gets
>> properly updated when its deps change.  (Even if ''make
>> port-lib-depends-check'' complains about extra libs.)

I guess you were not convinced by this point?  If you really don't want
to add those to LIB_DEPENDS, either make sure that gzdoom runs fine
without them, or also add them to RUN_DEPENDS.  But LIB_DEPENDS would be
better IMO.

>> On sparc64, I get this from cmake:
>>
>> ...
>> -- Performing Test HAVE_THREAD_LOCAL
>> -- Performing Test HAVE_THREAD_LOCAL - Failed
>> CMake Error at src/CMakeLists.txt:408 (message):
>>   C++ compiler doesn't support thread_local storage duration specifier
>>
>> COMPILER = base-clang ports-gcc lets the port build and package on
>> sparc64.
>>
>> The rest of the port looks fine.
>
> Ok, here is an another attempt which allows passing the library names to build
> process and adding the COMPILER flag to Makefile.

Still looks mostly good.

Looks like for the gtk3 libs the build now depends on cmake setting the
define, it doesn't seem to be the case for the other dlopen'd libs.
Anyway, this looks fine for now but you'll probably have to discuss with
upstream.

(review aborted, back to $DAYJOB...)
-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply via email to