Re: [PATCH v5 0/6] jansson edk2 port


Abner Chang
 

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
Michael D Kinney
Sent: Saturday, December 12, 2020 3:23 AM
To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist)
<abner.chang@...>; gaoliming@...; Kinney, Michael D
<michael.d.kinney@...>
Cc: 'Sean Brogan' <sean.brogan@...>; 'Bret Barkelew'
<Bret.Barkelew@...>; 'Andrew Fish' <afish@...>; 'Laszlo
Ersek' <lersek@...>; 'Leif Lindholm' <leif@...>; Wang,
Nickle (HPS SW) <nickle.wang@...>; O'Hanley, Peter (EXL)
<peter.ohanley@...>
Subject: Re: [edk2-devel] [PATCH v5 0/6] jansson edk2 port

Abner,

Feedback below.

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Abner
Chang
Sent: Wednesday, December 9, 2020 8:02 PM
To: Kinney, Michael D <michael.d.kinney@...>;
devel@edk2.groups.io; gaoliming@...
Cc: 'Sean Brogan' <sean.brogan@...>; 'Bret Barkelew'
<Bret.Barkelew@...>; 'Andrew Fish'
<afish@...>; 'Laszlo Ersek' <lersek@...>; 'Leif Lindholm'
<leif@...>; Wang, Nickle (HPS SW) <nickle.wang@...>;
O'Hanley, Peter (EXL) <peter.ohanley@...>
Subject: Re: [edk2-devel] [PATCH v5 0/6] jansson edk2 port



-----Original Message-----
From: Kinney, Michael D [mailto:michael.d.kinney@...]
Sent: Thursday, December 10, 2020 10:33 AM
To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@...>;
devel@edk2.groups.io; gaoliming@...; Kinney, Michael D
<michael.d.kinney@...>
Cc: 'Sean Brogan' <sean.brogan@...>; 'Bret Barkelew'
<Bret.Barkelew@...>; 'Andrew Fish' <afish@...>;
'Laszlo Ersek' <lersek@...>; 'Leif Lindholm'
<leif@...>; Wang, Nickle (HPS SW) <nickle.wang@...>;
O'Hanley, Peter (EXL) <peter.ohanley@...>
Subject: RE: [edk2-devel] [PATCH v5 0/6] jansson edk2 port

Abner,

Some questions included below.

Mike


-----Original Message-----
From: Chang, Abner (HPS SW/FW Technologist)
<abner.chang@...>
Sent: Wednesday, December 9, 2020 6:14 PM
To: devel@edk2.groups.io; gaoliming@...
Cc: 'Sean Brogan' <sean.brogan@...>; 'Bret Barkelew'
<Bret.Barkelew@...>; 'Andrew Fish'
<afish@...>; 'Laszlo Ersek' <lersek@...>; 'Leif
Lindholm'
<leif@...>; Kinney, Michael D
<michael.d.kinney@...>; Wang, Nickle (HPS SW)
<nickle.wang@...>; O'Hanley, Peter (EXL)
<peter.ohanley@...>
Subject: RE: [edk2-devel] [PATCH v5 0/6] jansson edk2 port



-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On
Behalf Of gaoliming
Sent: Tuesday, December 8, 2020 2:40 PM
To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist)
<abner.chang@...>
Cc: 'Sean Brogan' <sean.brogan@...>; 'Bret Barkelew'
<Bret.Barkelew@...>; 'Andrew Fish' <afish@...>;
'Laszlo Ersek' <lersek@...>; 'Leif Lindholm'
<leif@...>; 'Michael D Kinney'
<michael.d.kinney@...>; Wang, Nickle (HPS SW)
<nickle.wang@...>; O'Hanley, Peter (EXL)
<peter.ohanley@...>
Subject: 回复: [edk2-devel] [PATCH v5 0/6] jansson edk2 port

Abner:
I have minor comments on the library header file.

1. JasonLib.h & BaseUcs2Utf8Lib.h. They don't need to include
the additional header files, such as Uefi.h and BaseLib.h,
because the library header file doesn't depend on the definitions
from BaseLib.
2. CrtLib.inf needs to list the required library class:
BaseMemoryLib &
PrintLib.

OK, I will wait couple days for other comments and address that
all
together.

Thanks

Thanks
Liming
-----邮件原件-----
发件人: bounce+27952+68426+4905953+8761045@groups.io
<bounce+27952+68426+4905953+8761045@groups.io> 代表 Abner
Chang
发送时间: 2020年12月8日 10:11
收件人: devel@edk2.groups.io
抄送: Sean Brogan <sean.brogan@...>; Bret Barkelew
<Bret.Barkelew@...>; Andrew Fish <afish@...>;
Laszlo Ersek <lersek@...>; Leif Lindholm
<leif@...>; Michael D Kinney
<michael.d.kinney@...>; Liming Gao
<gaoliming@...>; Nickle Wang
<nickle.wang@...>;
Peter
O'Hanley <peter.ohanley@...>
主题: [edk2-devel] [PATCH v5 0/6] jansson edk2 port

