[dpdk-dev] [PATCH 1/2] eal/windows: fix symbol export

[dpdk-dev] [PATCH 1/2] eal/windows: fix symbol export

[David Marchand]

The incriminated commit forgot to clean the Windows export file.

Fixes: 3cd73a1a1c4d ("eal: simplify exit functions")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/librte_eal/rte_eal_exports.def | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/librte_eal/rte_eal_exports.def b/lib/librte_eal/rte_eal_exports.def
index 16f8e33874..975acb8ffe 100644
--- a/lib/librte_eal/rte_eal_exports.def
+++ b/lib/librte_eal/rte_eal_exports.def
@@ -32,7 +32,6 @@ EXPORTS
 	rte_devargs_remove
 	rte_devargs_type_count
 	rte_dump_physmem_layout
-	rte_dump_registers
 	rte_dump_stack
 	rte_dump_tailq
 	rte_eal_alarm_cancel
-- 
2.23.0

[dpdk-dev] [PATCH 2/2] devtools: check Windows export files

[David Marchand]

Updating export files (supposed to disappear at some point, but still
there) might be missed when removing symbols in the API / map files.
Add a check for this case.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 devtools/check-symbol-maps.sh | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/devtools/check-symbol-maps.sh b/devtools/check-symbol-maps.sh
index 7fdfaa11c4..4a77fde12b 100755
--- a/devtools/check-symbol-maps.sh
+++ b/devtools/check-symbol-maps.sh
@@ -7,6 +7,8 @@ cd $(dirname $0)/..
 # speed up by ignoring Unicode details
 export LC_ALL=C
 
