Bug 251521

Summary: audio/ecasound: Update to 2.9.3
Product: Ports & Packages Reporter: Neel Chauhan <nc>
Component: Individual Port(s)Assignee: Rainer Hurling <rhurlin>
Status: Closed FIXED    
Severity: Affects Only Me CC: nc, rhurlin
Priority: --- Keywords: buildisok
Version: Latest   
Hardware: Any   
OS: Any   
URL: https://nosignal.fi/ecasound/history.php
Attachments:
Description Flags
Patch (Revision 1)
none
Patch (Revision 2)
none
Patch (Revision 3)
none
Patch (Revision 4)
none
Patch (Revision 5)
none
Patch with alsa and python options corrected none

Description Neel Chauhan freebsd_committer 2020-12-02 06:08:19 UTC
Created attachment 220157 [details]
Patch (Revision 1)

Passes poudriere.
Comment 1 Automation User 2020-12-02 11:26:50 UTC
Build and package info is available at https://gitlab.com/swills/freebsd-ports/pipelines/224061493
Comment 2 Rainer Hurling freebsd_committer 2020-12-02 20:54:10 UTC
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 ;)
Comment 3 Neel Chauhan freebsd_committer 2020-12-02 23:08:42 UTC
Created attachment 220194 [details]
Patch (Revision 2)
Comment 4 Neel Chauhan freebsd_committer 2020-12-02 23:13:53 UTC
Created attachment 220195 [details]
Patch (Revision 3)
Comment 5 Rainer Hurling freebsd_committer 2020-12-03 07:20:18 UTC
(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?
Comment 6 Neel Chauhan freebsd_committer 2020-12-03 16:56:38 UTC
Created attachment 220225 [details]
Patch (Revision 4)

Try this, it worked for me.
Comment 7 Rainer Hurling freebsd_committer 2020-12-03 19:30:59 UTC
(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) ...
Comment 8 Neel Chauhan freebsd_committer 2020-12-03 19:35:58 UTC
Created attachment 220229 [details]
Patch (Revision 5)

In that case if it's okay with you I'll just force ALSA as a dependency.
Comment 9 Rainer Hurling freebsd_committer 2020-12-03 19:50:06 UTC
(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 ...
Comment 10 Rainer Hurling freebsd_committer 2020-12-05 08:37:04 UTC
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.
Comment 11 Rainer Hurling freebsd_committer 2020-12-05 13:52:25 UTC
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 12 Neel Chauhan freebsd_committer 2020-12-05 22:46:17 UTC
Comment on attachment 220278 [details]
Patch with alsa and python options corrected

Thanks for telling me. +1 to your patch.
Comment 13 commit-hook freebsd_committer 2020-12-06 08:03:26 UTC
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
Comment 14 Rainer Hurling freebsd_committer 2020-12-06 08:04:36 UTC
Committed, thanks :)