In v5, move BaseUcs2Utf8Lib to under RedfishPkg.
In v4,
- Address review comments
- Seperate CRT functions to a individule library CrtLib under
RedfishPkg.
- Seperate UCS2-UTF8 functions to a individule library
BaseUcs2Utf8Lib under MdeModulePkg.

In v3, Add jansson library as the required submoudle in
CiSettings.py for CI test.
In v2, JsonLib is moved to under RedfishPkg.

edk2 JSON library is based on jansson open source
(https://github.com/akheron/jansson) and wrapped as an edk2
library.
edk2 JsonLib will be used by edk2 Redfish feature drivers (not
contributed yet) and the edk2 port of libredfish library (not
contributed yet) based on DMTF GitHub
(https://github.com/DMTF/libredfish).

Jansson is licensed under the MIT license(refer to ReadMe.rst
under
edk2).
It is used in production and its API is stable. In UEFI/EDKII
environment, Redfish project consumes jansson to achieve JSON
operations.

* Jansson version on edk2: 2.13.1

* EDKII jansson library wrapper:
- JsonLib.h:
This is the denifitions of EDKII JSON APIs which are mapped to
jannson funcitons accordingly.

- JanssonJsonLibMapping.h:
This is the wrapper file to map funcitons and definitions used in
native jannson applications to edk2 JsonLib. This avoids the
modifications on native jannson applications to be built under
edk2 environment.
Can you explain the use case for this in more detail?
What are these native jannson applications and why do we need to
build them in edk2?
If we have the jannson submodule, why can't these apps just use the
standard jannson services?

I ask because this is a new concept for edk2 and I want to make sure
it is really required.
The users of jannson library are LibRedfish open source
(https://github.com/DMTF/libredfish) and the Edk2 Redfish features
drivers (WIP) to communicate with Redfish service. EDK2 port of Libredfish
will be sent for review after this one.
Libredfish use "jansson.h" in its source code and uses the native
jansson functions directly. I think most of open source projects use the
same way to leverage jansson open source project.
However, "jansson.h" is defined in the source code directory that edk2
module can't refer to it in edk2 metafiles.

Why not? The package DEC file can list include paths.
Mike, I thought we are recommended to not referring to the header file in the source code or the private header files in the module. My memory may out of date but I remember that the package's "Include/" directory should contains all public headers files that are exposed to other packages or this package. Is this still valid in the edk2 driver writer's guide? I can't find any similar sentence which restrict to use the header files out of "Include/" directory in UEFI driver writer's guide.
But any way, that's why we use a wrapper header file and put it under Include/.


For example, the RedFishPkg DEC file currently has the following [Includes]
section:

[Includes]
Include

I could be updated to:

[Includes]
Include
Library\JsonLib\jansson\src

This would allow libs/modules that want to directly use the jansson services
to use

#include <janson.h>

If you wanted to limit the use of <janson.h> to only libs/modules in the
RedfishPkg, then you could use the private include feature:

[Includes]
Include

[Includes.Common.Private]
Library\JsonLib\jansson\src

In fact, this exact pattern is used in the UnitTestFrameworkPkg to allow
comonponents to use the standard includes from the cmocka submodule:

[Includes]
Library/CmockaLib/cmocka/include

[Includes.Common.Private]
PrivateInclude
Library/CmockaLib/cmocka/include/cmockery


Please evaluate this approach and see of the JassonJsoonMapping.h file can
be removed.

Thus we need an wrapper for these jansson applications. That
JanssonJsonMapping.h defines the EDK2 style API for mapping native
jansson functions. For those edk2 based JSON applications, they can just
use edk2 style APIs. Such as the edk2 Redfish feature drivers, those can use
the EDK2 coding style compliant API to invoke jansson functions by using
JsonLib.h.
For those native jansson applications, we can just use
JanssonJsonMapping.h to map the native jansson API to EDK2 style API
provided by JsonLib.lib. JanssonJsonMapping.h is not just map the
functions, it also gives edk2 style prototype for jansson variables such as
json_t. These edk2 style prototypes are used in EDK2 Redfish drivers or other
edk2 based application as well.



*Known issue:
Build fail with jansson/src/load.c, overrride and add code in load.c
to conditionally use stdin according to HAVE_UNISTD_H macro.
The PR is submitted to jansson open source community.
https://github.com/akheron/jansson/pull/558

Signed-off-by: Abner Chang <abner.chang@...>

Cc: Sean Brogan <sean.brogan@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Andrew Fish <afish@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Leif Lindholm <leif@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Nickle Wang <nickle.wang@...>
Cc: Peter O'Hanley <peter.ohanley@...>

Abner Chang (6):
RedfishPkg/Ucs2Utf8lib: UCS2 to UFT8 manipulation library
edk2: jansson submodule for edk2 JSON library
RedfishPkg/CrtLib: C runtime library
RedfishPkg/library: EDK2 port of jansson library
RedfishPkg: Add EDK2 port of jansson library to build
.pytool: Add required submodule for JsonLib

.gitmodules | 3 +
.pytool/CISettings.py | 2 +
ReadMe.rst | 1 +
RedfishPkg/Include/JanssonJsonMapping.h | 63 +
RedfishPkg/Include/Library/BaseUcs2Utf8Lib.h | 69 +
RedfishPkg/Include/Library/CrtLib.h | 195 +++
RedfishPkg/Include/Library/JsonLib.h | 768 ++++++++++++
.../Library/BaseUcs2Utf8Lib/BaseUcs2Utf8Lib.c | 417 +++++++
.../BaseUcs2Utf8Lib/BaseUcs2Utf8Lib.inf | 31 +
RedfishPkg/Library/CrtLib/CrtLib.c | 705 +++++++++++
RedfishPkg/Library/CrtLib/CrtLib.inf | 37 +
RedfishPkg/Library/JsonLib/JsonLib.c | 961 ++++++++++++++
RedfishPkg/Library/JsonLib/JsonLib.inf | 101 ++
RedfishPkg/Library/JsonLib/Readme.rst | 40 +
RedfishPkg/Library/JsonLib/assert.h | 16 +
RedfishPkg/Library/JsonLib/errno.h | 16 +
RedfishPkg/Library/JsonLib/jansson | 1 +
RedfishPkg/Library/JsonLib/jansson_config.h | 46 +
.../Library/JsonLib/jansson_private_config.h | 19 +
RedfishPkg/Library/JsonLib/limits.h | 16 +
RedfishPkg/Library/JsonLib/load.c | 1111
+++++++++++++++++
RedfishPkg/Library/JsonLib/math.h | 16 +
RedfishPkg/Library/JsonLib/stdarg.h | 15 +
RedfishPkg/Library/JsonLib/stddef.h | 16 +
RedfishPkg/Library/JsonLib/stdio.h | 15 +
RedfishPkg/Library/JsonLib/stdlib.h | 16 +
RedfishPkg/Library/JsonLib/string.h | 16 +
RedfishPkg/Library/JsonLib/sys/time.h | 15 +
RedfishPkg/Library/JsonLib/sys/types.h | 15 +
RedfishPkg/Library/JsonLib/time.h | 15 +
RedfishPkg/RedfishLibs.dsc.inc | 3 +
RedfishPkg/RedfishPkg.ci.yaml | 33 +
RedfishPkg/RedfishPkg.dec | 15 +
RedfishPkg/RedfishPkg.dsc | 3 +
34 files changed, 4811 insertions(+) create mode 100644
RedfishPkg/Include/JanssonJsonMapping.h
create mode 100644
RedfishPkg/Include/Library/BaseUcs2Utf8Lib.h
create mode 100644 RedfishPkg/Include/Library/CrtLib.h
create mode 100644 RedfishPkg/Include/Library/JsonLib.h
create mode 100644
RedfishPkg/Library/BaseUcs2Utf8Lib/BaseUcs2Utf8Lib.c
create mode 100644
RedfishPkg/Library/BaseUcs2Utf8Lib/BaseUcs2Utf8Lib.inf
create mode 100644 RedfishPkg/Library/CrtLib/CrtLib.c
create mode 100644 RedfishPkg/Library/CrtLib/CrtLib.inf
create mode 100644 RedfishPkg/Library/JsonLib/JsonLib.c
create mode 100644 RedfishPkg/Library/JsonLib/JsonLib.inf
create mode 100644 RedfishPkg/Library/JsonLib/Readme.rst
create mode 100644 RedfishPkg/Library/JsonLib/assert.h
create mode 100644 RedfishPkg/Library/JsonLib/errno.h
create mode 160000 RedfishPkg/Library/JsonLib/jansson
create mode 100644
RedfishPkg/Library/JsonLib/jansson_config.h
create mode 100644
RedfishPkg/Library/JsonLib/jansson_private_config.h
create mode 100644 RedfishPkg/Library/JsonLib/limits.h
create mode 100644 RedfishPkg/Library/JsonLib/load.c create
mode
100644 RedfishPkg/Library/JsonLib/math.h create mode 100644
RedfishPkg/Library/JsonLib/stdarg.h
create mode 100644 RedfishPkg/Library/JsonLib/stddef.h
create mode 100644 RedfishPkg/Library/JsonLib/stdio.h
create mode 100644 RedfishPkg/Library/JsonLib/stdlib.h
create mode 100644 RedfishPkg/Library/JsonLib/string.h
create mode 100644 RedfishPkg/Library/JsonLib/sys/time.h
create mode 100644 RedfishPkg/Library/JsonLib/sys/types.h
create mode 100644 RedfishPkg/Library/JsonLib/time.h

--
2.17.1















Join {devel@edk2.groups.io to automatically receive all group messages.