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
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
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
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>
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
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
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>
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
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
>
>
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>
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
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
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
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.