Re: [Minios-devel] [ UNIKRAFT/HTTP_PARSER PATCH v2 2/3] Add Makefile.uk

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

On 05.09.19 15:12, Santiago Pagani wrote:
> Signed-off-by: Felipe Huici <felipe.huici@xxxxxxxxx>
> Signed-off-by: Santiago Pagani <santiago.pagani@xxxxxxxxx>
> ---
>   Makefile.uk | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 62 insertions(+)
>   create mode 100644 Makefile.uk
>
> diff --git a/Makefile.uk b/Makefile.uk
> new file mode 100644
> index 0000000..0fc731f
> --- /dev/null
> +++ b/Makefile.uk
> @@ -0,0 +1,62 @@
> +#  http-parser Makefile.uc
> +#
> +#  Authors: Felipe Huici <felipe.huici@xxxxxxxxx>
> +#
> +#
> +#  Copyright (c) 2019, NEC Europe Ltd., NEC Corporation. All rights reserved.
> +#
> +#  Redistribution and use in source and binary forms, with or without
> +#  modification, are permitted provided that the following conditions
> +#  are met:
> +#
> +#  1. Redistributions of source code must retain the above copyright
> +#     notice, this list of conditions and the following disclaimer.
> +#  2. Redistributions in binary form must reproduce the above copyright
> +#     notice, this list of conditions and the following disclaimer in the
> +#     documentation and/or other materials provided with the distribution.
> +#  3. Neither the name of the copyright holder nor the names of its
> +#     contributors may be used to endorse or promote products derived from
> +#     this software without specific prior written permission.
> +#
> +#  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> +#  AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> +#  IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> +#  ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
> +#  LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> +#  CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> +#  SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> +#  INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> +#  CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> +#  ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> +#  POSSIBILITY OF SUCH DAMAGE.
> +#
> +#  THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
> +#
> +
> +################################################################################
> +# Library registration
> +################################################################################
> +$(eval $(call addlib_s,libhttp_parser,$(CONFIG_LIBHTTP_PARSER)))
> +
> +################################################################################
> +# Sources
> +################################################################################
A minor thing. If you would do the version without prefixing "v"
LIBHTTP_PARSER_VERSION=2.8.1
you can avoid needing the version number again in _SUBDIR:

LIBHTTP_PARSER_URL=https://github.com/nodejs/http-parser/archive/v$(LIBHTTP_PARSER_VERSION).zip
LIBHTTP_PARSER_SUBDIR=http-parser-$(LIBHTTP_PARSER_VERSION)

> +LIBHTTP_PARSER_VERSION=v2.8.1
> +LIBHTTP_PARSER_URL=https://github.com/nodejs/http-parser/archive/$(LIBHTTP_PARSER_VERSION).zip
> +LIBHTTP_PARSER_SUBDIR=http-parser-2.8.1
> +$(eval $(call 
> fetch,libhttp_parser,$(LIBHTTP_PARSER_URL),$(LIBHTTP_PARSER_VERSION).zip))
> +
> +################################################################################
> +# Helpers
> +################################################################################
> +LIBHTTP_PARSER_EXTRACTED=$(LIBHTTP_PARSER_ORIGIN)/$(LIBHTTP_PARSER_SUBDIR)
> +
> +################################################################################
> +# Library includes
> +################################################################################
> +CINCLUDES-$(CONFIG_LIBHTTP_PARSER) += -I$(LIBHTTP_PARSER_EXTRACTED)
Since this is such a simple library, I would prefer to show case with it 
how to prepare an include folder that just have a link to the actual 
header files (instead of the CINCLUDE line that you have):
$(call·mk_sub_build_dir,libhttp_parser/include)
CINCLUDES-$(CONFIG_LIBHTTP_PARSER) += -I$(LIBHTTP_PARSER_BUILD)/include

...please also provide the library headers to C++ sources since the 
library supports it:
CINCLUDES-$(CONFIG_LIBHTTP_PARSER) += -I$(LIBHTTP_PARSER_BUILD)/include

> +
> +################################################################################
> +# Sources
> +################################################################################
> +LIBHTTP_PARSER_SRCS-y += $(LIBHTTP_PARSER_EXTRACTED)/http_parser.c
...then add a rule for linking:

$(LIBHTTP_PARSER_BUILD)/include/%.h:·$(LIBHTTP_PARSER_EXTRACTED)/%.h
»       $(call·build_cmd,LN,libhttp_parser,$@,\
»       ln·-sf·»ln -sf $< $@)lt;·$@)

...and register it to the prepare target:

UK_PREPARE·+=·$(LIBUUID_BUILD)/include/http_parser.h

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel