Re: [Minios-devel] [LCMS PATCH 1/1] [LCMS PATCH] Add new library lcms 2.9 to Unikraft's project.

[Costin Lupu <costin.lup@xxxxxxxxx>]

Hi Esteban,

First of all, Makefile.uk should come in a different patch. For the
first patch we introduce only the library skeleton. Also, please add
here a .gitignore file, you can take the one from pcre [1].

For the rest of the comments please see inline.

[1] https://github.com/unikraft/lib-pcre




On 12/2/19 1:45 PM, Esteban wrote:
> Signed-off-by: Esteban <esteban.martinez@xxxxxxxx>

Please use your complete name in signatures, e.g.:

Signed-off-by: Esteban Martinez <esteban.martinez@xxxxxxxx>

> ---
>  CODING_STYLE.md |   4 ++
>  CONTRIBUTING.md |   4 ++
>  COPYING.md      |  41 +++++++++++++++
>  Config.uk       |  11 +++++
>  MAINTAINERS.md  |  11 +++++
>  Makefile.uk     | 129 ++++++++++++++++++++++++++++++++++++++++++++++++
>  README.md       |   7 +++
>  exportsyms.uk   |   1 +
>  8 files changed, 208 insertions(+)
>  create mode 100755 CODING_STYLE.md
>  create mode 100755 CONTRIBUTING.md
>  create mode 100755 COPYING.md
>  create mode 100755 Config.uk
>  create mode 100755 MAINTAINERS.md
>  create mode 100755 Makefile.uk
>  create mode 100755 README.md
>  create mode 100755 exportsyms.uk

The permissions on the files should be 644, meaning that the shouldn't
be executable.

Also, please get rid of exportsyms.uk, we don't use that one anymore.

> 
> diff --git a/CODING_STYLE.md b/CODING_STYLE.md
> new file mode 100755
> index 0000000..5730041
> --- /dev/null
> +++ b/CODING_STYLE.md
> @@ -0,0 +1,4 @@
> +Coding Style
> +============
> +
> +Please refer to the `CODING_STYLE.md` file in the main Unikraft repository.
> diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
> new file mode 100755
> index 0000000..14f6ac6
> --- /dev/null
> +++ b/CONTRIBUTING.md
> @@ -0,0 +1,4 @@
> +Contributing to Unikraft
> +========================
> +
> +Please refer to the `CONTRIBUTING.md` file in the main Unikraft repository.
> diff --git a/COPYING.md b/COPYING.md
> new file mode 100755
> index 0000000..fa05041
> --- /dev/null
> +++ b/COPYING.md
> @@ -0,0 +1,41 @@
> +License
> +=======
> +
> +Unikraft lcms wrappers
> +------------------------
> +
> +This repository contains wrapper code to build lcms with Unikraft.  The code
> +is published as a mixture of BSD and MIT licences; each C code file in
> +this repository should declare who is the copyright owner and under which 
> terms
> +and conditions the code is licensed. If such a licence note is missing, the
> +following copyright notice will apply:
> +
> +     Copyright (c) 2017, NEC Europe Ltd., NEC Corporation. All rights 
> reserved.

Here you should put a placeholder, e.g.:

Copyright (c) Year, Institution. 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.
> +
> +lcms source code
> +------------------

Please remove these 2 lines above.

> \ No newline at end of file
> diff --git a/Config.uk b/Config.uk
> new file mode 100755
> index 0000000..edb3659
> --- /dev/null
> +++ b/Config.uk
> @@ -0,0 +1,11 @@
> +config LIBLCMS
> +     bool "liblcms - Color management engine"
> +     default n
> +     select LIBNOLIBC if !HAVE_LIBC
> +     select LIBUKDEBUG
> +    select LIBUKALLOC
> +    select LIBPTHREAD_EMBEDDED
> +    select LIBNEWLIBC

It would be nice for the 3 selects above to have the same indentation as
the lines before them.

> +
> +if LIBLCMS
> +endif
> diff --git a/MAINTAINERS.md b/MAINTAINERS.md
> new file mode 100755
> index 0000000..227c4dd
> --- /dev/null
> +++ b/MAINTAINERS.md
> @@ -0,0 +1,11 @@
> +Maintainers List
> +================
> +
> +For notes on how to read this information, please refer to `MAINTAINERS.md` 
> in
> +the main Unikraft repository.
> +
> +     LCMS-UNIKRAFT
> +     M:      Costin Lupu <costin.lupu@xxxxxxxxx>

The first maintainer here should be you, Esteban. So please remove me
from this list. And for the second maintainer either you can add Xavier
or you can keep Felipe.

> +     M:      Felipe Huici <felipe.huici@xxxxxxxxx>
> +     L:      minios-devel@xxxxxxxxxxxxx
> +     F: *
> diff --git a/Makefile.uk b/Makefile.uk
> new file mode 100755
> index 0000000..86f941b
> --- /dev/null
> +++ b/Makefile.uk
> @@ -0,0 +1,129 @@
> +#  SPDX-License-Identifier: BSD-3-Clause
> +#
> +#  lcms Makefile.uk
> +#
> +#  Authors: Costin Lupu <costin.lupu@xxxxxxxxx>

For sure the author should be only you here.

