Created attachment 220157 [details] Patch (Revision 1) Passes poudriere.
Build and package info is available at https://gitlab.com/swills/freebsd-ports/pipelines/224061493
Hi Neel, Thanks for the patch. From a developer's point of view, a few points are still worthy of improvement: (1) #portlint -AC WARN: /poudriere/ports/default/audio/ecasound/pkg-plist: You have defined USE_LDCONFIG, but this port does not install any shared objects. Obviously there is no need for USE_LDCONFIG=yes in the Makefile (2) #make [with DEVELOPER=yes in /etc/make.conf] [..snip..] ====> Running Q/A tests (stage-qa) Error: /usr/local/bin/ecaconvert is linked to /usr/local/lib/libasound.so.2 from audio/alsa-lib but it is not declared as a dependency Warning: you need LIB_DEPENDS+=libasound.so:audio/alsa-lib Error: /usr/local/bin/ecasignalview is linked to /usr/local/lib/libncurses.so.6 from devel/ncurses but it is not declared as a dependency Warning: you need USES+=ncurses So here are two issues: - Adding 'ncurses' to the USES= line of the Makefile - Finding a way to really disable alsa, if OPTION ALSA is disabled (somewhat tricky) (3) Probably there are more issues, but let's see ;) It would be nice if you consider these points and submit a new patch. Please feel free to ask, if you need help ;)
Created attachment 220194 [details] Patch (Revision 2)
Created attachment 220195 [details] Patch (Revision 3)
(In reply to Neel Chauhan from comment #4) ncurses and USE_LDCONFIG are fine now :) What remains is, that if the ALSA (default) option is disabled, the build will still find and link the ALSA library libasound.so if the audio/alsa-lib port is present on the system (issue 2a in comment #2). This can be easily checked by first installing audio/alsa-lib and then building audio/ecasound. To see the error message, DEVELOPERS=yes must be set in /etc/make.conf. I have already tried to start with 'ALSA_CONFIGURE_OFF= --disable-alsa' or similar constructs to suppress ALSA building in the port. Unfortunately without success. Maybe you can think of and try something else?
Created attachment 220225 [details] Patch (Revision 4) Try this, it worked for me.
(In reply to Neel Chauhan from comment #6) I am sorry, but patch revision 4 does not prevent to link against libasound (from audio/alsa-lib) :( On systems with audio/alsa-lib installed, the configure script of audio/ecasound is looking for libasound, even if ALSA is disabled. If I understand right , there is a line with 'AC_SEARCH_LIBS(snd_pcm_open, asound)' in configure.ac:l.943 outside of the '.if test x$alsa_support = x;' testing, which checks for snd_pcm_open under all circumstances: checking for library containing snd_pcm_open... -lasound Theoretically, configure.ac could be patched before the configure script is executed (pre-configure) ...
Created attachment 220229 [details] Patch (Revision 5) In that case if it's okay with you I'll just force ALSA as a dependency.
(In reply to Neel Chauhan from comment #8) This would be one possibility to always use ALSA. But not all users like it ;) I just asked my two mentors for advice. Eventually they have an idea ...
After investigating more and get some hints to try with the original configure script, I found a way which should work :) Could you please try with your patch revision 4 (with ALSA option, with general LIB_DEPENDS=libasound.so:audio/alsa-lib), with this line added: ALSA_CONFIGURE_ENV_OFF= ac_cv_search_snd_pcm_open=no So your OPTIONs area for ALSA should look like this: ALSA_CONFIGURE_ENABLE= alsa ALSA_CONFIGURE_ENV_OFF= ac_cv_search_snd_pcm_open=no ALSA_LIB_DEPENDS= libasound.so:audio/alsa-lib For me, it configures and builds fine now. No more unwanted dependencies against audio/alsa-lib via libasound.so.2. Could you please check, if you could confirm and more important, if this brings any regressions in functionality of the port. Does for example ecaconvert work like expected? Do combinations of other options work like expected? Thanks for testing.
Created attachment 220278 [details] Patch with alsa and python options corrected Hi Neel, It turns out, that ALSA option from comment #10 works correct. I tried several combinations with other options and all seems fine. It turns out, that there is an issue with the python option: ------------------------ Disabling OPTION PYTHON leads to an unexpected error message in stage-qa: ====> Running Q/A tests (stage-qa) Error: '/usr/local/bin/python' is an invalid shebang you need USES=shebangfix for 'bin/ecamonitor' ------------------------ I think, the script from Scripts/qa.sh, which handles shebangfix, is not able to determine the right Python version, if the option is disabled. But that shouldn't matter, because the (Python) Skript ecamonitor and its man page are not wanted and should not be installed, if Python option is disabled. If you agree, we could take my attachment as the base for a commit. What do you think?
Comment on attachment 220278 [details] Patch with alsa and python options corrected Thanks for telling me. +1 to your patch.
A commit references this bug: Author: rhurlin Date: Sun Dec 6 08:03:17 UTC 2020 New revision: 557114 URL: https://svnweb.freebsd.org/changeset/ports/557114 Log: audio/ecasound: Update to 2.9.3 Minor release with some bugfixes. Most important change is full support for Python 3 now. Changelog: https://nosignal.fi/ecasound/history.php PR: 251521 Submitted by: Neel Chauhan <neel@neelc.org> (maintainer) Approved by: arrowd (mentor) Differential Revision: https://reviews.freebsd.org/D27491 Changes: head/audio/ecasound/Makefile head/audio/ecasound/distinfo head/audio/ecasound/pkg-plist
Committed, thanks :)