Bug 234547 - net/asterisk16: build fails when textproc/xmlstarlet is installed
Summary: net/asterisk16: build fails when textproc/xmlstarlet is installed
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: Guido Falsi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-01-01 20:27 UTC by Ludovic Desweemer
Modified: 2019-01-17 13:07 UTC (History)
0 users

See Also:
madpilot: maintainer-feedback+
madpilot: merge-quarterly-


Attachments
Use bash in Makefile.moddir_rules (1.51 KB, patch)
2019-01-01 20:27 UTC, Ludovic Desweemer
no flags Details | Diff
New patch (4.89 KB, patch)
2019-01-01 22:19 UTC, Guido Falsi
no flags Details | Diff
opus patch (8.48 KB, patch)
2019-01-11 13:31 UTC, Ludovic Desweemer
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ludovic Desweemer 2019-01-01 20:27:46 UTC
Created attachment 200683 [details]
Use bash in Makefile.moddir_rules

Build fails with the following error when textproc/xmlstarlet is installed :

gmake[4]: Leaving directory '/usr/ports/net/asterisk16/work/asterisk-16.1.1/codecs/ilbc'
Installing modules from codecs...
/bin/sh: declare: not found
/bin/sh: DISABLED_MODS[codec_speex]=1: not found
/bin/sh: DISABLED_MODS[codec_opus]=1: not found
/bin/sh: DISABLED_MODS[codec_silk]=1: not found
/bin/sh: DISABLED_MODS[codec_siren7]=1: not found
/bin/sh: DISABLED_MODS[codec_siren14]=1: not found
/bin/sh: DISABLED_MODS[codec_g729a]=1: not found
/bin/sh: ${DISABLED_MODS[...}: Bad substitution
gmake[3]: *** [/usr/ports/net/asterisk16/work/asterisk-16.1.1/Makefile.moddir_rules:110: install] Error 2
gmake[3]: Leaving directory '/usr/ports/net/asterisk16/work/asterisk-16.1.1/codecs'
gmake[2]: *** [Makefile:614: codecs-install] Error 2
gmake[2]: Leaving directory '/usr/ports/net/asterisk16/work/asterisk-16.1.1'
*** Error code 2
Stop.
make[1]: stopped in /usr/ports/net/asterisk16
*** Error code 1
Stop.
make: stopped in /usr/ports/net/asterisk16
Comment 1 Guido Falsi freebsd_committer freebsd_triage 2019-01-01 21:41:07 UTC
Hi,

thanks for spotting this.

I never noticed this stanza in that Makefile.

It should affect also asterisk13 and asterisk15 which have similar stanzas in their Makefile and configure scripts.

The port build fine when xmlstarlet is not installed so it's not a strict requirement. Also this piece of code seems to try to download things during the install phase, which is not supported (and actually forbidden) by the ports system.

The strange thing is I am unable to reproduce this in my poudriere by adding a BUILD_DEPENDS and a RUN_DEPENDS on xmlstarlet to the port. This is strange, since this should trigger the problem. It also does find the xml binary installed by xmlstarlet via configure.

Anyway, since this functionality is not used by the port, my solution is to simply remove checks for bash and xmlstarlet from configure, so as not to trigger this behavior.

I see no point in adding an unconditional dependency on bash for no actual difference in result.

I'm following up with a patch for you to test.
Comment 2 Guido Falsi freebsd_committer freebsd_triage 2019-01-01 22:19:52 UTC
Created attachment 200687 [details]
New patch

I have created a patch removing xmlstarlet and bash detection from configure.

They are not needed to correctly build the FreeBSD port and it's options. They are actually used to perform operations not supported by the ports tree.

BTW bash would be needed ONLY is xmlstarlet is present, but such a condition can't be explained to the ports system.

Can you test asterisk16 with this patch and report back it builds successfully whatever port is installed?

Thanks!
Comment 3 Ludovic Desweemer 2019-01-02 11:57:01 UTC
(In reply to Guido Falsi from comment #2)

Hi Guido,

Thanks for your promptness. I faced the issue when I was testing the integration of the opus codec in the port. Makefile.moddir_rules tries to download external packages (see https://github.com/asterisk/asterisk/commit/6caf6bcdad003237725bf458a26db8ff74120508). You have right, xmlstarlet and bash are not required here.

I will provide you the full patch to support opus based upon the code from https://github.com/traud/asterisk-opus.
Comment 4 Guido Falsi freebsd_committer freebsd_triage 2019-01-02 12:47:06 UTC
(In reply to Ludovic Desweemer from comment #3)
> (In reply to Guido Falsi from comment #2)
> 
> Hi Guido,
> 
> Thanks for your promptness. I faced the issue when I was testing the
> integration of the opus codec in the port. Makefile.moddir_rules tries to
> download external packages (see
> https://github.com/asterisk/asterisk/commit/
> 6caf6bcdad003237725bf458a26db8ff74120508).

This explains why I was not able to reproduce the error. It will show up only when enabling an external package which needs to be downloaded.

I'd avoid touching the port as is then, since it works fine for the supported situations.

> You have right, xmlstarlet and
> bash are not required here.

Definitely not. Downloading random things during install is actually forbidden by the ports rules. Everything needed should be downloaded during the fetch stage.

This is also actually enforced by poudriere if configured to do so.

> 
> I will provide you the full patch to support opus based upon the code from
> https://github.com/traud/asterisk-opus.

Thanks! patches welcome.

Please remember, as stated above, that no downloading is allowed except in the fetch phase, so you'll have to add the required files as distfiles to the port, generate checksum and maybe extract them where the asterisk ditribution will be able to find them.
Comment 5 Ludovic Desweemer 2019-01-11 13:31:15 UTC
Created attachment 201021 [details]
opus patch

Hi,


Sorry for the late reply. Here is the patch that includes your modifications and that provides support for opus codec. I set DIST_SUBDIR to asterisk16 so every file in distinfo is affected by the change. I didn't integrate untested and experimental patches (file format and native PLC).

Please let me known if you encounter any issues with the patch.

Thanks
Comment 6 Guido Falsi freebsd_committer freebsd_triage 2019-01-17 07:59:16 UTC
Will not merge to quarterly since it's not a bug causing problems on the existing port if only supported options are used.
Comment 7 Guido Falsi freebsd_committer freebsd_triage 2019-01-17 08:04:33 UTC
(In reply to Ludovic Desweemer from comment #5)
> Created attachment 201021 [details]
> opus patch
> 
> Hi,
> 
> 
> Sorry for the late reply. Here is the patch that includes your modifications
> and that provides support for opus codec. I set DIST_SUBDIR to asterisk16 so
> every file in distinfo is affected by the change. I didn't integrate
> untested and experimental patches (file format and native PLC).
> 
> Please let me known if you encounter any issues with the patch.
> 

Thanks for the patch.

There is one issue. Removing the xmlstarlet and bash stanzas would break the functionality in the port to allow users to provide their own menuselect makeopts file with custom selections. This functionality is not really fully supported, but is used and was strongly asked for by some users.

Anyway I'm testing leaving it there, if possible.
Comment 8 commit-hook freebsd_committer freebsd_triage 2019-01-17 13:06:29 UTC
A commit references this bug:

Author: madpilot
Date: Thu Jan 17 13:06:19 UTC 2019
New revision: 490557
URL: https://svnweb.freebsd.org/changeset/ports/490557

Log:
  Add OPUS option to asterisk ports to enable the opus codec.

  While here use DIST_SUBDIR to keep all the asterisk files in one
  subdirectory.

  PR:		234547
  Submitted by:	Ludovic Desweemer <ludovic.desweemer@gmail.com>

Changes:
  head/net/asterisk13/Makefile
  head/net/asterisk13/distinfo
  head/net/asterisk15/Makefile
  head/net/asterisk15/distinfo
  head/net/asterisk16/Makefile
  head/net/asterisk16/distinfo
Comment 9 Guido Falsi freebsd_committer freebsd_triage 2019-01-17 13:07:45 UTC
Thanks for the patch.

I adapted it for asterisk 13 and 15 too and committed it.