+ret=0
+
 find_orphan_symbols ()
 {
     for map in $(find lib drivers -name '*.map') ; do
@@ -30,5 +32,25 @@ orphan_symbols=$(find_orphan_symbols)
 if [ -n "$orphan_symbols" ] ; then
     echo "Found only in symbol map file:"
     echo "$orphan_symbols" | sed 's,^,\t,'
-    exit 1
+    ret=1
+fi
+
+validate_windows_exports ()
+{
+    for map in $(find lib drivers -name '*.map') ; do
+        def=${map/_version.map}_exports.def
+        [ -e $def ] || continue
+        for sym in $(grep -v ^EXPORTS $def); do
+            grep -q $sym $map || echo $sym
+        done
+    done
+}
+
+unknown_windows_symbols=$(validate_windows_exports)
+if [ -n "$unknown_windows_symbols" ] ; then
+    echo "Found only in Windows export file:"
+    echo "$unknown_windows_symbols" | sed 's,^,\t,'
+    ret=1
 fi
+
+exit $ret
-- 
2.23.0

Re: [dpdk-dev] [PATCH 2/2] devtools: check Windows export files

[David Marchand]

On Fri, Oct 16, 2020 at 11:39 AM David Marchand
<david.marchand@redhat.com> wrote:
> +validate_windows_exports ()
> +{
> +    for map in $(find lib drivers -name '*.map') ; do
> +        def=${map/_version.map}_exports.def
> +        [ -e $def ] || continue

I should have inverted this logic and look for exports.def files.
I'll respin.


-- 
David marchand

Re: [dpdk-dev] [PATCH 1/2] eal/windows: fix symbol export

[Thomas Monjalon]

16/10/2020 11:38, David Marchand:
> The incriminated commit forgot to clean the Windows export file.
> 
> Fixes: 3cd73a1a1c4d ("eal: simplify exit functions")

Yes the patch has been merged after the symbol has been added in the .def.

> Signed-off-by: David Marchand <david.marchand@redhat.com>

Acked-by: Thomas Monjalon <thomas@monjalon.net>

[dpdk-dev] [PATCH v2 1/2] eal/windows: fix symbol export

[David Marchand]

The incriminated commit forgot to clean the Windows export file.

Fixes: 3cd73a1a1c4d ("eal: simplify exit functions")

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/librte_eal/rte_eal_exports.def | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/librte_eal/rte_eal_exports.def b/lib/librte_eal/rte_eal_exports.def
index 16f8e33874..975acb8ffe 100644
--- a/lib/librte_eal/rte_eal_exports.def
+++ b/lib/librte_eal/rte_eal_exports.def
@@ -32,7 +32,6 @@ EXPORTS
 	rte_devargs_remove
 	rte_devargs_type_count
 	rte_dump_physmem_layout
-	rte_dump_registers
 	rte_dump_stack
 	rte_dump_tailq
 	rte_eal_alarm_cancel
-- 
2.23.0

[dpdk-dev] [PATCH v2 2/2] devtools: check Windows export files

[David Marchand]

Updating export files (supposed to disappear at some point, but still
there) might be missed when removing symbols in the API / map files.
Add a check for this case.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changelog since v1:
- invert logic, as .def files are the exception,
- reuse orphan denomination,

---
 devtools/check-symbol-maps.sh | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/devtools/check-symbol-maps.sh b/devtools/check-symbol-maps.sh
index 7fdfaa11c4..dae92f77a4 100755
--- a/devtools/check-symbol-maps.sh
+++ b/devtools/check-symbol-maps.sh
@@ -7,6 +7,8 @@ cd $(dirname $0)/..
 # speed up by ignoring Unicode details
 export LC_ALL=C
 
+ret=0
+
 find_orphan_symbols ()
 {
     for map in $(find lib drivers -name '*.map') ; do
@@ -30,5 +32,24 @@ orphan_symbols=$(find_orphan_symbols)
 if [ -n "$orphan_symbols" ] ; then
     echo "Found only in symbol map file:"
     echo "$orphan_symbols" | sed 's,^,\t,'
-    exit 1
+    ret=1
 fi
+
+find_orphan_windows_symbols ()
+{
+    for def in $(find lib drivers -name '*_exports.def') ; do
+        map=${def/_exports.def}_version.map
+        for sym in $(grep -v ^EXPORTS $def); do
+            grep -q $sym $map || echo $sym
+        done
+    done
+}
+
+orphan_windows_symbols=$(find_orphan_windows_symbols)
+if [ -n "$orphan_windows_symbols" ] ; then
+    echo "Found only in Windows export file:"
+    echo "$orphan_windows_symbols" | sed 's,^,\t,'
+    ret=1
+fi
+
+exit $ret
-- 
2.23.0

Re: [dpdk-dev] [PATCH v2 1/2] eal/windows: fix symbol export

[Kinsella, Ray]

The windows exports and the map files, feels like duplication of effort. 
Could we massage one into the other during the build?

On 16/10/2020 11:27, David Marchand wrote:
> The incriminated commit forgot to clean the Windows export file.
> 
> Fixes: 3cd73a1a1c4d ("eal: simplify exit functions")
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  lib/librte_eal/rte_eal_exports.def | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/lib/librte_eal/rte_eal_exports.def b/lib/librte_eal/rte_eal_exports.def
> index 16f8e33874..975acb8ffe 100644
> --- a/lib/librte_eal/rte_eal_exports.def
> +++ b/lib/librte_eal/rte_eal_exports.def
> @@ -32,7 +32,6 @@ EXPORTS
>  	rte_devargs_remove
>  	rte_devargs_type_count
>  	rte_dump_physmem_layout
> -	rte_dump_registers
>  	rte_dump_stack
>  	rte_dump_tailq
>  	rte_eal_alarm_cancel
> 

Acked-by: Ray Kinsella <mdr@ashroe.eu>

Re: [dpdk-dev] [PATCH v2 1/2] eal/windows: fix symbol export

[David Marchand]

On Fri, Oct 16, 2020 at 1:16 PM Kinsella, Ray <mdr@ashroe.eu> wrote:
>
>
> The windows exports and the map files, feels like duplication of effort.
> Could we massage one into the other during the build?

That's what is done with map-to-win.py, unless we have one exception
when the full library is not ready, like EAL.

https://git.dpdk.org/dpdk/tree/lib/meson.build#n152
https://git.dpdk.org/dpdk/tree/buildtools/map_to_win.py#n27


-- 
David Marchand

Re: [dpdk-dev] [PATCH v2 1/2] eal/windows: fix symbol export

[Kinsella, Ray]

ah ... perfect.

Ray K

On 16/10/2020 12:22, David Marchand wrote:
> On Fri, Oct 16, 2020 at 1:16 PM Kinsella, Ray <mdr@ashroe.eu> wrote:
>>
>>
>> The windows exports and the map files, feels like duplication of effort.
>> Could we massage one into the other during the build?
> 
> That's what is done with map-to-win.py, unless we have one exception
> when the full library is not ready, like EAL.
> 
> https://git.dpdk.org/dpdk/tree/lib/meson.build#n152
> https://git.dpdk.org/dpdk/tree/buildtools/map_to_win.py#n27
> 
> 

Re: [dpdk-dev] [PATCH v2 2/2] devtools: check Windows export files

[Thomas Monjalon]

16/10/2020 12:27, David Marchand:
> Updating export files (supposed to disappear at some point, but still
> there) might be missed when removing symbols in the API / map files.
> Add a check for this case.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> Changelog since v1:
> - invert logic, as .def files are the exception,
> - reuse orphan denomination,

Acked-by: Thomas Monjalon <thomas@monjalon.net>

Re: [dpdk-dev] [PATCH v2 2/2] devtools: check Windows export files

[David Marchand]

On Fri, Oct 16, 2020 at 1:56 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 16/10/2020 12:27, David Marchand:
> > Updating export files (supposed to disappear at some point, but still
> > there) might be missed when removing symbols in the API / map files.
> > Add a check for this case.
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> Acked-by: Thomas Monjalon <thomas@monjalon.net>

Series applied.


-- 
David Marchand

Re: [dpdk-dev] [PATCH v2 1/2] eal/windows: fix symbol export

[Kinsella, Ray]

On 16/10/2020 12:22, David Marchand wrote:
> On Fri, Oct 16, 2020 at 1:16 PM Kinsella, Ray <mdr@ashroe.eu> wrote:
>>
>>
>> The windows exports and the map files, feels like duplication of effort.
>> Could we massage one into the other during the build?
> 
> That's what is done with map-to-win.py, unless we have one exception
> when the full library is not ready, like EAL.
> 
> https://git.dpdk.org/dpdk/tree/lib/meson.build#n152
> https://git.dpdk.org/dpdk/tree/buildtools/map_to_win.py#n27
> 

Thinking about this again - future work might be to check if a .def exists.
And then to warn in checkpatch if one is modified without the other?

Ray K
 

Re: [dpdk-dev] [PATCH v2 1/2] eal/windows: fix symbol export

[Bruce Richardson]

On Fri, Oct 16, 2020 at 04:48:59PM +0100, Kinsella, Ray wrote:
> 
> On 16/10/2020 12:22, David Marchand wrote:
> > On Fri, Oct 16, 2020 at 1:16 PM Kinsella, Ray <mdr@ashroe.eu> wrote:
> >>
> >>
> >> The windows exports and the map files, feels like duplication of effort.
> >> Could we massage one into the other during the build?
> > 
> > That's what is done with map-to-win.py, unless we have one exception
> > when the full library is not ready, like EAL.
> > 
> > https://git.dpdk.org/dpdk/tree/lib/meson.build#n152
> > https://git.dpdk.org/dpdk/tree/buildtools/map_to_win.py#n27
> > 
> 
> Thinking about this again - future work might be to check if a .def exists.
> And then to warn in checkpatch if one is modified without the other?
> 
Possibly, but right now there is only the one .def override file, for EAL,
and a better use of effort is to close the gap between windows and other
versions so that it can be removed completely.

$ find lib/ drivers/ -name *.def
lib/librte_eal/rte_eal_exports.def

/Bruce

Re: [dpdk-dev] [PATCH v2 1/2] eal/windows: fix symbol export

[Thomas Monjalon]

16/10/2020 17:48, Kinsella, Ray:
> On 16/10/2020 12:22, David Marchand wrote:
> > On Fri, Oct 16, 2020 at 1:16 PM Kinsella, Ray <mdr@ashroe.eu> wrote:
> >>
> >> The windows exports and the map files, feels like duplication of effort.
> >> Could we massage one into the other during the build?
> > 
> > That's what is done with map-to-win.py, unless we have one exception
> > when the full library is not ready, like EAL.
> > 
> > https://git.dpdk.org/dpdk/tree/lib/meson.build#n152
> > https://git.dpdk.org/dpdk/tree/buildtools/map_to_win.py#n27
> 
> Thinking about this again - future work might be to check if a .def exists.
> And then to warn in checkpatch if one is modified without the other?

It would not have avoided this miss,
because the symbol has been added in the .def
after I've sent my patch removing the function.
What missed was a check to run before pushing,
and this is what David did in the patch 2 of this series.