[Buildroot] [PATCHv3] package/pugixml: add support for a limited set of configuration options
Arnout Vandecappelle
arnout at mind.be
Sun Feb 3 07:46:49 UTC 2019
Hi Thomas, Wouter,
I'm sorry, I only did a really quick review yesterday. There's a lot of stuff
here that gives me some doubts...
On 02/02/2019 21:25, Thomas De Schampheleire wrote:
> From: Wouter Vermeiren <wouter.vermeiren at nokia.com>
>
> Signed-off-by: Wouter Vermeiren <wouter.vermeiren at nokia.com>
> [ThomasDS:
> - align with Buildroot coding style
> - rename some options
> - always enable 'long long' type
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire at nokia.com>
> ---
> package/pugixml/Config.in | 62 ++++++++++++++++++++++++++++++++++++++
> package/pugixml/pugixml.mk | 25 +++++++++++++++
> 2 files changed, 87 insertions(+)
>
> v3:
> - always enable 'long long' type (feedback Arnout Vandecappelle)
> - remove unnecessary 'strip' on PUGIXML_BUILD_DEFINES
> v2: (feedback Thomas Petazzoni)
> - change 'disable' options into 'enable' and reorder them to be first
> - rename 'has-long-long' option
>
> diff --git a/package/pugixml/Config.in b/package/pugixml/Config.in
> index c37b1df9e5..421f19f0f2 100644
> --- a/package/pugixml/Config.in
> +++ b/package/pugixml/Config.in
> @@ -18,5 +18,67 @@ config BR2_PACKAGE_PUGIXML
> http://pugixml.org/
> https://github.com/zeux/pugixml
>
> +if BR2_PACKAGE_PUGIXML
> +
> +config BR2_PACKAGE_PUGIXML_XPATH_SUPPORT
> + bool "Enable XPath support"
> + default y
> + help
> + Enables XPath support in pugixml.
> + When disabled, both XPath interfaces and XPath implementation
> + are excluded from compilation. This option is provided in case
> + you do not need XPath functionality and need to save code
> + space.
Does it make a significant difference? Like, more than 10% of the code size of
pugixml?
If you added this option, I guess you have a reason for it, so you probably can
quote some number?
> +
> +config BR2_PACKAGE_PUGIXML_STL_SUPPORT
> + bool "Enable STL support"
> + default y
> + help
> + Enables use of STL in pugixml.
> + When disabled, the functions that operate on STL types are no
> + longer present (i.e. load/save via iostream). This option is
> + provided in case your target platform does not have a
> + standard-compliant STL implementation.
If "STL" means C++98 STL, all the compilers that we support have it. So it
doesn't make sense to make it optional. But again, I guess you had a reason for it?
> +
> +config BR2_PACKAGE_PUGIXML_EXCEPTIONS
> + bool "Enable exceptions"
> + default y
> + help
> + Enables use of exceptions in pugixml. This option is provided
> + in case your target platform does not have exception handling
> + capabilities.
Same same same. For exceptions, there may in fact be a performance impact
(though for your typical C++ code base it would be negligible).
> +
> +config BR2_PACKAGE_PUGIXML_WCHAR_MODE
> + bool "Enable wchar_t mode"
> + help
> + Toggles between UTF-8 (default) style interface (the in-memory
> + text encoding is assumed to be UTF-8, most functions use char
> + as character type) and UTF-16/32 style interface (the
> + in-memory text encoding is assumed to be UTF-16/32, depending
> + on wchar_t size, most functions use wchar_t as character type)
> +
> + http://pugixml.org/docs/manual.html#dom.unicode
This option makes complete sense. However, I have the feeling it might need to
depend on BR2_USE_WCHAR...
This time I reviewed the patch completely :-)
Regards,
Arnout
> +
> +config BR2_PACKAGE_PUGIXML_COMPACT
> + bool "Enable compact mode"
> + help
> + Activates a different internal representation of document
> + storage that is much more memory efficient for documents with
> + a lot of markup (i.e. nodes and attributes), but is slightly
> + slower to parse and access.
> +
> + http://pugixml.org/docs/manual.html#dom.memory.compact
> +
> +config BR2_PACKAGE_PUGIXML_HEADER_ONLY
> + bool "Enable header-only version"
> + help
> + All source code for pugixml will be included in every
> + translation unit that includes pugixml.hpp. This is how most
> + of Boost and STL libraries work.
> +
> + http://pugixml.org/docs/manual.html#install.building.header
> +
> +endif
> +
> comment "pugixml needs a toolchain w/ C++"
> depends on !BR2_INSTALL_LIBSTDCPP
> diff --git a/package/pugixml/pugixml.mk b/package/pugixml/pugixml.mk
> index e5188e5f53..faeec4368a 100644
> --- a/package/pugixml/pugixml.mk
> +++ b/package/pugixml/pugixml.mk
> @@ -10,4 +10,29 @@ PUGIXML_LICENSE = MIT
> PUGIXML_LICENSE_FILES = readme.txt
> PUGIXML_INSTALL_STAGING = YES
>
> +# gcc always supports 'long long' type, see
> +# https://gcc.gnu.org/onlinedocs/gcc/Long-Long.html
> +PUGIXML_BUILD_DEFINES += PUGIXML_HAS_LONG_LONG
> +
> +ifeq ($(BR2_PACKAGE_PUGIXML_XPATH_SUPPORT),)
> +PUGIXML_BUILD_DEFINES += PUGIXML_NO_XPATH
> +endif
> +ifeq ($(BR2_PACKAGE_PUGIXML_STL_SUPPORT),)
> +PUGIXML_BUILD_DEFINES += PUGIXML_NO_STL
> +endif
> +ifeq ($(BR2_PACKAGE_PUGIXML_EXCEPTIONS),)
> +PUGIXML_BUILD_DEFINES += PUGIXML_NO_EXCEPTIONS
> +endif
> +ifeq ($(BR2_PACKAGE_PUGIXML_WCHAR_MODE),y)
> +PUGIXML_BUILD_DEFINES += PUGIXML_WCHAR_MODE
> +endif
> +ifeq ($(BR2_PACKAGE_PUGIXML_COMPACT),y)
> +PUGIXML_BUILD_DEFINES += PUGIXML_COMPACT
> +endif
> +ifeq ($(BR2_PACKAGE_PUGIXML_HEADER_ONLY),y)
> +PUGIXML_BUILD_DEFINES += PUGIXML_HEADER_ONLY
> +endif
> +
> +PUGIXML_CONF_OPTS += -DBUILD_DEFINES="$(subst $(space),;,$(PUGIXML_BUILD_DEFINES))"
> +
> $(eval $(cmake-package))
>
More information about the buildroot
mailing list