[RFC,1/3] os: begin separating some OS compatibility from EAL
Checks
Commit Message
Some library functionality we may want ahead of EAL build depends upon
some OS-specific functionality, so we create a new lib for that to be
built separately. For now, just includes fnmatch function for windows.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
lib/eal/windows/meson.build | 1 -
lib/meson.build | 11 ++++++-----
lib/os/freebsd/fnmatch.c | 3 +++
lib/os/linux/fnmatch.c | 3 +++
lib/os/meson.build | 8 ++++++++
lib/os/os.c | 3 +++
lib/os/version.map | 7 +++++++
lib/{eal => os}/windows/fnmatch.c | 0
lib/{eal/windows/include => os/windows}/fnmatch.h | 0
9 files changed, 30 insertions(+), 6 deletions(-)
create mode 100644 lib/os/freebsd/fnmatch.c
create mode 100644 lib/os/linux/fnmatch.c
create mode 100644 lib/os/meson.build
create mode 100644 lib/os/os.c
create mode 100644 lib/os/version.map
rename lib/{eal => os}/windows/fnmatch.c (100%)
rename lib/{eal/windows/include => os/windows}/fnmatch.h (100%)
Comments
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Monday, 29 August 2022 17.19
> To: dev@dpdk.org
> Cc: Bruce Richardson
> Subject: [RFC PATCH 1/3] os: begin separating some OS compatibility
> from EAL
>
> Some library functionality we may want ahead of EAL build depends upon
> some OS-specific functionality, so we create a new lib for that to be
> built separately. For now, just includes fnmatch function for windows.
The description given in patch 0/3 mentions that this causes a circular dependency between the EAL and Log libraries. You should mention that here too. Until I re-read that, I didn't understand the need to move fnmatch() out of the EAL library - I was even sidetracking wildly, considering if it had to do with needing it on the host computer (for some host compiler checks).
FYI, and not important: fnmatch() is a C library function (man 3), not a System call (man 2). But obviously still O/S specific, since it is not included with the C library for Windows.
On Mon, Aug 29, 2022 at 08:57:53PM +0200, Morten Brørup wrote:
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent: Monday, 29 August 2022 17.19
> > To: dev@dpdk.org
> > Cc: Bruce Richardson
> > Subject: [RFC PATCH 1/3] os: begin separating some OS compatibility
> > from EAL
> >
> > Some library functionality we may want ahead of EAL build depends upon
> > some OS-specific functionality, so we create a new lib for that to be
> > built separately. For now, just includes fnmatch function for windows.
>
> The description given in patch 0/3 mentions that this causes a circular dependency between the EAL and Log libraries. You should mention that here too. Until I re-read that, I didn't understand the need to move fnmatch() out of the EAL library - I was even sidetracking wildly, considering if it had to do with needing it on the host computer (for some host compiler checks).
>
Sorry about that! :-)
> FYI, and not important: fnmatch() is a C library function (man 3), not a System call (man 2). But obviously still O/S specific, since it is not included with the C library for Windows.
>
Interesting, but as you say, it doesn't actually affect this patch.
/Bruce
On Mon, Aug 29, 2022 at 5:19 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> Some library functionality we may want ahead of EAL build depends upon
> some OS-specific functionality, so we create a new lib for that to be
> built separately. For now, just includes fnmatch function for windows.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> lib/eal/windows/meson.build | 1 -
> lib/meson.build | 11 ++++++-----
> lib/os/freebsd/fnmatch.c | 3 +++
> lib/os/linux/fnmatch.c | 3 +++
> lib/os/meson.build | 8 ++++++++
> lib/os/os.c | 3 +++
> lib/os/version.map | 7 +++++++
> lib/{eal => os}/windows/fnmatch.c | 0
> lib/{eal/windows/include => os/windows}/fnmatch.h | 0
> 9 files changed, 30 insertions(+), 6 deletions(-)
> create mode 100644 lib/os/freebsd/fnmatch.c
> create mode 100644 lib/os/linux/fnmatch.c
> create mode 100644 lib/os/meson.build
> create mode 100644 lib/os/os.c
> create mode 100644 lib/os/version.map
> rename lib/{eal => os}/windows/fnmatch.c (100%)
> rename lib/{eal/windows/include => os/windows}/fnmatch.h (100%)
>
> diff --git a/lib/eal/windows/meson.build b/lib/eal/windows/meson.build
> index 845e406ca1..e4b2427610 100644
> --- a/lib/eal/windows/meson.build
> +++ b/lib/eal/windows/meson.build
> @@ -18,7 +18,6 @@ sources += files(
> 'eal_mp.c',
> 'eal_thread.c',
> 'eal_timer.c',
> - 'fnmatch.c',
> 'getopt.c',
> 'rte_thread.c',
> )
> diff --git a/lib/meson.build b/lib/meson.build
> index c648f7d800..7b61b2a5d7 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -9,6 +9,7 @@
> # given as a dep, no need to mention ring. This is especially true for the
> # core libs which are widely reused, so their deps are kept to a minimum.
> libraries = [
> + 'os', # load os compatibility material
> 'kvargs', # eal depends on kvargs
> 'telemetry', # basic info querying
> 'eal', # everything depends on eal
> @@ -106,6 +107,7 @@ if cc.has_argument('-Wno-format-truncation')
> endif
>
> enabled_libs = [] # used to print summary at the end
> +default_deps = []
>
> foreach l:libraries
> build = true
> @@ -124,11 +126,7 @@ foreach l:libraries
> # use "deps" for internal DPDK dependencies, and "ext_deps" for
> # external package/library requirements
> ext_deps = []
> - deps = []
> - # eal is standard dependency once built
> - if dpdk_conf.has('RTE_LIB_EAL')
> - deps += ['eal']
> - endif
> + deps = default_deps
>
> if disabled_libs.contains(l)
> build = false
> @@ -271,4 +269,7 @@ foreach l:libraries
> if developer_mode
> message('lib/@0@: Defining dependency "@1@"'.format(l, name))
> endif
> + if name == 'os' or name == 'eal'
> + default_deps = [name]
> + endif
> endforeach
> diff --git a/lib/os/freebsd/fnmatch.c b/lib/os/freebsd/fnmatch.c
> new file mode 100644
> index 0000000000..ca8a050fda
> --- /dev/null
> +++ b/lib/os/freebsd/fnmatch.c
> @@ -0,0 +1,3 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2022 Intel Corporation
> + */
> diff --git a/lib/os/linux/fnmatch.c b/lib/os/linux/fnmatch.c
> new file mode 100644
> index 0000000000..ca8a050fda
> --- /dev/null
> +++ b/lib/os/linux/fnmatch.c
> @@ -0,0 +1,3 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2022 Intel Corporation
> + */
> diff --git a/lib/os/meson.build b/lib/os/meson.build
> new file mode 100644
> index 0000000000..53949ca17e
> --- /dev/null
> +++ b/lib/os/meson.build
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2022 Intel Corporation
> +
> +includes += global_inc
> +includes += include_directories(exec_env)
> +sources += files(
> + exec_env / 'fnmatch.c',
> +)
Not really important (that's only a RFC), but os.c is not compiled anywhere.
> diff --git a/lib/os/os.c b/lib/os/os.c
> new file mode 100644
> index 0000000000..ca8a050fda
> --- /dev/null
> +++ b/lib/os/os.c
> @@ -0,0 +1,3 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2022 Intel Corporation
> + */
> diff --git a/lib/os/version.map b/lib/os/version.map
> new file mode 100644
> index 0000000000..e1dbd6051e
> --- /dev/null
> +++ b/lib/os/version.map
> @@ -0,0 +1,7 @@
> +DPDK_22 {
> + global:
> +
> + fnmatch;
> +
> + local: *;
> +};
Could we perhaps consider a per-os version.map file or some kind of
inclusion of os specific symbols?
That would avoid odd exporting of a symbol that is provided by the C
library in other OS.
> diff --git a/lib/eal/windows/fnmatch.c b/lib/os/windows/fnmatch.c
> similarity index 100%
> rename from lib/eal/windows/fnmatch.c
> rename to lib/os/windows/fnmatch.c
> diff --git a/lib/eal/windows/include/fnmatch.h b/lib/os/windows/fnmatch.h
> similarity index 100%
> rename from lib/eal/windows/include/fnmatch.h
> rename to lib/os/windows/fnmatch.h
> --
> 2.34.1
>
On Tue, Aug 30, 2022 at 10:42:43AM +0200, David Marchand wrote:
> On Mon, Aug 29, 2022 at 5:19 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > Some library functionality we may want ahead of EAL build depends upon
> > some OS-specific functionality, so we create a new lib for that to be
> > built separately. For now, just includes fnmatch function for windows.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> > lib/eal/windows/meson.build | 1 -
> > lib/meson.build | 11 ++++++-----
> > lib/os/freebsd/fnmatch.c | 3 +++
> > lib/os/linux/fnmatch.c | 3 +++
> > lib/os/meson.build | 8 ++++++++
> > lib/os/os.c | 3 +++
> > lib/os/version.map | 7 +++++++
> > lib/{eal => os}/windows/fnmatch.c | 0
> > lib/{eal/windows/include => os/windows}/fnmatch.h | 0
> > 9 files changed, 30 insertions(+), 6 deletions(-)
> > create mode 100644 lib/os/freebsd/fnmatch.c
> > create mode 100644 lib/os/linux/fnmatch.c
> > create mode 100644 lib/os/meson.build
> > create mode 100644 lib/os/os.c
> > create mode 100644 lib/os/version.map
> > rename lib/{eal => os}/windows/fnmatch.c (100%)
> > rename lib/{eal/windows/include => os/windows}/fnmatch.h (100%)
> >
> > diff --git a/lib/eal/windows/meson.build b/lib/eal/windows/meson.build
> > index 845e406ca1..e4b2427610 100644
> > --- a/lib/eal/windows/meson.build
> > +++ b/lib/eal/windows/meson.build
> > @@ -18,7 +18,6 @@ sources += files(
> > 'eal_mp.c',
> > 'eal_thread.c',
> > 'eal_timer.c',
> > - 'fnmatch.c',
> > 'getopt.c',
> > 'rte_thread.c',
> > )
> > diff --git a/lib/meson.build b/lib/meson.build
> > index c648f7d800..7b61b2a5d7 100644
> > --- a/lib/meson.build
> > +++ b/lib/meson.build
> > @@ -9,6 +9,7 @@
> > # given as a dep, no need to mention ring. This is especially true for the
> > # core libs which are widely reused, so their deps are kept to a minimum.
> > libraries = [
> > + 'os', # load os compatibility material
> > 'kvargs', # eal depends on kvargs
> > 'telemetry', # basic info querying
> > 'eal', # everything depends on eal
> > @@ -106,6 +107,7 @@ if cc.has_argument('-Wno-format-truncation')
> > endif
> >
> > enabled_libs = [] # used to print summary at the end
> > +default_deps = []
> >
> > foreach l:libraries
> > build = true
> > @@ -124,11 +126,7 @@ foreach l:libraries
> > # use "deps" for internal DPDK dependencies, and "ext_deps" for
> > # external package/library requirements
> > ext_deps = []
> > - deps = []
> > - # eal is standard dependency once built
> > - if dpdk_conf.has('RTE_LIB_EAL')
> > - deps += ['eal']
> > - endif
> > + deps = default_deps
> >
> > if disabled_libs.contains(l)
> > build = false
> > @@ -271,4 +269,7 @@ foreach l:libraries
> > if developer_mode
> > message('lib/@0@: Defining dependency "@1@"'.format(l, name))
> > endif
> > + if name == 'os' or name == 'eal'
> > + default_deps = [name]
> > + endif
> > endforeach
> > diff --git a/lib/os/freebsd/fnmatch.c b/lib/os/freebsd/fnmatch.c
> > new file mode 100644
> > index 0000000000..ca8a050fda
> > --- /dev/null
> > +++ b/lib/os/freebsd/fnmatch.c
> > @@ -0,0 +1,3 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2022 Intel Corporation
> > + */
> > diff --git a/lib/os/linux/fnmatch.c b/lib/os/linux/fnmatch.c
> > new file mode 100644
> > index 0000000000..ca8a050fda
> > --- /dev/null
> > +++ b/lib/os/linux/fnmatch.c
> > @@ -0,0 +1,3 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2022 Intel Corporation
> > + */
> > diff --git a/lib/os/meson.build b/lib/os/meson.build
> > new file mode 100644
> > index 0000000000..53949ca17e
> > --- /dev/null
> > +++ b/lib/os/meson.build
> > @@ -0,0 +1,8 @@
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright(c) 2022 Intel Corporation
> > +
> > +includes += global_inc
> > +includes += include_directories(exec_env)
> > +sources += files(
> > + exec_env / 'fnmatch.c',
> > +)
>
> Not really important (that's only a RFC), but os.c is not compiled anywhere.
>
>
> > diff --git a/lib/os/os.c b/lib/os/os.c
> > new file mode 100644
> > index 0000000000..ca8a050fda
> > --- /dev/null
> > +++ b/lib/os/os.c
> > @@ -0,0 +1,3 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2022 Intel Corporation
> > + */
> > diff --git a/lib/os/version.map b/lib/os/version.map
> > new file mode 100644
> > index 0000000000..e1dbd6051e
> > --- /dev/null
> > +++ b/lib/os/version.map
> > @@ -0,0 +1,7 @@
> > +DPDK_22 {
> > + global:
> > +
> > + fnmatch;
> > +
> > + local: *;
> > +};
>
> Could we perhaps consider a per-os version.map file or some kind of
> inclusion of os specific symbols?
> That would avoid odd exporting of a symbol that is provided by the C
> library in other OS.
>
Yep, should be possible, especially if we update our minimum meson to 0.53,
allowing us to use the fs module to check if particular files exist. That
would allow us to check for a version.map.<exec_env> file and use that if
it exists, falling back to regular version.map file if not.
Alternatively, we could use a script to create the version.map files, by
appending an exec_env-specific version, if one exists, to the main
version.map file for each lib.
/Bruce
@@ -18,7 +18,6 @@ sources += files(
'eal_mp.c',
'eal_thread.c',
'eal_timer.c',
- 'fnmatch.c',
'getopt.c',
'rte_thread.c',
)
@@ -9,6 +9,7 @@
# given as a dep, no need to mention ring. This is especially true for the
# core libs which are widely reused, so their deps are kept to a minimum.
libraries = [
+ 'os', # load os compatibility material
'kvargs', # eal depends on kvargs
'telemetry', # basic info querying
'eal', # everything depends on eal
@@ -106,6 +107,7 @@ if cc.has_argument('-Wno-format-truncation')
endif
enabled_libs = [] # used to print summary at the end
+default_deps = []
foreach l:libraries
build = true
@@ -124,11 +126,7 @@ foreach l:libraries
# use "deps" for internal DPDK dependencies, and "ext_deps" for
# external package/library requirements
ext_deps = []
- deps = []
- # eal is standard dependency once built
- if dpdk_conf.has('RTE_LIB_EAL')
- deps += ['eal']
- endif
+ deps = default_deps
if disabled_libs.contains(l)
build = false
@@ -271,4 +269,7 @@ foreach l:libraries
if developer_mode
message('lib/@0@: Defining dependency "@1@"'.format(l, name))
endif
+ if name == 'os' or name == 'eal'
+ default_deps = [name]
+ endif
endforeach
new file mode 100644
@@ -0,0 +1,3 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2022 Intel Corporation
+ */
new file mode 100644
@@ -0,0 +1,3 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2022 Intel Corporation
+ */
new file mode 100644
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2022 Intel Corporation
+
+includes += global_inc
+includes += include_directories(exec_env)
+sources += files(
+ exec_env / 'fnmatch.c',
+)
new file mode 100644
@@ -0,0 +1,3 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2022 Intel Corporation
+ */
new file mode 100644
@@ -0,0 +1,7 @@
+DPDK_22 {
+ global:
+
+ fnmatch;
+
+ local: *;
+};
similarity index 100%
rename from lib/eal/windows/fnmatch.c
rename to lib/os/windows/fnmatch.c
similarity index 100%
rename from lib/eal/windows/include/fnmatch.h
rename to lib/os/windows/fnmatch.h