> +#
> +#  Copyright (c) 2019, University Politehnica of Bucharest. All rights 
> reserved.

Please replace the institution with yours.

> +#
> +#  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,liblcms,$(CONFIG_LIBLCMS)))
> +#$(eval $(call addlib_s,liblcmsglue,$(CONFIG_LIBLCMS)))

If you don't use liblcmsglue then we can remove this commented line. It
was used in the pcre port and walkthrough only as an example.

> +
> +################################################################################
> +# Original sources
> +################################################################################
> +LIBLCMS_VERSION=2.9
> +LIBLCMS_URL=https://raw.githubusercontent.com/python-pillow/pillow-depends/master/lcms2-$(LIBLCMS_VERSION).tar.gz
> +LIBLCMS_PATCHDIR=$(LIBLCMS_BASE)/patches
> +LIBLCMS_SUBDIR=lcms2-$(LIBLCMS_VERSION)
> +$(eval $(call fetch,liblcms,$(LIBLCMS_URL)))
> +$(eval $(call patch,liblcms,$(LIBLCMS_PATCHDIR),$(LIBLCMS_SUBDIR)))
> +
> +################################################################################
> +# Helpers
> +################################################################################
> +LIBLCMS_EXTRACTED = $(LIBLCMS_ORIGIN)/lcms2-$(LIBLCMS_VERSION)

I think you can set it to $(LIBLCMS_ORIGIN)/$(LIBLCMS_SUBDIR)

> +
> +################################################################################
> +# Library includes
> +################################################################################
> +LIBLCMS_COMMON_INCLUDES-y      += -I$(LIBLCMS_EXTRACTED)/include
> +LIBLCMS_COMMON_INCLUDES-y      += -I$(LIBLCMS_EXTRACTED)/src
> +
> +CINCLUDES-$(CONFIG_LIBLCMS)    += $(LIBLCMS_COMMON_INCLUDES-y)
> +
> +LIBLCMS_CINCLUDES   += -I$(LIBLCMS_EXTRACTED)
> +
> +################################################################################
> +# Global flags
> +################################################################################
> +# Suppressed flags

s/Suppressed/Suppress

> +LIBLCMS_SUPPRESS_FLAGS += -Wno-unused-parameter \
> +     -Wno-unused-variable -Wno-unused-value -Wno-unused-function \
> +     -Wno-missing-field-initializers -Wno-implicit-fallthrough
> +LIBLCMS_CFLAGS-y   += $(LIBLCMS_SUPPRESS_FLAGS) \
> +     -Wno-pointer-to-int-cast -Wno-int-to-pointer-cast
> +

When building you still get some warnings like -Wmisleading-indentation
or -Wstringop-truncation. You can supress them with
-Wno-misleading-indentation and -Wno-stringop-truncation. You can do the
same for the other warnings as well.

> +
> +
> +################################################################################
> +# OS dependencies code - Glue between Unikraft and lcms
> +################################################################################
> +#LIBLCMSGLUE_SRCS-y += $(LIBLCMS_BASE)/sample.c|glue

If you don't use liblcmsglue then we can remove it. It was used in the
pcre port and walkthrough only as an example.

> +
> +################################################################################
> +# LCMS src
> +################################################################################
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmscnvrt.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmserr.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmsgamma.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmsgmt.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmsintrp.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmsio0.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmsio1.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmslut.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmsplugin.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmssm.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmsmd5.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmsmtrx.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmspack.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmswtpnt.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmsxform.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmssamp.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmsnamed.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmscam02.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmsvirt.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmstypes.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmscgats.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmsps2.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmsopt.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmshalf.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmsalpha.c
> +LIBLCMS_SRCS-y += $(LIBLCMS_EXTRACTED)/src/cmspcs.c
> +
> +
> +################################################################################
> +# LCMS prepare
> +################################################################################
> +# Run ./configure
> +$(LIBLCMS_EXTRACTED)/config.status: $(LIBLCMS_BUILD)/.origin
> +     $(call verbose_cmd,CONFIG,liblcms: $(notdir $@), \
> +             cd $(LIBLCMS_EXTRACTED) && ./configure)
> +
> +LIBLCMS_PREPARED_DEPS = \
> +     $(LIBLCMS_EXTRACTED)/config.status \
> +
> +$(LIBLCMS_BUILD)/.prepared: $(LIBLCMS_PREPARED_DEPS)
> +
> +UK_PREPARE += $(LIBLCMS_BUILD)/.prepared

In this case we don't need to run ./configure on the origin code so you
can remove anything from `LCMS prepare` to the end of the file.

> \ No newline at end of file
> diff --git a/README.md b/README.md
> new file mode 100755
> index 0000000..41f3ea5
> --- /dev/null
> +++ b/README.md
> @@ -0,0 +1,7 @@
> +lcms for Unikraft
> +=============================
> +
> +This is the port of lcms for Unikraft as external library.
> +
> +Please refer to the `README.md` as well as the documentation in the `doc/`
> +subdirectory of the main unikraft repository.
> diff --git a/exportsyms.uk b/exportsyms.uk
> new file mode 100755
> index 0000000..c86c3f3
> --- /dev/null
> +++ b/exportsyms.uk
> @@ -0,0 +1 @@
> +none
> \ No newline at end of file
> 

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