[Asterisk-code-review] build: Add download capability for external packages (asterisk[13])

[Corey Farrell]

Corey Farrell has posted comments on this change.

Change subject: build: Add download capability for external packages
......................................................................


Patch Set 4: Code-Review-1

(2 comments)

If it weren't for tmpdir being left behind I'd +1 this, so feel free to ignore the ASTTOPDIR comment if you want.  I think 'readlink -f' is available on all except OSX.


Idea for further improvement (separate change):
Can we somehow define 'STANDARD_ABI'?  This would require LOADABLE_MODULES and maybe OPTIONAL_API.  It would conflict with DEBUG_THREADS, DEBUG_FD_LEAKS, MALLOC_DEBUG, etc.  This way the external modules could each depend on STANDARD_ABI.

https://gerrit.asterisk.org/#/c/3769/4/build_tools/download_externals
File build_tools/download_externals:

PS4, Line 8: script_path=$(readlink -f ${0})
           : script_dir=$(dirname ${script_path})
           : ASTTOPDIR=$(readlink -f ${script_dir}/.. )
Nit: can the Makefile just pass ASTTOPDIR as an environmental variable to these scripts?  'readlink -f' is not actually universal.


Line 157: trap "rm -rf ${cache_dir}/${module_dir}" EXIT
This actually replaces the previous trap, so it needs to also clean ${tmpdir}.


-- 
To view, visit https://gerrit.asterisk.org/3769
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id3dcf1289ffd3cb0bbd7dfab3cafbb87be60323a
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-HasComments: Yes