Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add kpcre group #901

Closed
wants to merge 1 commit into from
Closed

Add kpcre group #901

wants to merge 1 commit into from

Conversation

ffontaine
Copy link
Contributor

Previously pcre was included into kstandard but pcre module depends on
pcre library so create its own group and remove it from kstandard as
standard modules should have no internal/external compile or link
dependencies

Signed-off-by: Fabrice Fontaine fabrice.fontaine@orange.com

Previously pcre was included into kstandard but pcre module depends on
pcre library so create its own group and remove it from kstandard as
standard modules should have no internal/external compile or link
dependencies

Signed-off-by: Fabrice Fontaine <fabrice.fontaine@orange.com>
@miconda
Copy link
Member

miconda commented Jan 2, 2017

I think this was added because of lcr module to be included in the main package (e.g., deb) and pcre being very common.

If many people want to change this and have a dedicated group, then comment here.

@miconda
Copy link
Member

miconda commented Jan 10, 2017

Maybe @linuxmaniac can have an additional opinion on this topic. There is a conflict already due to other changes, so cannot be just merged any longer, but the question of this PR is whether to to create or not a pcre group.

@linuxmaniac
Copy link
Member

I do believe pcre is quite common and we can live with it on kstandard

@ffontaine
Copy link
Contributor Author

My goal was to integrate kamailio into buildroot (https://buildroot.org/). buildroot allows the user to create its own embedded linux system and the pcre package is not always selected by the user.

@miconda
Copy link
Member

miconda commented Jan 13, 2017

@ffontaine - is the buildroot using deb or rpm specs from kamailio tree?

There is another set of groups, which doesn't have 'k' as prefix of the last word in the name -- for example, there is module_group_standard and module_group_kstandard. The module_group_k... were intended for packaging, the others should be for default compilation.

An alternative is to add some switch on a env variable and then set the values of those groups based on it, so when using it for buildroot, will skip pcre modules in standard and have own group.

@ffontaine
Copy link
Contributor Author

buildroot does not use deb or rpm specs from kamailio tree.
To integrate a package to buildroot, I have to define a .mk that explain how to download the source from github and selects the appropriate modules depending on what packages were selected by the user.
For example, if the package libmemcached is selected then this package must be built before kamailio and kmemcached module can be compiled:

ifeq ($(BR2_PACKAGE_LIBMEMCACHED),y)
KAMAILIO_DEPENDENCIES += libmemcached
KAMAILIO_GROUPS += kmemcached
endif

Indeed, you're right, I could use the standard group instead of kstandard. That was my first intent but I realised that it didn't give me enough granularity. For example, in the standard module, kamailio compiles the mod_list_devel which are not necessary as one of the goal of buildroot is to make the most tiny and efficient distribution.

But I perfectly understand that you do not want to modify the behavior of the kstandard target as it could impact other users. May I suggest to keep kstandard as it is and define kpcre and kmini targets like this (I think this option is more readable than adding an env variable but this is only my personal opinion)?

module_group_kmini=$(mod_list_basic) $(mod_list_extra) \
					  $(mod_list_db) $(mod_list_dbuid)

module_group_kpcre=$(mod_list_pcre)

@miconda
Copy link
Member

miconda commented Jan 13, 2017

Yes, it's ok to add new groups that can help in what you need.

@ffontaine
Copy link
Contributor Author

OK thanks, I will close this PR and create 2 PRs: one for kmini and another for kpcre

@ffontaine ffontaine closed this Jan 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants