| 00:06.54 | *** join/#asterisk-dev kharwell (kharwell@nat/digium/x-leefzhmwpvbslgif) |
| 00:34.34 | gtjoseph | so i have a review (2072) that is based on another review that i abandoned. 2072 still shows it as related and when I git review -d 2072 it still pulls down the earlier review. how do i get rid of the earlier one? |
| 00:35.52 | gtjoseph | nevermind. git rebase -i |
| 01:25.57 | coreyfarrell | gtjoseph: still around? |
| 01:26.06 | gtjoseph | yep |
| 01:26.53 | *** part/#asterisk-dev kharwell (kharwell@nat/digium/x-leefzhmwpvbslgif) |
| 01:26.59 | coreyfarrell | if you have some time I'd like to talk to you about some alternatives to some of the plans you have |
| 01:27.19 | gtjoseph | got time now if you do |
| 01:28.19 | coreyfarrell | how many situations of &ref_to_other_module exist? the point of this is to ask how much it would take to create accessors like: void *get_ref_to_other_module(void); |
| 01:28.37 | gtjoseph | they'e in pjproject |
| 01:29.31 | gtjoseph | i was trying to to make res_pjproject proxy those things |
| 01:29.36 | gtjoseph | trying NOT to |
| 01:29.37 | coreyfarrell | maybe I don't understand, can you give me an example? |
| 01:29.49 | gtjoseph | yep, let me find it |
| 01:30.30 | gtjoseph | wcgrep "&pjsip" res |
| 01:32.00 | coreyfarrell | ahh I get it |
| 01:32.23 | gtjoseph | they get resolved now because the modules do -lpj -pjsip, etc |
| 01:32.31 | gtjoseph | all individually |
| 01:32.47 | coreyfarrell | yup it's not dependent on the module load order htey all link to libpj* |
| 01:33.00 | gtjoseph | exactly |
| 01:33.10 | coreyfarrell | so what is actually gained by having a static build instead of having nice scripts to build and install pjproject shared libraries the right way? |
| 01:33.44 | gtjoseph | did you read the email chain? :):) |
| 01:33.59 | gtjoseph | i have several beeves |
| 01:34.22 | coreyfarrell | but I mean if people were using your scripts to build the shared library.. |
| 01:34.23 | gtjoseph | 1 trying to troubleshoot when you have no idea what the runtime environment looks like |
| 01:34.59 | gtjoseph | we still don't know |
| 01:35.08 | gtjoseph | maybe they didn't use the script |
| 01:35.16 | gtjoseph | maybe they used the distro |
| 01:36.20 | gtjoseph | if digium took over pjproject it wouldn't be an issue. :) |
| 01:36.26 | coreyfarrell | that'll still be the case though. people can still decide to not use your static build. if they have an existing libpjproject installed then it would use that anyways. |
| 01:37.10 | gtjoseph | true and we can say "tough shit" when they request support (very nicely of course) |
| 01:37.57 | gtjoseph | got a segfault and not running the static build? reproduce it with the static build and we can help |
| 01:39.22 | coreyfarrell | hmm ok.. so for the bundled pjproject build, have you considered creating a a libasteriskpj.so (similar to libasteriskssl.so)? Instead of linking it into res_pjproject.. |
| 01:39.48 | gtjoseph | could do that what's the difference? |
| 01:40.09 | gtjoseph | oh, make i tnot a module |
| 01:40.13 | gtjoseph | you mean |
| 01:40.15 | coreyfarrell | yes |
| 01:40.50 | coreyfarrell | sorry module_loadorder.h is a hack.. main/asterisk shouldn't need to know how the modules work to be able to load them |
| 01:41.21 | gtjoseph | why? that's like saying systemd or initscripts are a hack |
| 01:41.29 | gtjoseph | we have the info why not use it |
| 01:41.59 | gtjoseph | this gets rid of the GLOBAL_SYMBOLS processing altogether |
| 01:42.29 | gtjoseph | the hack is that we have to dlopen a module twice to get the GLOBAL_SYMBOLS |
| 01:42.30 | coreyfarrell | won't work for 3rd party modules with globals |
| 01:42.36 | gtjoseph | sure it will |
| 01:42.42 | gtjoseph | that's what modules.conf is for |
| 01:43.44 | gtjoseph | besides, what asterisk modules would be dependent on third-party modules? |
| 01:44.19 | gtjoseph | they've still got load_pri to use for sequencing |
| 01:44.33 | coreyfarrell | I'm talking about a group of 3rd party modules.. where one has global exports and the others use those symbols |
| 01:44.43 | gtjoseph | yep, load_pri |
| 01:46.11 | gtjoseph | if the group has multiple exporters, they HAVE to use modules.conf anyway, even today |
| 01:46.22 | gtjoseph | cuz it's BUSTED |
| 01:46.59 | gtjoseph | and don't forget, we still use LAZY |
| 01:47.11 | coreyfarrell | I mean: res_module_1 (has exports), res_module_2 (calls exports from res_module_1). currently those will load in the correct order without load_pri, res_module_1 will load first due to globals |
| 01:48.39 | gtjoseph | it still won't matter unless they are referencing function pointers |
| 01:49.50 | coreyfarrell | it does matter if the module_load() of res_module_2 calls the export |
| 01:50.58 | gtjoseph | nope because ALL modules are dlopened before any have their 'load' functions run |
| 01:52.52 | coreyfarrell | you can't assume that the export from res_module_1 is safe to run before the 'load' function of that module is run |
| 01:54.14 | gtjoseph | load_pri |
| 01:54.18 | coreyfarrell | that' |
| 01:54.19 | gtjoseph | that's it's job |
| 01:55.28 | coreyfarrell | that's a good enough answer for master, but not 13. currently load_pri is not needed to have the module with exports initialized first. |
| 01:56.15 | gtjoseph | well, i can leave the global processing in to catch those cases. |
| 01:57.06 | gtjoseph | the whole GLOBAL/load_pri thing is overly complex now |
| 01:58.01 | gtjoseph | if module writers aren't paying attention to load_pri, then some education is needed |
| 01:59.26 | gtjoseph | do you write third-party modules? If so, do you do it in-tree or out-of-tree? |
| 01:59.42 | coreyfarrell | wasn't for nothing that I trashed what we currently have and started fresh in my module loader experiment. really the module loader needs to be explicitly told "module a requires module b". |
| 02:00.23 | gtjoseph | quite true. that's why i'm using the depend and use tags |
| 02:01.02 | gtjoseph | i looked at your patch, but it had a LOT of changes that I didn't understand |
| 02:01.59 | gtjoseph | or just didn't have the time to delve into. :) |
| 02:02.10 | coreyfarrell | yeah it got a bit out of control. that patch actually caused 'dlclose' to be run on shutdown, which exposed *A LOT* of shutdown races.. |
| 02:02.23 | gtjoseph | :) |
| 02:02.50 | coreyfarrell | did you have a chance to check out the wiki page? |
| 02:03.00 | gtjoseph | i didn't |
| 02:03.20 | coreyfarrell | at this point the patch is stale but the wiki page is not |
| 02:03.25 | gtjoseph | i always have a hard time finding stuff on the wiki |
| 02:03.30 | gtjoseph | link? |
| 02:04.06 | coreyfarrell | https://wiki.asterisk.org/wiki/display/AST/Module+Loader+Ideas |
| 02:08.01 | coreyfarrell | one difference I want to point out is that main/asterisk would not be compiled to know that res_pjsip requires res_pjproject, res_pjsip.manifest would tell asterisk of the dependency. |
| 02:08.01 | gtjoseph | there's a lot of stuff there |
| 02:08.38 | gtjoseph | how do the manifests get read at runtime? |
| 02:09.14 | *** join/#asterisk-dev DivideBy0x00 (~DivideBy0@unaffiliated/divideby0x0) |
| 02:09.40 | coreyfarrell | ast_config_load2 |
| 02:10.26 | gtjoseph | and how are they distributed and where do they live? |
| 02:11.02 | gtjoseph | along side the modules? |
| 02:11.30 | coreyfarrell | that's what the latest patch did.. |
| 02:11.57 | gtjoseph | and is there a dependency resolver in you patch? |
| 02:12.41 | gtjoseph | I.E where does the loader start? |
| 02:13.19 | coreyfarrell | yes.. let me look for it. |
| 02:13.48 | gtjoseph | oh, and there are 2 dependecy trees... one for dlopen and one for running the 'load' function. they may not be the same |
| 02:14.31 | gtjoseph | load-order and initialization-order |
| 02:16.25 | coreyfarrell | actually the load order can be done in initialization order, if you know enough before you load. |
| 02:17.03 | gtjoseph | not necessarily |
| 02:17.30 | coreyfarrell | how? |
| 02:17.51 | gtjoseph | well, maybe if we scrub the modules so they're not setting stupid load_pri |
| 02:18.24 | gtjoseph | we have cases where the load_pri for an exported is higher than modules that use it |
| 02:18.32 | gtjoseph | exporter |
| 02:18.54 | gtjoseph | it only works because of globals. :) |
| 02:19.29 | coreyfarrell | currently my patch did not reimplement load_pri. in either case load_pri would be a suggestion where dependencies are a requirement. |
| 02:19.59 | gtjoseph | so still, how do you know where to start? |
| 02:20.20 | coreyfarrell | with modules that don't have any unfulfilled dependencies :) |
| 02:21.27 | coreyfarrell | if you put autoload=no, load=chan_pjsip into your modules.conf, the load of chan_pjsip would force all deps to load |
| 02:22.02 | coreyfarrell | by default it loads in the same order as listed in the manifest which is generated in the same order as the MODULEINFO block of sources. |
| 02:22.50 | gtjoseph | but don't you have to build a single list |
| 02:23.58 | gtjoseph | you basically have to do what make does |
| 02:24.16 | coreyfarrell | not sure what you mean? |
| 02:24.56 | gtjoseph | a needs b, c, d b needs x, y y needs m, n, o |
| 02:25.26 | gtjoseph | you need to load m, n, o first |
| 02:25.53 | coreyfarrell | oh, so to go from 'load=a' to knowing that we need: m,n,o,y,x,b,c,d,a ? |
| 02:26.00 | gtjoseph | exactly |
| 02:26.21 | gtjoseph | actually goiong from 'load everything' |
| 02:26.37 | coreyfarrell | ah module_manifest_build_alldeps in module_manifest.c (the end of the file) |
| 02:26.53 | coreyfarrell | at startup it builds a full list of everything needed by each module. |
| 02:27.17 | gtjoseph | by each module? |
| 02:27.56 | coreyfarrell | so at runtime the module loader would know that a requires m,n,o,y,x,b,c,d |
| 02:28.06 | coreyfarrell | y needs m, n, o |
| 02:28.20 | coreyfarrell | b needs x, y, m, n, o |
| 02:28.25 | gtjoseph | so assuming autoload is yes, you just start with any module |
| 02:28.49 | gtjoseph | or is there a master manifest? |
| 02:29.10 | gtjoseph | or is that what modules.conf becomes |
| 02:31.02 | coreyfarrell | there isn't really a master manifest.. modules.conf becomes the place for 'config dependent requirements'. |
| 02:31.20 | gtjoseph | so autoload? |
| 02:31.46 | coreyfarrell | let me look at something again |
| 02:32.21 | gtjoseph | i guess if each manifest gets fully resolved, then it doesn't matter where you start |
| 02:33.26 | coreyfarrell | yes but there is one important thing on the TODO list about that. |
| 02:33.51 | coreyfarrell | chan_sip does not have any dependency on format_whatever, but obviously the format needs to be loaded first. |
| 02:33.53 | gtjoseph | isn't there always? |
| 02:34.23 | gtjoseph | sure, they'd definately need to be cleanup |
| 02:34.35 | gtjoseph | on the MODULEINFO structures |
| 02:35.23 | gtjoseph | ok, so i can go the libastpjproject.so route for 13 and that eliminates the need for messing with the loader for now |
| 02:36.06 | gtjoseph | the next question is... can you strip down your patch for master to just handling load order as a start? |
| 02:36.36 | gtjoseph | i think it's too complex as a single unit |
| 02:37.27 | gtjoseph | <PROTECTED> |
| 02:38.30 | coreyfarrell | it would be really helpful to me if someone would review the concept of the manifest files |
| 02:38.43 | gtjoseph | that i can do |
| 02:39.05 | gtjoseph | it looked sane anyway at first glance |
| 02:40.43 | coreyfarrell | well the manifest files and the modules.conf kind of go together.. |
| 02:40.45 | gtjoseph | let me look at it overnight and we can chat more tomorrow |
| 02:41.24 | coreyfarrell | sounds great thanks |
| 02:41.55 | gtjoseph | no prob. i only had a can of soup to eat today, so i'm off for the night. :) |
| 02:42.42 | coreyfarrell | well get something to eat.. have a good night |
| 05:45.19 | *** join/#asterisk-dev bof22 (~Thunderbi@185.13.183.107) |
| 05:45.31 | *** join/#asterisk-dev bof22 (~Thunderbi@185.13.183.107) |
| 07:46.00 | *** join/#asterisk-dev hehol (~hehol@gatekeeper.loca.net) |
| 08:16.26 | *** join/#asterisk-dev pchero_work (~pchero@109.70.54.56) |
| 08:20.50 | *** join/#asterisk-dev tzafrir (~tzafrir@local.xorcom.com) |
| 08:20.50 | *** mode/#asterisk-dev [+o tzafrir] by ChanServ |
| 08:45.10 | *** join/#asterisk-dev bof22 (~Thunderbi@185.13.183.107) |
| 12:49.47 | *** join/#asterisk-dev bof22 (~Thunderbi@185.13.183.107) |
| 12:52.54 | *** join/#asterisk-dev oej (~oej@195.149.146.45) |
| 12:52.54 | *** mode/#asterisk-dev [+o oej] by ChanServ |
| 13:15.20 | *** join/#asterisk-dev [TK]D-Fender (~chatzilla@216-191-106-163.dedicated.allstream.net) |
| 13:24.29 | *** join/#asterisk-dev mjordan (~mjordan@75.76.55.191) |
| 13:24.29 | *** mode/#asterisk-dev [+o mjordan] by ChanServ |
| 13:43.16 | *** join/#asterisk-dev newtonr (RustyNewto@nat/digium/x-oxltklfjvuchnbtm) |
| 13:43.16 | *** mode/#asterisk-dev [+o newtonr] by ChanServ |
| 14:22.32 | *** join/#asterisk-dev newtonr (RustyNewto@nat/digium/x-ddtlcqdymwvysysx) |
| 14:22.32 | *** mode/#asterisk-dev [+o newtonr] by ChanServ |
| 14:38.48 | *** join/#asterisk-dev goddva (~goddva@2a02:fe0:c510:e300:ae9e:17ff:fe91:a8c9) |
| 15:02.20 | gtjoseph | coreyfarrell: got some feedback on the manifest whenever you have time. |
| 15:02.36 | *** join/#asterisk-dev w9sh (~sph@c-73-54-187-250.hsd1.ga.comcast.net) |
| 15:03.10 | file | should your reviews be held in WIP until you two discuss things further? |
| 15:03.25 | gtjoseph | yep, i;m doing that now |
| 15:03.34 | file | ack |
| 15:03.50 | *** join/#asterisk-dev mjordan (mjordan@nat/digium/x-zubfoycdzkueboph) |
| 15:03.50 | *** mode/#asterisk-dev [+o mjordan] by ChanServ |
| 15:05.49 | coreyfarrell | good morning gtjoseph |
| 15:06.02 | gtjoseph | good morning coreyfarrell |
| 15:07.06 | gtjoseph | coreyfarrell: want some feedback now or later in the day? |
| 15:07.19 | coreyfarrell | now is good |
| 15:07.38 | gtjoseph | cool. i'm looking at the wiki page now...\ |
| 15:08.07 | gtjoseph | so for module level... I'd remove the load priority and export globals |
| 15:08.28 | gtjoseph | my thinkingis that the dependency tree shoul dbe the only place to express order |
| 15:09.59 | coreyfarrell | yup I think I agree with removing load priority and that 'optional dependencies' would work better.. like a way to say "if format_wav and chan_sip are being loaded, the format needs to be first". |
| 15:10.26 | gtjoseph | which prings me to... |
| 15:10.27 | coreyfarrell | the 'export globals' doesn't actually control load order in this case, it controls the dlopen options |
| 15:10.29 | gtjoseph | brings |
| 15:10.43 | gtjoseph | Ah |
| 15:11.01 | gtjoseph | ok, ok |
| 15:11.21 | gtjoseph | maybe rename to dlopen_options |
| 15:12.29 | gtjoseph | now for the dependencies... |
| 15:12.48 | gtjoseph | although systemd gets a bad rap, i kind of like the concepts |
| 15:12.55 | coreyfarrell | but the dlopen flags aren't intended to be exposed through the manifest. just export symbols or don't |
| 15:13.00 | gtjoseph | required, required-by, wants, wanted-by |
| 15:13.29 | gtjoseph | ok, i just don't want people to get confused by it's current meaning |
| 15:14.10 | gtjoseph | back to the dependencies... |
| 15:14.28 | gtjoseph | i ran into an issue yesterday that I think your 'provides' might solve but i wanted to make sure... |
| 15:14.51 | coreyfarrell | I think that would be a documentation issue.. ensure it's clear that order is controlled by deps only |
| 15:15.36 | gtjoseph | if your dialplan does an 'include = parkedcalls', pbx.c will barf if res_parking hasn't been loaded yet to ptovide the context. |
| 15:15.43 | gtjoseph | cool on the docs |
| 15:16.10 | newtonr | what is the yum/dnf equivalent to apt-file on Debian? |
| 15:16.58 | gtjoseph | newtonr: if apt file tells you what package a file is in, it's 'dnf provides <path>' |
| 15:17.05 | newtonr | gtjoseph, thanks! |
| 15:17.07 | *** join/#asterisk-dev kharwell (kharwell@nat/digium/x-etiyklojhyxyrzex) |
| 15:18.26 | gtjoseph | coreyfarrell: as you pointed out, there are cases where a core function might want something but can't express it as a depends or uses but the module knows what it provides |
| 15:19.10 | coreyfarrell | gtjoseph: I don't think a module manifest would list a pbx context as a provide.. remember the manifest is for compile-time dependencies, so if 'parkedcalls' is a configurable context that probably wouldn't be a provide. |
| 15:19.21 | gtjoseph | i'm just thinking that there are 4 types, 2 hard, 2 soft... 2 i need, 2 i provide |
| 15:19.51 | gtjoseph | bu tmaybe it provides 'CONTEXTS' |
| 15:20.12 | gtjoseph | or 'DIALPLAN_FUNCTION' |
| 15:20.20 | *** join/#asterisk-dev putnopvut (putnopvut@asterisk/master-of-queues/mmichelson) |
| 15:20.20 | *** mode/#asterisk-dev [+o putnopvut] by ChanServ |
| 15:20.45 | gtjoseph | there are probably some pseudo-targets like that |
| 15:20.58 | gtjoseph | CODECS , FORMATS, |
| 15:21.07 | coreyfarrell | though in this case maybe it would be better from pbx_config to 'want' res_parking. what we were talking about with optional deps where it would set load order but not force res_parking to load. |
| 15:21.09 | gtjoseph | or CODEC, FORMAT |
| 15:21.31 | gtjoseph | but in this case, it does need to load |
| 15:21.35 | gtjoseph | or the context won't be there |
| 15:21.53 | coreyfarrell | only if extensions.conf has 'include => parkedcalls' |
| 15:21.57 | gtjoseph | plus it's be better to say res_parking is wanteds-by pbx |
| 15:22.32 | gtjoseph | true but if you use res_parking, then your dialplan m,ust reference it |
| 15:22.37 | gtjoseph | or there's no point |
| 15:24.00 | gtjoseph | if we do it in pbx_config, then any time a new module provides something, pbx_config has to be updated |
| 15:24.27 | gtjoseph | i think it's cleaner if it becomes kind of a well-known target |
| 15:28.33 | gtjoseph | The only other thing I was thinking about was dlopen order vs initialization order. I'm hoping that with a better expression of the dependency tree, we shoul dbe able to dlopen and initialize at the same time but... |
| 15:28.54 | coreyfarrell | not sure I follow you on the 'include => parkedcalls'. what do you think should be in the manifests for this? keep in mind there is no pbx.manifest, manifests are for modules only. |
| 15:29.02 | gtjoseph | exactly |
| 15:29.48 | gtjoseph | i'm thinking that in res_parking we do a wanted-by pbx, or provides CONTEXT |
| 15:30.06 | gtjoseph | or wanted-by pbx_config, etc |
| 15:31.26 | gtjoseph | provides DIALPLAN_FUNCTION, DIALPLAN_APP, CHANNEL, CDR, CEL, TEXT, etc |
| 15:31.32 | gtjoseph | TEST |
| 15:31.44 | coreyfarrell | ok.. so maybe res_parking is <wantedby type="virtual">dialplan</wantedby> and pbx_config has <provides type="virtual">dialplan</provides> |
| 15:32.11 | gtjoseph | ah, that might be cool |
| 15:33.45 | gtjoseph | doe spbx_config provide it or publish or |
| 15:34.04 | gtjoseph | actually provides is ok |
| 15:34.19 | gtjoseph | it's providing the dialplan |
| 15:35.05 | gtjoseph | to we need the virtual on the want, requi ? |
| 15:35.08 | gtjoseph | require |
| 15:35.17 | gtjoseph | maybe that should be on the provides |
| 15:35.34 | gtjoseph | that way the requiring module doesn't have to know what type it is |
| 15:35.44 | coreyfarrell | what do you mean? |
| 15:36.05 | gtjoseph | well, i'm thinking the namespace should be 'global' |
| 15:36.40 | gtjoseph | t5ype=virtual should only be specified by the thing that publishes the name |
| 15:37.31 | gtjoseph | ./configure could create the default ones for the system capabilities it discovers |
| 15:37.39 | gtjoseph | as external |
| 15:39.49 | coreyfarrell | I think the types need to be specified.. nothing stops SHELL from being a function and an application for example. also if we were to allow <requires> to be specified without type I think type="module" would be the better choice. |
| 15:41.53 | gtjoseph | hmmm. |
| 15:42.05 | coreyfarrell | making the 'type' attribute optional is also problematic for build_tools/get_module_manifest which is just a gawk script, not an xml reader... |
| 15:42.40 | gtjoseph | i've got a network issue i've got to deal with. can we pick this up in about 30 minutes? |
| 15:42.46 | coreyfarrell | everything in the manifests are generated from the MODULEINFO and DOCUMENTATION blocks. |
| 15:42.54 | coreyfarrell | gtjoseph: sure talk to you soon |
| 16:22.54 | *** join/#asterisk-dev rmudgett (rmudgett@nat/digium/x-nysgjervquruuxcm) |
| 16:32.31 | stefan27 | chan_sip.c in receive_message() seems to set the variable SIP_RECVADDR on an ast_msg object which seems to be passed to message.c's msg_route(struct ast_channel *chan, struct ast_msg *msg) which in turn spawns a weird channel c which executes dialplan in the messages context. I want to access the value of SIP_RECVADDR on the spawned channel c through dialplan commands, but I don't seem |
| 16:32.31 | stefan27 | able to with any existing dialplan cmds. What makes most sense to patch to make this happen? What is SIP_RECVADDR used for anyways? |
| 16:41.07 | stefan27 | not so familiar with the pbx architecture so is it safe to do a combo of ast_msg_get_var(msg, "SIP_RECVADDR") and pbx_builtin_setvar_helper(chan,...) in msg_route before ast_pbx_run_args(chan, &pbx_args) is run? (I'm just trying to get a dirty quickfix running without breaking much) |
| 16:41.50 | file | does the MESSAGE_DATA dialplan function not do what you need? |
| 16:43.24 | stefan27 | ill check - thanks I might have overlooked that function |
| 16:43.31 | stefan27 | I looked at MESSAGE |
| 16:44.37 | mjordan | it's MESSAGE_DATA |
| 16:45.44 | mjordan | you can access any of the arbitrary key/value pairs set up via that dialplan function |
| 16:46.57 | stefan27 | If that's true that's really good, patching complicates maintenance |
| 16:47.58 | mjordan | indeed :-) |
| 16:48.03 | mjordan | looks for an example |
| 16:48.51 | mjordan | http://git.asterisk.org/gitweb/?p=asterisk/testsuite.git;a=blob;f=tests/channels/SIP/message_auth_cust_hdr/configs/ast1/extensions.conf;h=013753be7e659df193abacceff86d926e5bbb8c6;hb=HEAD |
| 16:48.58 | mjordan | You can even get SIP headers :-) |
| 16:57.53 | stefan27 | it worked... thanks a lot |
| 17:06.37 | stefan27 | Oh dear, our video+text-enabled asterisk is such a mess. There's so many custom patches and features that are 80% complete |
| 17:13.07 | *** join/#asterisk-dev w9sh (~sph@c-73-54-187-250.hsd1.ga.comcast.net) |
| 17:15.00 | stefan27 | we fetched a bunch of video-aiding patches from the asterisk-video lists but they never made it to trunk so maintenance is nightmare |
| 17:41.08 | gtjoseph | coreyfarrell: back finally. 1 thing i wanted to mention about the xml, it's already parsed from the source files and available in menuselect-tree easy to use any xml tools on it |
| 17:41.45 | mjordan | stefan27: clean em up and contribute them back :-) |
| 17:43.47 | coreyfarrell | gtjoseph: if you have an idea for a better way to implement build build_tools/get_module_manifest I'm open to suggestions |
| 17:44.08 | gtjoseph | i'll look |
| 17:45.30 | coreyfarrell | gtjoseph: be aware the patch modifies many MODULEINFO blocks.. some just add new lines, in other cases the tags were changed |
| 17:45.36 | gtjoseph | right now i'm using xsltproc to convert menuselect-rett to a makefile. no reason we can't do the same to create the manifests. |
| 17:45.47 | gtjoseph | menuselect-tree |
| 17:46.57 | coreyfarrell | be aware get_module_manifest operates on MODULEINFO and DOCUMENTATION sections vs menuselect does not see DOCUMENTATION. |
| 17:47.37 | gtjoseph | why the need for DOCUMENTATION? |
| 17:49.23 | coreyfarrell | for the [provides] section. see https://wiki.asterisk.org/wiki/display/AST/Module+Loader+Ideas#ModuleLoaderIdeas-ExampleManifest that module provides a dialplan function so it's listed in the manifest |
| 17:50.01 | coreyfarrell | technically it's not needed for basic module to module dependencies |
| 17:50.55 | gtjoseph | i would rather see the documentation process pick the MODULEINFO section up than the dependency process pick up DOCUMENTATION |
| 17:51.04 | gtjoseph | rather than |
| 17:51.52 | coreyfarrell | I don't follow |
| 17:52.20 | gtjoseph | things that actually control how asterisk is built or loaded shouldn't be in the DOCUMENTATION section. |
| 17:53.23 | gtjoseph | if we're going to actually use something, it should be in one place...MODULEINFO |
| 17:53.51 | gtjoseph | the documentation generate process should pick up MODULEINFO for inclusion in the docs |
| 17:54.38 | coreyfarrell | well a couple things. 1) menuselect doesn't support one module requiring a dialplan app or function yet. 2) all dependencies must be listed in MODULEINFO. 3) only provides are found in DOCUMENTATION |
| 17:55.24 | gtjoseph | menuselect doesn't have to support it as long as it doesn't barf on it. :) |
| 17:55.55 | gtjoseph | still, if we use MODULEINFO only, then the existing extraction processes work as is |
| 17:57.18 | coreyfarrell | so talking about dialplan functions, the information is already in DOCUMENTATION.. they also have to be listed in the module_load.. I don't think we should require having a 3rd copy of all function names just to make the build process slightly easier. |
| 17:58.25 | gtjoseph | " they also have to be listed in the module_load" ? |
| 17:59.12 | gtjoseph | you mean as part of the register? |
| 18:00.29 | coreyfarrell | yeah so indirectly.. you have to pass the function name through the struct to ast_custom_function.. |
| 18:01.13 | gtjoseph | ok, gotcha. well we can update which ever extraction process to add the info the menuselect-tree so we have it in 1 central xml document |
| 18:03.10 | coreyfarrell | remember we're generating a separate manifest for each module, so I think operating on a central document is not helpful |
| 18:03.36 | gtjoseph | it is to validate the dependencies at build time |
| 18:04.22 | gtjoseph | simple process to xsltproc the single file and produce a manifest for each Mmodule> element (or whatever the tag is) |
| 18:05.30 | coreyfarrell | fair enough. I never did much work on menuselect.c, I actually modified the scripts that generated menuselect-tree so they filtered out the tags I added (global exports and support level for example) |
| 18:05.51 | gtjoseph | menuselect actually only looks for specific elements |
| 18:06.03 | gtjoseph | it ignores anything it doesn't need |
| 18:06.56 | coreyfarrell | I think it warns on things it doesn't need. otherwise i wouldn't have bothered filtering out the new tags |
| 18:07.19 | coreyfarrell | but still that's probably an easy issue |
| 18:07.31 | gtjoseph | hmmm. have to look again but your right, easy issue to fix |
| 18:08.01 | gtjoseph | enough feedback for now? :) |
| 18:11.39 | gtjoseph | so main goal: 1 call to dlopen for any module. if it fails it's because someone didn't set the tags correctly in MODULEINFO, right? |
| 18:12.37 | gtjoseph | no more load order stuff in the AST_MODULE_INFO structure, right? |
| 18:13.39 | gtjoseph | until someone proves otherwise, the module's "load" function is called directly after the dlopen |
| 18:14.32 | gtjoseph | if someone has a reason for load being called in a different order than dlopen, we add a new tag, makbe the systemd-like before and after |
| 18:15.04 | coreyfarrell | everything added to MODULEINFO was removed from AST_MODULE_INFO (support level, exports globals, load priorities) |
| 18:15.38 | gtjoseph | yep, just restating for the chat log and anyone else watching. :) |
| 18:16.19 | coreyfarrell | I'm still trying to get a better handle on how to replace load priorities. |
| 18:16.21 | gtjoseph | all other points ok? |
| 18:16.50 | gtjoseph | what's still troubling? |
| 18:18.05 | gtjoseph | from everything in the tags we discussed don't we have enough to create a linear list? |
| 18:18.06 | coreyfarrell | well the module loader needs to understand that cdr_csv is a CDR module and chan_sip is a channel module, and channel modules should be loaded after CDR modules. but we don't want chan_sip to "require" cdr_csv. |
| 18:18.29 | gtjoseph | yep, which is why I mentioned the provides=CDR, etc |
| 18:19.06 | gtjoseph | CHANNEL, DIALPLAN_FUNCTION, etc. well known targets that get loaded at specific times |
| 18:19.54 | gtjoseph | chan_sip requires CDR,CEL,BRIDGE |
| 18:19.59 | gtjoseph | or whatever |
| 18:20.29 | gtjoseph | or even CHANNEL requires CDR,CEL,BRIDGE |
| 18:20.52 | gtjoseph | built-in dependencies |
| 18:21.33 | coreyfarrell | yeah I was just thinking maybe we want to have a channel.manifest that would require cdr/cel/bridging.. |
| 18:21.45 | gtjoseph | exactly |
| 18:22.19 | gtjoseph | basically we're creating systemd unit files. :) |
| 18:22.58 | gtjoseph | maybe we can use libsystemd to do the work. |
| 18:23.05 | gtjoseph | i didn't say that |
| 18:23.12 | coreyfarrell | troublemaker |
| 18:23.43 | gtjoseph | seriously, we trying to solve the exact same problem |
| 18:24.26 | gtjoseph | actually we could also use libgmake |
| 18:25.51 | gtjoseph | make the manifests make-ish and at runtime concatenate them all and do a 'make all' and let make tell us the order. |
| 18:26.34 | gtjoseph | wait, i know.... we just rewrite asterisk in makefile. yeah, thatg's it. |
| 18:26.52 | gtjoseph | i must be low on blood-sugar |
| 18:30.09 | gtjoseph | coreyfarrell: you went really quiet. Too much? |
| 18:30.42 | mjordan | only suggestion I have is that when y'all have hammered out the approach you're outlining, make sure something is posted to the -dev list before writing a lot of code :-) |
| 18:30.54 | mjordan | this sounds rather sweeping :-) |
| 18:31.06 | file | sweeping, far reaching |
| 18:31.14 | gtjoseph | yep. got a timeline for 14 yet |
| 18:32.09 | file | the future. |
| 18:32.31 | gtjoseph | so no proposed freeze dates or anything |
| 18:34.10 | file | no, but provided stuff is discussed and far enough along... shouldn't be a problem imo |
| 18:34.56 | *** part/#asterisk-dev w9sh (~sph@c-73-54-187-250.hsd1.ga.comcast.net) |
| 18:35.31 | coreyfarrell | sorry had to walk away for a bit. |
| 18:35.52 | gtjoseph | although there aren't many source code changes that can't be automated it'd still be good to have it in place early enough that as new stuff goes into master, it ises the new MODULEINFO |
| 18:36.30 | file | regressions and testing on as many situations and platforms as possible is what I'd want |
| 18:36.37 | file | er, regression testing |
| 18:37.06 | gtjoseph | agreed. |
| 18:37.09 | coreyfarrell | regression testing is what blew up my original review |
| 18:37.45 | gtjoseph | i think if we restrict to just the load process, we'd be pretty good. |
| 18:39.16 | coreyfarrell | the unload process is changed by automatic module to module references. unload is actually where most regressions surfaced |
| 18:39.55 | gtjoseph | how. |
| 18:40.03 | gtjoseph | it's be no worse than it is today |
| 18:42.19 | coreyfarrell | sorry it was a while ago I worked on that part of the code, I just remember having a ton of races during unload. |
| 18:51.40 | gtjoseph | coreyfarrell: i'm off to lunch. think about how you want to proceed and how we can split up the work. |
| 18:54.09 | file | coreyfarrell, does this mean your old RFC is deprecated too? |
| 19:04.10 | *** join/#asterisk-dev tzafrir (~tzafrir@bzq-179-40-172.cust.bezeqint.net) |
| 19:04.10 | *** mode/#asterisk-dev [+o tzafrir] by ChanServ |
| 19:08.11 | *** join/#asterisk-dev newtonr (RustyNewto@nat/digium/x-fvcamftoaatlyxzx) |
| 19:08.11 | *** mode/#asterisk-dev [+o newtonr] by ChanServ |
| 19:30.52 | coreyfarrell | gtjoseph: I think we should start by updating the wiki page or making a new one. |
| 19:38.20 | dan_j | This has a link labelled 'CDR Consolidation' but the link doesnt work. Please can someone confirm what CDR Consolidation does and if it can be disabled? |
| 19:38.21 | dan_j | https://wiki.asterisk.org/wiki/display/AST/Asterisk+12+CDR+Specification |
| 19:45.16 | mjordan | er |
| 19:45.24 | rmudgett | IIRC mjordan gave up on CDR consolidation as being too complex to get right |
| 19:45.34 | mjordan | It is, although the language there is confusing. |
| 19:46.01 | mjordan | Really, what it is referring to is that when one bridge is merged into another bridge, the CDRs in the bridge that goes away are going to be finalized, and new CDRs generated for all of the newly created channel pairings |
| 19:46.01 | file | mjordan, blame CDRs and not yourself |
| 19:46.13 | dan_j | Just trying to figure out why, at the end of the call, a final CDR is created with the same channel name as the first channel that started the call. |
| 19:46.14 | mjordan | That's a SUPER rare occurrance. |
| 19:46.33 | mjordan | what is your endbeforehexten, and are you using the 'g' option in Dial? |
| 19:47.18 | dan_j | I've not set endbeforehexten to anything, and I'm not using g. Just read through the cdr spec sheet for v12 |
| 19:48.42 | dan_j | Sorry, just checked my config |
| 19:48.42 | dan_j | endbeforehexten=yes |
| 19:49.21 | mjordan | dan_j: What version of 13? I just fixed a bug related to this in the latest release |
| 19:49.36 | mjordan | or rather, fixed the behavior to be a little bit friendlier. |
| 19:49.46 | dan_j | It's a recent clone |
| 19:49.54 | mjordan | looks |
| 19:50.21 | mjordan | commit 1087b0c6eda6a2372f923a6cac6dccc865fd438a is where I tweaked it a bit |
| 19:50.37 | mjordan | that being said, you can think of what happens like this |
| 19:50.48 | mjordan | when you have a single channel executing dialplan, that gets a CDR - there's a call in Asterisk. |
| 19:51.10 | mjordan | If it dials someone, so long as they both go *directly* into a bridge, that results in one CDR. That is, we have a Party A, and a Party B, and life is good. |
| 19:51.26 | mjordan | Note that this doesn't care what *type* of channels you're using. So if you're using Local channels, you'll have two CDRs generally |
| 19:51.44 | mjordan | Now, if when people hang up, you generally should just have the one CDR. |
| 19:51.55 | mjordan | However, if either channel starts executing dialplan again, that gets a new CDR. |
| 19:52.12 | mjordan | We can't "re-use" the existing one, as the act of tearing down the bridge between the Party A and Party B finalizes that record |
| 19:52.57 | mjordan | And it *HAS* to finalize that record. There's too much that can happen in Asterisk dialplan to account for both situations, that is, where you want a bridge being finished to signify that the parties are done talking, and where you want to let someone to keep modifying that record. |
| 19:53.33 | mjordan | The reason why it used to get away with this shenaniganry was due to all the ridiculous exceptions put into the features/channel bridging code in 11 and previous versions, which was an unmaintainable mess that had a lot of subtle bugs that resulted in people losing billing records. |
| 19:54.36 | dan_j | I've got a single channel, which dials a Local channel, so one CDR. Then the Local channel dials a 2nd local channel, so thats two CDR records. Thats fine. The 2nd dials a 3rd and now theres 3 CDR records. Then the call ends and for some reason a 4th record is added. Interestingly, the 4th record has the first channel for PartyA and the final 4th channel |
| 19:54.36 | dan_j | for party B. As if it's consolidated everything and created a CDR entry telling me what the final conversation ended up looking like. |
| 19:55.12 | mjordan | I'm going to *guess* that's the bridge merging that happens, if you have local channel optimization on |
| 19:55.27 | mjordan | you can enable 'cdr set debug on', and it will dump a TON of information about what is happening during the CDR process |
| 19:56.14 | mjordan | if you aren't optimizing local channels, then that would feel like a bug. But when you optimize the local channels, you're allowing Asterisk to collapse the information down to a single pairing of channels, which, from the perspective of the CDR engine, ends up being those two channels. |
| 19:57.12 | dan_j | Ok, but when optimizing, the CDR from the original unoptimized channels still get stored? |
| 19:58.36 | dan_j | How can I search commits to see if/when they've been merged so I can work out if they've already been in an asterisk release? |
| 20:04.47 | coreyfarrell | dan_j: if you have the commit id you can search in gerrit, ex: https://gerrit.asterisk.org/#/q/commit:1087b0c6eda6a2372f923a6cac6dccc865fd438a |
| 20:05.29 | dan_j | Thanks |
| 20:05.32 | coreyfarrell | that's for commits done since git migration anyways. |
| 20:32.58 | dan_j | mjordan: thanks for the help. just pulled the latest code and checked i've got /n on all local channels, now i'm getting more usable CDR records. |
| 21:03.00 | *** join/#asterisk-dev newtonr (RustyNewto@nat/digium/x-anbblvozauuedvae) |
| 21:03.00 | *** mode/#asterisk-dev [+o newtonr] by ChanServ |
| 21:14.46 | *** join/#asterisk-dev newtonr (RustyNewto@nat/digium/x-eigzfoskjllleltc) |
| 21:14.46 | *** mode/#asterisk-dev [+o newtonr] by ChanServ |
| 21:21.28 | coreyfarrell | file: I'm confused, in the artifacts for the ref_debug the tests I checked so far still show the res_odbc leak. But the console log says that res_odbc.so was not built. |
| 21:22.13 | file | that's... odd |
| 21:24.02 | *** join/#asterisk-dev mjordan (mjordan@nat/digium/x-malvlqqkohxdgqzs) |
| 21:24.02 | *** mode/#asterisk-dev [+o mjordan] by ChanServ |
| 21:29.58 | coreyfarrell | hmm just noticed in the console log --- rm: cannot remove `/tmp/asterisk-testsuite': Operation not permitted |
| 21:30.25 | coreyfarrell | also a few lines later -- warning: failed to remove 'logs/' |
| 21:34.23 | file | yeah, that's something to be fixed but it shouldn't really be a problem besides nonsense in the output |
| 21:35.01 | coreyfarrell | awesome the artifact download failed (it's 3.6gb) |
| 21:35.17 | file | oh great |
| 21:38.32 | gtjoseph | anyone want a laugh?? I did a make distclean followed by a sudo make uninstall and got this... https://pastebin.com/gEMAy7Ef |
| 21:39.05 | coreyfarrell | ah, I just looked in one of the Asterisk log files from the artifact.. it says Jan 23 12:07 but test 7 was yesterday at 2pm |
| 21:42.44 | coreyfarrell | gtjoseph: it deleted your symlinks to usr dirs |
| 21:42.55 | gtjoseph | anyone know how to recreate the /usr/lib64->/lib64 symlink using only bash?? |
| 21:43.02 | gtjoseph | yeah, no shit |
| 21:44.24 | gtjoseph | oh well have to reboot i guess. |
| 21:59.57 | *** join/#asterisk-dev gtjoseph (~gtj0@unaffiliated/gtj) |
| 22:00.21 | gtjoseph | well, i won't be doing THAT again. :) |
| 22:09.44 | gtjoseph | Oh boy! Monster savings on Digium Phones!! |
| 22:18.04 | wdoekes | gtjoseph: busybox in /bin/, if you have it |
| 22:18.19 | wdoekes | a bit late now ;) |
| 22:18.24 | gtjoseph | dracut shell worked but that's a goot idea. |
| 22:18.47 | gtjoseph | i have it installed but didn't think about it |
| 22:29.33 | *** join/#asterisk-dev [TK]D-Fender (~joe@64.235.216.2) |
| 22:48.55 | gtjoseph | so this is weird... i can't delete a WIP review. The page you requested was not found, or you do not have permission to view this page. |
| 22:49.56 | *** join/#asterisk-dev snuff-work (~blatus@203.87.61.22) |
| 22:49.56 | *** mode/#asterisk-dev [+o snuff-work] by ChanServ |