[asterisk-dev] [Code Review]: Remove development header dependency for asterisk version

Paul Belanger reviewboard at asterisk.org
Thu Jan 26 14:43:09 CST 2012



> On Jan. 26, 2012, 1:38 p.m., Matt Jordan wrote:
> > Double-check that this doesn't create any 'fake' configuration folders when called from TestCase, asterisk, TestConfig, and other places higher then the base runtests.py script.  I don't think it will - but if it does those need to be handled somehow.

Yup, everything looks good.


> On Jan. 26, 2012, 1:38 p.m., Matt Jordan wrote:
> > asterisk/trunk/lib/python/asterisk/version.py, lines 54-70
> > <https://reviewboard.asterisk.org/r/1695/diff/1/?file=23659#file23659line54>
> >
> >     The version_str should probably be a static to the AsteriskVersion class:
> >     1. Once it is set within a process, we wouldn't expect the Asterisk version to change
> >     2. Every subsequent creation of an AsteriskVersion object could check to see if the version number was assigned - if so, it wouldn't need to spawn a process
> >     
> >     Otherwise, the asterisk wrapper class will be spawning a lot of children processes to wait on.

I think it is going to be more complicated adding the version string to a static variable right now, since we have object compare functions.  Or maybe my brains is not working today.

Either way, do you have an example of how you would like to see it?

FWIW: We do a good job of passing the ast_version object round within runtests.py


> On Jan. 26, 2012, 1:38 p.m., Matt Jordan wrote:
> > asterisk/trunk/lib/python/asterisk/version.py, line 65
> > <https://reviewboard.asterisk.org/r/1695/diff/1/?file=23659#file23659line65>
> >
> >     You shouldn't swallow any exceptions here.  It should be logged that we couldn't determine the version as the process failed to spawn.

ACK'd


> On Jan. 26, 2012, 1:38 p.m., Matt Jordan wrote:
> > asterisk/trunk/runtests.py, lines 345-357
> > <https://reviewboard.asterisk.org/r/1695/diff/1/?file=23660#file23660line345>
> >
> >     You can't remove this yet.  We still check buildoptions for some tests.
> >     
> >     When a run-time mechanism for that is available, then this can be removed.
> 
> wdoekes wrote:
>     Or you could adapt those tests to fetch the source path from the binary, e.g.:
>     
>     $ nm `which asterisk` | sed -e '/ t main$/!d;s/ .*//' | addr2line -e `which asterisk`
>     /home/walter/src/asterisk-1.8.x-WRITE/main/asterisk.c:3255
>

boo!

Let me see what I can come up with.


- Paul


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1695/#review5309
-----------------------------------------------------------


On Jan. 26, 2012, 10:39 a.m., Paul Belanger wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1695/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2012, 10:39 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Like the title says, we'll now use the asterisk binary over the development headers to figure out which version of asterisk is installed.
> 
> 
> Diffs
> -----
> 
>   asterisk/trunk/lib/python/asterisk/version.py 3015 
>   asterisk/trunk/runtests.py 3015 
> 
> Diff: https://reviewboard.asterisk.org/r/1695/diff
> 
> 
> Testing
> -------
> 
> local development box
> 
> 
> Thanks,
> 
> Paul
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120126/8cc77134/attachment-0001.htm>


More information about the asterisk-dev mailing list